clintropolis commented on code in PR #15451:
URL: https://github.com/apache/druid/pull/15451#discussion_r1409972331
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java:
##########
@@ -149,6 +153,42 @@ public DimFilter toDruidFilter(
);
}
}
+
+ // if the input is a direct array column, we can use sweet array filter
+ if (simpleExtractionExpr.isDirectColumnAccess() &&
simpleExtractionExpr.isArray()) {
+ // To convert this expression filter into an OR of ArrayContainsElement
filters, we need to extract all array
+ // elements.
+ if (expr.isLiteral()) {
Review Comment:
hmm, so i guess it will fail a precondition check when it makes the OR
filter because fields will be empty...
However, it currently isn't really possible to make an empty array literal
because `ARRAY[]` isn't accepted by calcite (or the native druid `array`
expression), so I'm not really sure how important it is to handle this case
either.
I can update this (and `ArrayContainsOperatorConversion`) to return `null`
in this case since we don't really handle it, or true i guess would also
probably be correct?
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ArrayOverlapOperatorConversion.java:
##########
@@ -149,6 +153,42 @@ public DimFilter toDruidFilter(
);
}
}
+
+ // if the input is a direct array column, we can use sweet array filter
Review Comment:
heh, i just copied this section from `ArrayContainsOperatorConversion` and
switched `AND` to `OR`
--
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]
