clintropolis commented on code in PR #18342:
URL: https://github.com/apache/druid/pull/18342#discussion_r2260910252
##########
processing/src/main/java/org/apache/druid/segment/AggregateProjectionMetadata.java:
##########
@@ -219,7 +233,7 @@ public Schema(
}
this.timeColumnName = timeColumnName;
this.timeColumnPosition = foundTimePosition;
- this.granularity = granularity == null ? Granularities.ALL : granularity;
+ this.effectiveGranularity = granularity == null ? Granularities.ALL :
granularity;
Review Comment:
>i think we're expecting the same size/ ordering bwteen groupingColumns and
OrderBy between line 223 and 224, this assumption is guaranteed from
ProjectionsSpec but not obvious here, maybe worth adding some checks.
yea, good spot, it is true they unfortunately are coupled right now, though
in the future I think i would like to make them more independent rather than
consolidate to keep us free to potentially allow user choice in ordering in the
future. Given that this is not new in this PR I would rather deal with this in
a follow-up
--
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]
