Copilot commented on code in PR #19101:
URL: https://github.com/apache/pinot/pull/19101#discussion_r3686893867
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java:
##########
@@ -83,6 +83,13 @@ public MultistageGroupByExecutor(int[] groupKeyIds,
AggregationFunction[] aggFun
_aggType = aggType;
_leafReturnFinalResult = leafReturnFinalResult;
_resultSchema = resultSchema;
+ for (int i = 0; i < groupKeyIds.length; i++) {
+ ColumnDataType dataType = resultSchema.getColumnDataType(i);
+ if (!dataType.supportsEquality() || !dataType.supportsHashing()) {
+ throw new IllegalArgumentException(
+ "Raw VARIANT values do not support GROUP BY; extract a typed path
with variantGet first");
+ }
+ }
Review Comment:
The raw-VARIANT GROUP BY validation is checking the first N columns in the
schema (`getColumnDataType(i)`), but the executor later uses `_groupKeyIds` to
select the actual group-by key columns from each row. If the group keys are not
exactly `[0..N-1]`, this can (a) miss rejecting a VARIANT group key, or (b)
incorrectly reject a query where a non-key column is VARIANT.
Validate the data types at the actual key indices instead of assuming keys
occupy the first columns.
--
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]
