cryptoe commented on a change in pull request #12078:
URL: https://github.com/apache/druid/pull/12078#discussion_r780624974
##########
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:
> As part of this patch, then, let's check all the MV functions to make sure
the return types are what we expect them to be.
Looks like all mv functions are overriding the output signature of the
corresponding array functions to return a VARCHAR instead of an array
>
> As to the array stuff, one of them is documented: ARRAY_AGG. And you could
do that in a subquery and group on the result. Some questions about that:
>
> * Do we have tests for the case of grouping on the result of an
ARRAY_AGG? If not we should add it.
Were you looking for
[this](https://github.com/apache/druid/pull/12078/files#diff-8bc53aec44924e671b3c6f6f34c6f0c499f873d9000649c47c237444707aea4bL1640)
test case ?
>
> * Does the behavior change or does this introduce excess risk due to
potential bugs? If so, a feature flag would be a good idea.
Still thinking about this
--
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]
