cryptoe commented on code in PR #16068:
URL: https://github.com/apache/druid/pull/16068#discussion_r1553254590


##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java:
##########
@@ -1250,8 +1249,8 @@ private TopNQuery toTopNQuery()
     }
 
     final DimensionSpec dimensionSpec = 
Iterables.getOnlyElement(grouping.getDimensions()).toDimensionSpec();
-    // grouping col cannot be type array
-    if (dimensionSpec.getOutputType().isArray()) {
+    // grouping col cannot be arrays or complex types

Review Comment:
   i think we should also mention that grouping complex col is not supported 
for topn query. 
   
   



##########
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidQuery.java:
##########
@@ -486,15 +484,16 @@ private static List<DimensionExpression> 
computeDimensions(
 
       final RelDataType dataType = rexNode.getType();
       final ColumnType outputType = 
Calcites.getColumnTypeForRelDataType(dataType);
-      if (Types.isNullOr(outputType, ValueType.COMPLEX)) {
-        // Can't group on unknown or COMPLEX types.
-        plannerContext.setPlanningError(
-            "SQL requires a group-by on a column of type %s that is 
unsupported.",
-            outputType
-        );
+      if (outputType == null) {
+        // Can't group on unknown types.
+        plannerContext.setPlanningError("SQL requires a group-by on a column 
with unknown type that is unsupported.");
+        throw new CannotBuildQueryException(aggregate, rexNode);
+      }
+      if (!outputType.getNullableStrategy().groupable()) {
+        // Can't group on 'ungroupable' types.
+        plannerContext.setPlanningError("SQL requires a group-by column with 
ungroupable type [%s].", outputType);

Review Comment:
   This should be SQL cannot group by a ungroupable column.



-- 
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]

Reply via email to