clintropolis commented on a change in pull request #12078:
URL: https://github.com/apache/druid/pull/12078#discussion_r780203919
##########
File path: sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
##########
@@ -128,27 +128,10 @@ public static String escapeStringLiteral(final String s)
}
/**
- * Convert {@link RelDataType} to the most appropriate {@link ValueType},
coercing all ARRAY types to STRING (until
- * the time is right and we are more comfortable handling Druid ARRAY types
in all parts of the engine).
- *
- * Callers who are not scared of ARRAY types should isntead call {@link
#getValueTypeForRelDataTypeFull(RelDataType)},
- * which returns the most accurate conversion of {@link RelDataType} to
{@link ValueType}.
+ * Convert {@link RelDataType} to the most appropriate {@link ValueType}}.
*/
@Nullable
public static ColumnType getColumnTypeForRelDataType(final RelDataType type)
Review comment:
the engine has been waiting on array grouping for this coercion to go
away, though there is a missing piece I think where we need to ensure that when
grouping on a virtual column we never plan to a topn query in `DruidQuery`.
I think most of the `MV_` prefixed functions have their return type as
`VARCHAR` rather than `ARRAY`, so in theory the "documented" multi-value string
functions shouldn't actually be affected, and any that will be is a bug that we
should fix. The undocumented `ARRAY_` prefixed functions will now behave as
intended and validate/operate as true SQL arrays (and so group correctly as
arrays). If anyone was using the undocumented `ARRAY_` prefix functions, then I
guess that behavior will change, since grouping on those expressions would
previously have been grouping as `STRING` due to the implicit unnest.
The only reason I can think of to add a feature flag is if need a way to
hide bugs that this change causes, and that _should_ only happen if any of the
`MV_` functions have bugs in their SQL output types, or if people were using
the undocumented `ARRAY_` functions.
--
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]
