You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "raghavyadav01 (via GitHub)" <gi...@apache.org> on 2026/10/22 03:13:55 UTC
[PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
raghavyadav01 opened a new pull request, #20001:
URL: https://github.com/apache/pinot/pull/20001
## Problem
On the multi-stage engine, `WHERE tags = 'urgent'` or `WHERE tags IN ('vip', 'normal')` on a multi-value column fails, while the single-stage engine answers it with "any element matches" semantics.
MV columns are ARRAY-typed in the planner, and the lenient BABEL conformance coerces the literal to an array: `=($3, CAST('urgent'):VARCHAR ARRAY)`. The leaf then gets `equals(tags, <array literal>)`, which is not a recognised MV predicate, so it runs as an expression filter that reads the MV column as single-value:
```
java.lang.UnsupportedOperationException
at org.apache.pinot.segment.spi.index.reader.ForwardIndexReader.readDictIds(ForwardIndexReader.java:106)
at org.apache.pinot.core.common.DataFetcher$ColumnValueReader.readStringValues(DataFetcher.java:392)
...
```
Above the leaf the same comparison fails with `ClassCastException: [Ljava.lang.String; cannot be cast to java.lang.Comparable`.
## Change
`PinotTypeCoercion`: for `=`, `<>`, `IN` and `NOT IN` between a plain MV column reference and a non-array value, wrap the column in `ARRAY_TO_MV` before the normal coercion. The query then plans exactly like the already supported `ARRAY_TO_MV(tags) = 'urgent'`, which is pushed to the leaf and uses MV semantics (`=`/`IN`: any element matches; `<>`/`NOT IN`: no element matches, same as the single-stage engine).
Deliberately not rewritten (they fail validation as before):
- range comparisons (`>`, `<`, ...): Calcite simplifies `NOT (mv > x)` into `mv <= x`, which has different MV semantics than the single-stage `NOT`;
- comparisons where coercion would cast the MV column itself (e.g. `tags = 5`); casting only the literal (`codes = '5'`) is fine;
- a NULL in the IN list, rejected with a clear error.
`ArrayToMvValidationVisitor`: when such a comparison ends up somewhere it cannot run (projection/CASE, outer join condition, a filter that cannot be pushed to the scan), the error now says that comparing a multi-value column to a value is only supported in a pushable WHERE filter and suggests `arrayContainsString` / `arrayContainsInt`.
None of the shapes that now get the new error returned rows before this change (they failed at planning or at runtime), so no working query is affected.
## Tests
- `pinot-query-runtime` `MultiValueFilters.json` (new, 23 cases): `=`, value on the left, `IN`, a 26-value `IN`, `= 'a' AND = 'b'`, INT MV, `<>`, `NOT (=)`, `NOT IN`, mixed-literal `codes = '5'`, below a join, LEFT JOIN ON, `COUNT(*) FILTER (WHERE ...)`, `arrayContainsString` above the leaf, plus error cases (ranges, `tags = 5`, NULL in IN, non-pushable positions). Results checked against single-stage semantics; the positive cases fail on master.
- Planner: `BasicQueryPlans.json` pins the `=(ARRAY_TO_MV($n), ...)` plan; `ValidationErrorPlan.json` pins the projection error.
- `ResourceBasedQueriesTest`, `QueryRunnerTest`, `QueryCompilationTest`, `ResourceBasedQueryPlansTest`, `SealedInListPlanningTest`, `PinotTypeCoercionTest`, `PinotRuleUtilsTest` pass.
Not covered: MV TIMESTAMP columns (no test fixture supports them); the TIMESTAMP-vs-BIGINT coercion rule is re-applied after wrapping.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #20001:
URL: https://github.com/apache/pinot/pull/20001#issuecomment-6429546876
@yashmayya @gortiz Please help review it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20001:
URL: https://github.com/apache/pinot/pull/20001#issuecomment-6334064445
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/20001?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 0.00%. Comparing base ([`2877723`](https://app.codecov.io/gh/apache/pinot/commit/2877723da384b783df372fb203fbf9fef0aea740?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`33fe606`](https://app.codecov.io/gh/apache/pinot/commit/33fe606cee2c58773b81c21be5785a4b4b5581b0?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (2877723) and HEAD (33fe606). Click for more details.
>
> <details><summary>HEAD has 18 uploads less than BASE</summary>
>
>| Flag | BASE (2877723) | HEAD (33fe606) |
>|------|------|------|
>|lane-a|2|1|
>|integration|4|2|
>|java-25|6|2|
>|integration1|2|0|
>|temurin|6|2|
>|lane-b|2|1|
>|unittests1|1|0|
>|unittests|2|0|
>|unittests2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #20001 +/- ##
=============================================
- Coverage 69.67% 0.00% -69.68%
=============================================
Files 3548 3 -3545
Lines 234929 6 -234923
Branches 37668 0 -37668
=============================================
- Hits 163677 0 -163677
+ Misses 58582 6 -58576
+ Partials 12670 0 -12670
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-69.68%)` | :arrow_down: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-100.00%)` | :arrow_down: |
| [lane-b](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (-69.68%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/20001/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/20001?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #20001:
URL: https://github.com/apache/pinot/pull/20001#discussion_r4386350482
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/validation/ArrayToMvValidationVisitor.java:
##########
@@ -172,6 +174,43 @@ public Void visitUnnest(UnnestNode node, Boolean isIntermediateStage) {
return null;
}
+ /// Comparing a multi-value column to a value (e.g. `mvCol = 'a'`) is planned as a comparison on `ARRAY_TO_MV`. As
+ /// the query does not contain `ARRAY_TO_MV` in this case, the error explains the comparison instead.
+ private QueryException arrayToMvNotSupported(String location, List<? extends RexExpression> expressions,
Review Comment:
Small consistency gap: the FILTER/JOIN/PROJECT paths now route through this helper and get the friendly "Comparing a multi-value column to a value..." message, but `visitAggregate` and `visitWindow` still throw the old generic "Function 'ArrayToMv' is not supported in ... Stage" text. An MV comparison that surfaces inside an aggregate or window call (e.g. in the intermediate stage) would get the less-helpful message. Non-blocking, but worth routing those two through `arrayToMvNotSupported` too for consistency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #20001:
URL: https://github.com/apache/pinot/pull/20001#discussion_r4352451191
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/validate/PinotTypeCoercion.java:
##########
@@ -108,7 +134,65 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) {
return super.binaryComparisonCoercion(binding);
}
+ @Override
+ public boolean inOperationCoercion(SqlCallBinding binding) {
+ SqlKind kind = binding.getOperator().getKind();
+ if ((kind == SqlKind.IN || kind == SqlKind.NOT_IN) && binding.operand(1) instanceof SqlNodeList
+ && isMultiValueColumn(binding, 0)
+ && isScalarValueListRejectingNull(binding, (SqlNodeList) binding.operand(1))) {
+ return coerceMultiValueComparison(binding, 0, () -> inOperationCoercion(binding));
+ }
+ return super.inOperationCoercion(binding);
+ }
+
+ /// Returns whether the operand at the given index is a reference to a multi-value (ARRAY type) column.
+ private static boolean isMultiValueColumn(SqlCallBinding binding, int index) {
+ return SqlTypeUtil.isArray(binding.getOperandType(index)) && isColumnReference(binding.operand(index));
+ }
+
+ /// Returns whether all values of the `IN` list are non-array values, and throws on a `NULL` value: Calcite types it
+ /// as the array type of the column, and the leaf stage cannot evaluate it in a multi-value predicate.
+ private static boolean isScalarValueListRejectingNull(SqlCallBinding binding, SqlNodeList values) {
+ for (SqlNode value : values) {
+ if (SqlUtil.isNullLiteral(value, false)) {
+ throw QueryErrorCode.QUERY_VALIDATION.asException(
+ "NULL is not supported in an IN list compared to multi-value column: " + binding.operand(0));
Review Comment:
Small asymmetry worth a note: a NULL in an `IN` list errors here, but `mvCol = NULL` on the binary path is silently wrapped and pushed down as `col = NULL` (always no-match). Both are defensible, but one form erroring while the sibling form is accepted may surprise users. Consider a one-line comment explaining the difference, and maybe a `mvCol = NULL` test so the accepted behavior is pinned.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/validation/ArrayToMvValidationVisitor.java:
##########
@@ -172,6 +174,42 @@ public Void visitUnnest(UnnestNode node, Boolean isIntermediateStage) {
return null;
}
+ /// Comparing a multi-value column to a value (e.g. `mvCol = 'a'`) is planned as a comparison on `ARRAY_TO_MV`. As
+ /// the query does not contain `ARRAY_TO_MV` in this case, the error explains the comparison instead.
+ private QueryException arrayToMvNotSupported(String location, List<? extends RexExpression> expressions,
+ String hint) {
+ if (containsMultiValueComparison(expressions)) {
+ return new QueryException(QueryErrorCode.QUERY_PLANNING,
+ "Comparing a multi-value column to a value (e.g. mvCol = 'a' or mvCol IN ('a', 'b')) is not supported in "
+ + location + ". It is only supported in a WHERE clause filter that can be pushed down to the table "
+ + "scan. Elsewhere, use an arrayContains function instead, e.g. arrayContainsString(mvCol, 'a')");
Review Comment:
Nit: for an INT MV column the only suggested example is `arrayContainsString`, which won't type-check. Since `arrayContainsInt` exists, a user on an INT column has to guess the right variant. Fine to leave as-is given you intentionally softened this to one example, but mentioning both (or picking by element type) would be friendlier.
##########
pinot-query-runtime/src/test/resources/queries/MultiValueFilters.json:
##########
@@ -0,0 +1,370 @@
+{
+ "mv_filter": {
+ "comment": "Comparisons of a multi-value column to a value. As in the single-stage engine, = and IN match a row if any value matches, <> and NOT IN if no value matches. They are only supported in filters pushed down to the leaf stage.",
+ "tables": {
+ "orders": {
+ "schema": [
+ {
+ "name": "order_id",
+ "type": "STRING"
+ },
+ {
+ "name": "customer_id",
+ "type": "STRING"
+ },
+ {
+ "name": "tags",
+ "type": "STRING",
+ "isSingleValue": false
+ },
+ {
+ "name": "codes",
+ "type": "INT",
+ "isSingleValue": false
+ }
+ ],
+ "inputs": [
+ [
+ "o1",
+ "c1",
+ [
+ "urgent",
+ "vip"
+ ],
+ [
+ 1,
+ 2
+ ]
+ ],
+ [
+ "o2",
+ "c1",
+ [
+ "urgent"
+ ],
+ [
+ 2
+ ]
+ ],
+ [
+ "o3",
+ "c2",
+ [
+ "urgent"
+ ],
+ [
+ 3
+ ]
+ ],
+ [
+ "o4",
+ "c2",
+ [
+ "vip"
+ ],
+ [
+ 4,
+ 5
+ ]
+ ],
+ [
+ "o5",
+ "c3",
+ [
+ "normal"
+ ],
+ [
+ 5
+ ]
+ ]
+ ]
+ },
+ "customers": {
+ "schema": [
+ {
+ "name": "customer_id",
+ "type": "STRING"
+ },
+ {
+ "name": "name",
+ "type": "STRING"
+ }
+ ],
+ "inputs": [
+ [
+ "c1",
+ "Alice"
+ ],
+ [
+ "c2",
+ "Bob"
+ ],
+ [
+ "c3",
+ "Charlie"
+ ]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "description": "Equality on a STRING MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE tags = 'urgent' ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o2"
+ ],
+ [
+ "o3"
+ ]
+ ]
+ },
+ {
+ "description": "Equality with the value on the left side",
+ "sql": "SELECT order_id FROM {orders} WHERE 'vip' = tags ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o4"
+ ]
+ ]
+ },
+ {
+ "description": "IN on a STRING MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE tags IN ('vip', 'normal') ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o4"
+ ],
+ [
+ "o5"
+ ]
+ ]
+ },
+ {
+ "description": "IN with a large value list on a STRING MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE tags IN ('v0', 'v1', 'v2', 'v3', 'v4', 'v5', 'v6', 'v7', 'v8', 'v9', 'v10', 'v11', 'v12', 'v13', 'v14', 'v15', 'v16', 'v17', 'v18', 'v19', 'v20', 'v21', 'v22', 'v23', 'v24', 'normal') ORDER BY order_id",
+ "outputs": [
+ [
+ "o5"
+ ]
+ ]
+ },
+ {
+ "description": "Two equalities on the same MV column are not folded to false",
+ "sql": "SELECT order_id FROM {orders} WHERE tags = 'urgent' AND tags = 'vip' ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ]
+ ]
+ },
+ {
+ "description": "Equality and IN on an INT MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE codes = 5 OR codes IN (1, 3) ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o3"
+ ],
+ [
+ "o4"
+ ],
+ [
+ "o5"
+ ]
+ ]
+ },
+ {
+ "description": "MV equality pushed below a join",
+ "sql": "SELECT o.order_id, c.name FROM {orders} o JOIN {customers} c ON o.customer_id = c.customer_id WHERE o.tags = 'urgent' ORDER BY o.order_id",
+ "outputs": [
+ [
+ "o1",
+ "Alice"
+ ],
+ [
+ "o2",
+ "Alice"
+ ],
+ [
+ "o3",
+ "Bob"
+ ]
+ ]
+ },
+ {
+ "description": "MV IN pushed below a join with an aggregate above it",
+ "sql": "SELECT c.name, COUNT(*) FROM {orders} o JOIN {customers} c ON o.customer_id = c.customer_id WHERE o.tags IN ('vip', 'normal') GROUP BY c.name ORDER BY c.name",
+ "outputs": [
+ [
+ "Alice",
+ 1
+ ],
+ [
+ "Bob",
+ 1
+ ],
+ [
+ "Charlie",
+ 1
+ ]
+ ]
+ },
+ {
+ "description": "Not-equals on an MV column matches rows with no equal value",
Review Comment:
Coverage gap (low): every input row has a non-empty, non-null MV. The `<>` / `NOT IN` "no value matches" complement is most error-prone on an empty or NULL MV row (where `= 'x'` is false and `<> 'x'` should be true). An extra row like `["o6","c3",[],[6]]` (or a null MV) extending the `<>` / `NOT IN` expected outputs would lock down that boundary — especially under null-handling-enabled.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/validation/ArrayToMvValidationVisitor.java:
##########
@@ -172,6 +174,42 @@ public Void visitUnnest(UnnestNode node, Boolean isIntermediateStage) {
return null;
}
+ /// Comparing a multi-value column to a value (e.g. `mvCol = 'a'`) is planned as a comparison on `ARRAY_TO_MV`. As
+ /// the query does not contain `ARRAY_TO_MV` in this case, the error explains the comparison instead.
+ private QueryException arrayToMvNotSupported(String location, List<? extends RexExpression> expressions,
+ String hint) {
+ if (containsMultiValueComparison(expressions)) {
+ return new QueryException(QueryErrorCode.QUERY_PLANNING,
+ "Comparing a multi-value column to a value (e.g. mvCol = 'a' or mvCol IN ('a', 'b')) is not supported in "
+ + location + ". It is only supported in a WHERE clause filter that can be pushed down to the table "
+ + "scan. Elsewhere, use an arrayContains function instead, e.g. arrayContainsString(mvCol, 'a')");
Review Comment:
Nit: for an INT MV column the only suggested example is `arrayContainsString`, which won't type-check. Since `arrayContainsInt` exists, a user on an INT column has to guess the right variant. Fine to leave as-is given you intentionally softened this to one example, but mentioning both (or picking by element type) would be friendlier.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/validate/PinotTypeCoercion.java:
##########
@@ -108,7 +134,65 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) {
return super.binaryComparisonCoercion(binding);
}
+ @Override
+ public boolean inOperationCoercion(SqlCallBinding binding) {
+ SqlKind kind = binding.getOperator().getKind();
+ if ((kind == SqlKind.IN || kind == SqlKind.NOT_IN) && binding.operand(1) instanceof SqlNodeList
+ && isMultiValueColumn(binding, 0)
+ && isScalarValueListRejectingNull(binding, (SqlNodeList) binding.operand(1))) {
+ return coerceMultiValueComparison(binding, 0, () -> inOperationCoercion(binding));
+ }
+ return super.inOperationCoercion(binding);
+ }
+
+ /// Returns whether the operand at the given index is a reference to a multi-value (ARRAY type) column.
+ private static boolean isMultiValueColumn(SqlCallBinding binding, int index) {
+ return SqlTypeUtil.isArray(binding.getOperandType(index)) && isColumnReference(binding.operand(index));
+ }
+
+ /// Returns whether all values of the `IN` list are non-array values, and throws on a `NULL` value: Calcite types it
+ /// as the array type of the column, and the leaf stage cannot evaluate it in a multi-value predicate.
+ private static boolean isScalarValueListRejectingNull(SqlCallBinding binding, SqlNodeList values) {
+ for (SqlNode value : values) {
+ if (SqlUtil.isNullLiteral(value, false)) {
+ throw QueryErrorCode.QUERY_VALIDATION.asException(
+ "NULL is not supported in an IN list compared to multi-value column: " + binding.operand(0));
Review Comment:
Small asymmetry worth a note: a NULL in an `IN` list errors here, but `mvCol = NULL` on the binary path is silently wrapped and pushed down as `col = NULL` (always no-match). Both are defensible, but one form erroring while the sibling form is accepted may surprise users. Consider a one-line comment explaining the difference, and maybe a `mvCol = NULL` test so the accepted behavior is pinned.
##########
pinot-query-runtime/src/test/resources/queries/MultiValueFilters.json:
##########
@@ -0,0 +1,370 @@
+{
+ "mv_filter": {
+ "comment": "Comparisons of a multi-value column to a value. As in the single-stage engine, = and IN match a row if any value matches, <> and NOT IN if no value matches. They are only supported in filters pushed down to the leaf stage.",
+ "tables": {
+ "orders": {
+ "schema": [
+ {
+ "name": "order_id",
+ "type": "STRING"
+ },
+ {
+ "name": "customer_id",
+ "type": "STRING"
+ },
+ {
+ "name": "tags",
+ "type": "STRING",
+ "isSingleValue": false
+ },
+ {
+ "name": "codes",
+ "type": "INT",
+ "isSingleValue": false
+ }
+ ],
+ "inputs": [
+ [
+ "o1",
+ "c1",
+ [
+ "urgent",
+ "vip"
+ ],
+ [
+ 1,
+ 2
+ ]
+ ],
+ [
+ "o2",
+ "c1",
+ [
+ "urgent"
+ ],
+ [
+ 2
+ ]
+ ],
+ [
+ "o3",
+ "c2",
+ [
+ "urgent"
+ ],
+ [
+ 3
+ ]
+ ],
+ [
+ "o4",
+ "c2",
+ [
+ "vip"
+ ],
+ [
+ 4,
+ 5
+ ]
+ ],
+ [
+ "o5",
+ "c3",
+ [
+ "normal"
+ ],
+ [
+ 5
+ ]
+ ]
+ ]
+ },
+ "customers": {
+ "schema": [
+ {
+ "name": "customer_id",
+ "type": "STRING"
+ },
+ {
+ "name": "name",
+ "type": "STRING"
+ }
+ ],
+ "inputs": [
+ [
+ "c1",
+ "Alice"
+ ],
+ [
+ "c2",
+ "Bob"
+ ],
+ [
+ "c3",
+ "Charlie"
+ ]
+ ]
+ }
+ },
+ "queries": [
+ {
+ "description": "Equality on a STRING MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE tags = 'urgent' ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o2"
+ ],
+ [
+ "o3"
+ ]
+ ]
+ },
+ {
+ "description": "Equality with the value on the left side",
+ "sql": "SELECT order_id FROM {orders} WHERE 'vip' = tags ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o4"
+ ]
+ ]
+ },
+ {
+ "description": "IN on a STRING MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE tags IN ('vip', 'normal') ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o4"
+ ],
+ [
+ "o5"
+ ]
+ ]
+ },
+ {
+ "description": "IN with a large value list on a STRING MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE tags IN ('v0', 'v1', 'v2', 'v3', 'v4', 'v5', 'v6', 'v7', 'v8', 'v9', 'v10', 'v11', 'v12', 'v13', 'v14', 'v15', 'v16', 'v17', 'v18', 'v19', 'v20', 'v21', 'v22', 'v23', 'v24', 'normal') ORDER BY order_id",
+ "outputs": [
+ [
+ "o5"
+ ]
+ ]
+ },
+ {
+ "description": "Two equalities on the same MV column are not folded to false",
+ "sql": "SELECT order_id FROM {orders} WHERE tags = 'urgent' AND tags = 'vip' ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ]
+ ]
+ },
+ {
+ "description": "Equality and IN on an INT MV column",
+ "sql": "SELECT order_id FROM {orders} WHERE codes = 5 OR codes IN (1, 3) ORDER BY order_id",
+ "outputs": [
+ [
+ "o1"
+ ],
+ [
+ "o3"
+ ],
+ [
+ "o4"
+ ],
+ [
+ "o5"
+ ]
+ ]
+ },
+ {
+ "description": "MV equality pushed below a join",
+ "sql": "SELECT o.order_id, c.name FROM {orders} o JOIN {customers} c ON o.customer_id = c.customer_id WHERE o.tags = 'urgent' ORDER BY o.order_id",
+ "outputs": [
+ [
+ "o1",
+ "Alice"
+ ],
+ [
+ "o2",
+ "Alice"
+ ],
+ [
+ "o3",
+ "Bob"
+ ]
+ ]
+ },
+ {
+ "description": "MV IN pushed below a join with an aggregate above it",
+ "sql": "SELECT c.name, COUNT(*) FROM {orders} o JOIN {customers} c ON o.customer_id = c.customer_id WHERE o.tags IN ('vip', 'normal') GROUP BY c.name ORDER BY c.name",
+ "outputs": [
+ [
+ "Alice",
+ 1
+ ],
+ [
+ "Bob",
+ 1
+ ],
+ [
+ "Charlie",
+ 1
+ ]
+ ]
+ },
+ {
+ "description": "Not-equals on an MV column matches rows with no equal value",
Review Comment:
Coverage gap (low): every input row has a non-empty, non-null MV. The `<>` / `NOT IN` "no value matches" complement is most error-prone on an empty or NULL MV row (where `= 'x'` is false and `<> 'x'` should be true). An extra row like `["o6","c3",[],[6]]` (or a null MV) extending the `<>` / `NOT IN` expected outputs would lock down that boundary — especially under null-handling-enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 closed pull request #20001: Support = and IN on multi-value columns in the multi-stage engine
URL: https://github.com/apache/pinot/pull/20001
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #20001:
URL: https://github.com/apache/pinot/pull/20001#discussion_r4344441868
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/validation/ArrayToMvValidationVisitor.java:
##########
@@ -172,6 +174,42 @@ public Void visitUnnest(UnnestNode node, Boolean isIntermediateStage) {
return null;
}
+ /// Comparing a multi-value column to a value (e.g. `mvCol = 'a'`) is planned as a comparison on `ARRAY_TO_MV`. As
+ /// the query does not contain `ARRAY_TO_MV` in this case, the error explains the comparison instead.
+ private QueryException arrayToMvNotSupported(String location, List<? extends RexExpression> expressions,
+ String hint) {
+ if (containsMultiValueComparison(expressions)) {
+ return new QueryException(QueryErrorCode.QUERY_PLANNING,
+ "Comparing a multi-value column to a value (e.g. mvCol = 'a' or mvCol IN ('a', 'b')) is not supported in "
+ + location + ". It is only supported in a WHERE clause filter that can be pushed down to the table "
+ + "scan. Elsewhere, use arrayContainsString(mvCol, 'a') or arrayContainsInt(mvCol, 1) instead");
Review Comment:
The hint hardcodes `arrayContainsString`/`arrayContainsInt`, but an MV column could be e.g. LONG/DOUBLE where neither applies verbatim. It's illustrative so this is fine, just flagging in case you want to soften it to "e.g. arrayContains...". Non-blocking.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/validate/PinotTypeCoercion.java:
##########
@@ -108,7 +134,64 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) {
return super.binaryComparisonCoercion(binding);
}
+ @Override
+ public boolean inOperationCoercion(SqlCallBinding binding) {
+ SqlKind kind = binding.getOperator().getKind();
+ if ((kind == SqlKind.IN || kind == SqlKind.NOT_IN) && binding.operand(1) instanceof SqlNodeList
+ && isMultiValueColumn(binding, 0) && isScalarValueList(binding, (SqlNodeList) binding.operand(1))) {
+ return coerceMultiValueComparison(binding, 0, () -> inOperationCoercion(binding));
+ }
+ return super.inOperationCoercion(binding);
+ }
+
+ /// Returns whether the operand at the given index is a reference to a multi-value (ARRAY type) column.
+ private static boolean isMultiValueColumn(SqlCallBinding binding, int index) {
+ return SqlTypeUtil.isArray(binding.getOperandType(index)) && isColumnReference(binding.operand(index));
+ }
+
+ /// Returns whether all values of the `IN` list are non-array values. A `NULL` value is rejected: Calcite types it
+ /// as the array type of the column, and the leaf stage cannot evaluate it in a multi-value predicate.
+ private static boolean isScalarValueList(SqlCallBinding binding, SqlNodeList values) {
Review Comment:
Tiny naming nit: `isScalarValueList` reads like a pure predicate but it also throws on a NULL in the list. Could be worth renaming to something like `validateScalarValueList` (or splitting the NULL check out) so the side effect isn't surprising to a future reader. Non-blocking.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/validate/PinotTypeCoercion.java:
##########
@@ -108,7 +134,64 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) {
return super.binaryComparisonCoercion(binding);
}
+ @Override
+ public boolean inOperationCoercion(SqlCallBinding binding) {
+ SqlKind kind = binding.getOperator().getKind();
+ if ((kind == SqlKind.IN || kind == SqlKind.NOT_IN) && binding.operand(1) instanceof SqlNodeList
+ && isMultiValueColumn(binding, 0) && isScalarValueList(binding, (SqlNodeList) binding.operand(1))) {
+ return coerceMultiValueComparison(binding, 0, () -> inOperationCoercion(binding));
+ }
+ return super.inOperationCoercion(binding);
+ }
+
+ /// Returns whether the operand at the given index is a reference to a multi-value (ARRAY type) column.
+ private static boolean isMultiValueColumn(SqlCallBinding binding, int index) {
+ return SqlTypeUtil.isArray(binding.getOperandType(index)) && isColumnReference(binding.operand(index));
+ }
+
+ /// Returns whether all values of the `IN` list are non-array values. A `NULL` value is rejected: Calcite types it
+ /// as the array type of the column, and the leaf stage cannot evaluate it in a multi-value predicate.
+ private static boolean isScalarValueList(SqlCallBinding binding, SqlNodeList values) {
Review Comment:
Tiny naming nit: `isScalarValueList` reads like a pure predicate but it also throws on a NULL in the list. Could be worth renaming to something like `validateScalarValueList` (or splitting the NULL check out) so the side effect isn't surprising to a future reader. Non-blocking.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/validation/ArrayToMvValidationVisitor.java:
##########
@@ -172,6 +174,42 @@ public Void visitUnnest(UnnestNode node, Boolean isIntermediateStage) {
return null;
}
+ /// Comparing a multi-value column to a value (e.g. `mvCol = 'a'`) is planned as a comparison on `ARRAY_TO_MV`. As
+ /// the query does not contain `ARRAY_TO_MV` in this case, the error explains the comparison instead.
+ private QueryException arrayToMvNotSupported(String location, List<? extends RexExpression> expressions,
+ String hint) {
+ if (containsMultiValueComparison(expressions)) {
+ return new QueryException(QueryErrorCode.QUERY_PLANNING,
+ "Comparing a multi-value column to a value (e.g. mvCol = 'a' or mvCol IN ('a', 'b')) is not supported in "
+ + location + ". It is only supported in a WHERE clause filter that can be pushed down to the table "
+ + "scan. Elsewhere, use arrayContainsString(mvCol, 'a') or arrayContainsInt(mvCol, 1) instead");
Review Comment:
The hint hardcodes `arrayContainsString`/`arrayContainsInt`, but an MV column could be e.g. LONG/DOUBLE where neither applies verbatim. It's illustrative so this is fine, just flagging in case you want to soften it to "e.g. arrayContains...". Non-blocking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Support = and IN on multi-value columns in the multi-stage engine [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #20001:
URL: https://github.com/apache/pinot/pull/20001#discussion_r4344442369
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/validate/PinotTypeCoercion.java:
##########
@@ -108,7 +134,64 @@ public boolean binaryComparisonCoercion(SqlCallBinding binding) {
return super.binaryComparisonCoercion(binding);
}
+ @Override
+ public boolean inOperationCoercion(SqlCallBinding binding) {
+ SqlKind kind = binding.getOperator().getKind();
+ if ((kind == SqlKind.IN || kind == SqlKind.NOT_IN) && binding.operand(1) instanceof SqlNodeList
+ && isMultiValueColumn(binding, 0) && isScalarValueList(binding, (SqlNodeList) binding.operand(1))) {
+ return coerceMultiValueComparison(binding, 0, () -> inOperationCoercion(binding));
+ }
+ return super.inOperationCoercion(binding);
+ }
+
+ /// Returns whether the operand at the given index is a reference to a multi-value (ARRAY type) column.
+ private static boolean isMultiValueColumn(SqlCallBinding binding, int index) {
+ return SqlTypeUtil.isArray(binding.getOperandType(index)) && isColumnReference(binding.operand(index));
+ }
+
+ /// Returns whether all values of the `IN` list are non-array values. A `NULL` value is rejected: Calcite types it
+ /// as the array type of the column, and the leaf stage cannot evaluate it in a multi-value predicate.
+ private static boolean isScalarValueList(SqlCallBinding binding, SqlNodeList values) {
Review Comment:
Tiny naming nit: `isScalarValueList` reads like a pure predicate but it also throws on a NULL in the list. Could be worth renaming to something like `validateScalarValueList` (or splitting the NULL check out) so the side effect isn't surprising to a future reader. Non-blocking.
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/planner/validation/ArrayToMvValidationVisitor.java:
##########
@@ -172,6 +174,42 @@ public Void visitUnnest(UnnestNode node, Boolean isIntermediateStage) {
return null;
}
+ /// Comparing a multi-value column to a value (e.g. `mvCol = 'a'`) is planned as a comparison on `ARRAY_TO_MV`. As
+ /// the query does not contain `ARRAY_TO_MV` in this case, the error explains the comparison instead.
+ private QueryException arrayToMvNotSupported(String location, List<? extends RexExpression> expressions,
+ String hint) {
+ if (containsMultiValueComparison(expressions)) {
+ return new QueryException(QueryErrorCode.QUERY_PLANNING,
+ "Comparing a multi-value column to a value (e.g. mvCol = 'a' or mvCol IN ('a', 'b')) is not supported in "
+ + location + ". It is only supported in a WHERE clause filter that can be pushed down to the table "
+ + "scan. Elsewhere, use arrayContainsString(mvCol, 'a') or arrayContainsInt(mvCol, 1) instead");
Review Comment:
The hint hardcodes `arrayContainsString`/`arrayContainsInt`, but an MV column could be e.g. LONG/DOUBLE where neither applies verbatim. It's illustrative so this is fine, just flagging in case you want to soften it to "e.g. arrayContains...". Non-blocking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org