clintropolis commented on code in PR #18342:
URL: https://github.com/apache/druid/pull/18342#discussion_r2245009342
##########
processing/src/main/java/org/apache/druid/segment/AggregateProjectionMetadata.java:
##########
@@ -396,6 +417,45 @@ public Projections.ProjectionMatch matches(
}
}
}
+
+ // if the projection has a filter, the query must contain this filter
match
+ if (filter != null) {
Review Comment:
hmm, this whole method is giant so I started to split all the sub parts out
into separate functions to like do the grouping match, aggregator match, and
realized there is another place we are doing the query filter match, i think
they need to be consolidated to be able to support projections where the filter
inputs aren't actually stored in the projection as well as it would be more
efficient to look at filter stuff once. I think we would need to shift the
filter matching to happen last since it needs the remapped columns the be
populated
Will consider splitting into a separate file, though in that case should
almost all of the logic be moved out of this file into like `Projections` which
already has `findMatchingProjection` which is what calls this matches method?
That seems fine to me, just pass in the `AggregateProjectionMetadata.Schema` to
most of these methods. Or I guess i could make a utility file per match type,
like a `GroupingMatcher` and `FilterMatcher` and `AggregatorMatcher` :shrug:
--
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]
