Skip to content

Fallback vectorization for FunctionExpr and BaseMacroFunctionExpr.#16366

Merged
gianm merged 11 commits intoapache:masterfrom
gianm:expr-vec-fallback
Jun 6, 2024
Merged

Fallback vectorization for FunctionExpr and BaseMacroFunctionExpr.#16366
gianm merged 11 commits intoapache:masterfrom
gianm:expr-vec-fallback

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented May 1, 2024

This patch adds FallbackVectorProcessor, a processor that adapts non-vectorizable operations into vectorizable ones. It is used in FunctionExpr and BaseMacroFunctionExpr. As a result, all such expressions can now participate in vectorized queries.

In addition:

  • Identifiers are updated to offer getObjectVector for ARRAY and COMPLEX in addition to STRING. ExprEvalObjectVector is updated to offer ARRAY and COMPLEX as well. Identifiers already did return true from canVectorize, so this enables them to live up to their claims.

  • In SQL tests, cannotVectorize now fails tests if an exception is not thrown. This makes it easier to identify tests that can now vectorize.

  • Fixes a null-matcher bug in StringObjectVectorValueMatcher that was uncovered by certain newly-vectorizable test cases.

Benchmarks follow for SqlExpressionBenchmark queries 26 and 27. These two queries are:

-- 26: group by string expr with non-expr agg
SELECT CONCAT(string2, '-', long2), SUM(double1) FROM foo GROUP BY 1 ORDER BY 2

-- 27: group by string expr with expr agg
SELECT CONCAT(string2, '-', long2), SUM(long1 * double4) FROM foo GROUP BY 1 ORDER BY 2

In these cases fallback vectorization is not as compelling as proper vectorization, but it's better than unvectorized execution. The relative benefit is greater for query 27, likely because fallback vectorization for CONCAT enables the long1 * double4 to vectorize as well. In general I would expect the benefit to be greater for more complex queries, due to this effect.

Benchmark                        (query)  (rowsPerSegment)  (schema)  (vectorize)  Mode  Cnt    Score    Error  Units

SqlExpressionBenchmark.querySql       26            500000      auto        false  avgt    5  259.288 ±  4.863  ms/op
SqlExpressionBenchmark.querySql       26            500000      auto       native  avgt    5  194.405 ± 10.185  ms/op
SqlExpressionBenchmark.querySql       26            500000      auto     fallback  avgt    5  244.807 ± 6.851  ms/op

SqlExpressionBenchmark.querySql       27            500000      auto        false  avgt    5  289.065 ± 15.253  ms/op
SqlExpressionBenchmark.querySql       27            500000      auto       native  avgt    5  194.829 ±  7.732  ms/op
SqlExpressionBenchmark.querySql       27            500000      auto     fallback  avgt    5  248.331 ± 4.732  ms/op

This patch adds FallbackVectorProcessor, a processor that adapts non-vectorizable
operations into vectorizable ones. It is used in FunctionExpr and BaseMacroFunctionExpr.

In addition:

- Identifiers are updated to offer getObjectVector for ARRAY and COMPLEX in addition
  to STRING. ExprEvalObjectVector is updated to offer ARRAY and COMPLEX as well.

- In SQL tests, cannotVectorize now fails tests if an exception is not thrown. This makes
  it easier to identify tests that can now vectorize.

- Fix a null-matcher bug in StringObjectVectorValueMatcher.
Copy link
Copy Markdown
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

for (int i = 0; i < mask.getSelectionSize(); i++) {
final int rowNum = mask.getSelection()[i];
if ((value == null && includeUnknown) || Objects.equals(value, vector[rowNum])) {
if ((vector[rowNum] == null && includeUnknown) || Objects.equals(value, vector[rowNum])) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍

* because bindings should only be used by identifiers, and this fallback processor is never used to
* implement identifiers.
*/
private static class UnusedBinding implements Expr.ObjectBinding
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this seems pretty similar to the thing made by InputBindings.validateConstant which is used during sql planning to reduce constant expressions, maybe should consolidate.

I guess not totally possible though since the other one attaches the expression using the constant bindings for nicer validation messages, but that's not a thing here so it can be static and doesn't have the need since it should only be used for identifiers

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they are very similar, but probably makes sense to keep them different for error-message reasons.

@gianm gianm merged commit 2770064 into apache:master Jun 6, 2024
@gianm gianm deleted the expr-vec-fallback branch June 6, 2024 03:03
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
gianm added a commit to gianm/druid that referenced this pull request Sep 9, 2025
PR apache#16366 originally added fallback vectorization, a mechanism for
making all expressions vectorizable. Later, apache#17098 fixed some issues
that arose and apache#17248 disabled fallback vectorization in the out-of-box
configuration.

This patch fixes various remaining issues with inconsistent type handling
between the vectorized and nonvectorized expr implementations. It does not
yet re-enable fallback vectorization out of the box, due to remaining
inconsistencies with conditional exprs like "case_searched", "case_simple",
and "if".

1) Aligns the behavior of missing columns and literal nulls so they are
   always treated as null longs. This was already the case for vectorized
   identifiers, but non-vectorized identifiers and literal nulls were still
   represented as strings.

2) Replaces all occurrences of "ExprEval.of(null)" with either an explicit
   type, or a call to "ExprEval.ofMissing()". ofMissing is a new function
   for situations where an eval represents a null value of unknown type.
   It is equivalent to "ExprEval.ofLong(null)", but is a separate function
   for clarity at the call site.

3) Update "cast" to return the target type even for null values.

4) Update "greatest", "least", and "array" so they eval to types that
   match what is reported by "getOutputType".

5) Update "scalb" to coerce input strings as numbers, to better allow
   for type evolution and missing columns.

6) Update "reverse" to coerce inputs to strings, to better allow for
   type evolution and missing columns.
gianm added a commit that referenced this pull request Sep 18, 2025
* Additional expr type alignment.

PR #16366 originally added fallback vectorization, a mechanism for
making all expressions vectorizable. Later, #17098 fixed some issues
that arose and #17248 disabled fallback vectorization in the out-of-box
configuration.

This patch fixes various remaining issues with inconsistent type handling
between the vectorized and nonvectorized expr implementations. It does not
yet re-enable fallback vectorization out of the box, due to remaining
inconsistencies with conditional exprs like "case_searched", "case_simple",
and "if".

1) Aligns the behavior of missing columns and literal nulls so they are
   always treated as null longs. This was already the case for vectorized
   identifiers, but non-vectorized identifiers and literal nulls were still
   represented as strings.

2) Replaces all occurrences of "ExprEval.of(null)" with either an explicit
   type, or a call to "ExprEval.ofMissing()". ofMissing is a new function
   for situations where an eval represents a null value of unknown type.
   It is equivalent to "ExprEval.ofLong(null)", but is a separate function
   for clarity at the call site.

3) Update "cast" to return the target type even for null values.

4) Update "greatest", "least", and "array" so they eval to types that
   match what is reported by "getOutputType".

5) Update "scalb" to coerce input strings as numbers, to better allow
   for type evolution and missing columns.

6) Update "reverse" to coerce inputs to strings, to better allow for
   type evolution and missing columns.

* Restore fallback in testArrayFns.

* Fix issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants