clintropolis commented on a change in pull request #12078:
URL: https://github.com/apache/druid/pull/12078#discussion_r788384963
##########
File path:
core/src/main/java/org/apache/druid/math/expr/ExpressionProcessingConfig.java
##########
@@ -28,17 +28,22 @@
{
public static final String NESTED_ARRAYS_CONFIG_STRING =
"druid.expressions.allowNestedArrays";
public static final String NULL_HANDLING_LEGACY_LOGICAL_OPS_STRING =
"druid.expressions.useStrictBooleans";
+ public static final String CAST_ARRAY_TO_STRING_CONFIG_STRING =
"druid.expressions.allowArrayToStringCast";
@JsonProperty("allowNestedArrays")
private final boolean allowNestedArrays;
@JsonProperty("useStrictBooleans")
private final boolean useStrictBooleans;
+ @JsonProperty("allowArrayToStringCast")
+ private final boolean allowArrayToStringCast;
+
@JsonCreator
public ExpressionProcessingConfig(
@JsonProperty("allowNestedArrays") @Nullable Boolean allowNestedArrays,
- @JsonProperty("useStrictBooleans") @Nullable Boolean useStrictBooleans
+ @JsonProperty("useStrictBooleans") @Nullable Boolean useStrictBooleans,
+ @JsonProperty("allowArrayToStringCast") @Nullable Boolean
allowArrayToStringCast
Review comment:
this name seems too positive, like something someone might want to
enable to allow permissive casting of stuff.. how about
`processArraysAsMultiValueStrings`?
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/ArraySqlAggregator.java
##########
@@ -114,7 +113,7 @@ public Aggregation toDruidAggregation(
final String fieldName;
final String initialvalue;
- final ColumnType druidType =
Calcites.getValueTypeForRelDataTypeFull(aggregateCall.getType());
+ final ColumnType druidType =
Calcites.getColumnTypeForRelDataType(aggregateCall.getType());
Review comment:
we should maybe still call the old version of this method so that
behavior doesn't change when the flag is set to the old behavior (which would
change the behavior of this method)
##########
File path:
processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/GroupByQueryEngineV2.java
##########
@@ -403,6 +413,20 @@ public GroupByColumnSelectorStrategy
makeColumnSelectorStrategy(
return makeNullableNumericStrategy(new
FloatGroupByColumnSelectorStrategy());
case DOUBLE:
return makeNullableNumericStrategy(new
DoubleGroupByColumnSelectorStrategy());
+ case ARRAY:
+ switch (capabilities.getElementType().getType()) {
+ case LONG:
+ return new ArrayLongGroupByColumnSelectorStrategy();
+ case STRING:
+ return new ArrayStringGroupByColumnSelectorStrategy();
+ case DOUBLE:
+ return new ArrayDoubleGroupByColumnSelectorStrategy();
+ case FLOAT:
+ // Array<Float> not supported in expressions, ingestion
Review comment:
note to self, double check this is actually true at this layer (if it is
not, it might be possibly handled with the Double strategy). While definitely
true that `FLOAT` doesn't exist in expressions, and so within expressions there
exists no float array, this type might still be specified by the SQL planner,
whenever some float column is added into an array for example. I'm unsure if
the expression selectors column capabilities would report ARRAY<FLOAT> or
ARRAY<DOUBLE> as the type of the virtual column, i know it coerces DOUBLE back
to FLOAT when the planner requests FLOAT types, but don't think it does the
same thing for `ARRAY` so, this is _probably_ true.
--
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]
