yashmayya commented on code in PR #13425:
URL: https://github.com/apache/pinot/pull/13425#discussion_r1648424934
##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java:
##########
@@ -399,6 +400,8 @@ private static DataType getDataType(Expression expression,
Schema schema) {
dataType = DataType.INT;
} else if ("VARCHAR".equals(targetTypeLiteral)) {
dataType = DataType.STRING;
+ } else if (targetTypeLiteral.endsWith("_ARRAY")) {
Review Comment:
Hmm but before this fix, v1 didn't support `ARRAY` keyword in cast function
at all - MV fields were cast the exact same way as SV fields. So `CAST(mv_col
AS INT)` and `CAST(mv_col AS INTEGER)` both work whereas none of `CAST(mv_col
AS INT ARRAY)` / `CAST(mv_col AS INTEGER ARRAY)` / `CAST(mv_col AS INT_ARRAY)`
/ `CAST(mv_col AS INTEGER_ARRAY)` work (all result in the error: `No enum
constant org.apache.pinot.spi.data.FieldSpec.DataType.XYZ`).
And in the v2 query engine, before this fix, there was no way to cast MV
columns to a numeric type that works. After this fix, `CAST(mv_col AS INT
ARRAY)` / `CAST(mv_col AS INTEGER ARRAY)` both work but `CAST(mv_col AS
INT_ARRAY)` / `CAST(mv_col AS INTEGER_ARRAY)` don't work since `INT_ARRAY` /
`INTEGER_ARRAY` aren't recognized by Calcite. Inadvertently, this fix also
makes `CAST(mv_col AS INT_ARRAY)` work in the v1 query engine, but as you
pointed out `CAST(mv_col as INTEGER_ARRAY)` won't work. Note that even
`CAST(mv_col AS INT ARRAY)` won't work in the v1 query engine (with or without
this fix).
This is fairly messy but I think the question boils down to whether we want
to support `ARRAY` keyword for MV columns in the cast function in the v1 query
engine and whether we're okay with introducing an additional difference between
the two query engines (`INT_ARRAY` in v1 vs `INT ARRAY` in v2)?
##########
pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java:
##########
@@ -399,6 +400,8 @@ private static DataType getDataType(Expression expression,
Schema schema) {
dataType = DataType.INT;
} else if ("VARCHAR".equals(targetTypeLiteral)) {
dataType = DataType.STRING;
+ } else if (targetTypeLiteral.endsWith("_ARRAY")) {
Review Comment:
Ah never mind but the issue in v1 was due to this
[NumericalFilterOptimizer](https://github.com/apache/pinot/blob/99f6934166633308547d37cacc634e03c723ab17/pinot-core/src/main/java/org/apache/pinot/core/query/optimizer/filter/NumericalFilterOptimizer.java#L385)
itself so things like `CAST(mv_col AS STRING_ARRAY)` did work in v1 before
this fix. I think it makes sense to support `INTEGER_ARRAY` as well as
`INT_ARRAY` in v1 👍
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
