cryptoe commented on a change in pull request #12210:
URL: https://github.com/apache/druid/pull/12210#discussion_r794995068
##########
File path:
processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java
##########
@@ -256,30 +257,43 @@ public static boolean canMapOverDictionary(
final List<String> columns = plan.getAnalysis().getRequiredBindingsList();
final Map<String, Pair<ExpressionType, Supplier<Object>>> suppliers = new
HashMap<>();
for (String columnName : columns) {
- final ColumnCapabilities columnCapabilities =
columnSelectorFactory.getColumnCapabilities(columnName);
- final boolean multiVal = columnCapabilities != null &&
columnCapabilities.hasMultipleValues().isTrue();
+ final ColumnCapabilities capabilities =
columnSelectorFactory.getColumnCapabilities(columnName);
+ final boolean multiVal = capabilities != null &&
capabilities.hasMultipleValues().isTrue();
final Supplier<Object> supplier;
- final ExpressionType expressionType =
ExpressionType.fromColumnType(columnCapabilities);
-
- if (columnCapabilities == null ||
- columnCapabilities.isArray() ||
- (plan.is(ExpressionPlan.Trait.NON_SCALAR_OUTPUT) &&
!plan.is(ExpressionPlan.Trait.NEEDS_APPLIED))
- ) {
- // Unknown ValueType or array type. Try making an Object selector and
see if that gives us anything useful.
+ final ExpressionType expressionType =
ExpressionType.fromColumnType(capabilities);
+
+ final boolean useObjectSupplierForMultiValueStringArray =
+ capabilities != null
+ // if homogenizing null multi-value string arrays, or if a single
valued function that must be applied across
+ // multi-value rows, we can just use the dimension selector, which
has the homogenization behavior built-in
+ && ((!capabilities.is(ValueType.STRING))
+ || (capabilities.is(ValueType.STRING)
+ &&
!ExpressionProcessing.isHomogenizeNullMultiValueStringArrays()
+ && !plan.is(ExpressionPlan.Trait.NEEDS_APPLIED)
+ )
+ )
+ // expression has array output
+ && plan.is(ExpressionPlan.Trait.NON_SCALAR_OUTPUT);
+
+ final boolean homogenizeNullMultiValueStringArrays =
Review comment:
Nit: Maybe move this inside the if statement at 281?
--
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]
