add support for filtered projections by clintropolis · Pull Request #18342 · apache/druid

clintropolis · 2025-07-30T10:23:10Z

Description

This PR adds a filter property to AggregateProjectionSpec to allow projections to be pre-filtered to further reduce row counts for views to power queries with commonly used filters. The filter is built into a ValueMatcher in OnHeapAggregateProjection and rows are checked to match before adding to the facts table. At query time, if a projection has a filter we check to see that the query either contains the exact filter, or is part of an AND filter to ensure that the projection contains all of the rows which would normally match if querying the base table instead. If the query filter matches, the projection filter is removed from the query since it is no longer necessary.

The filter matching is pretty brittle right now and requires an exact match, but this can be improved in follow-up work.

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
a release note entry in the PR description.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
added integration tests.
been tested in a test Druid cluster.

gianm · 2025-07-31T07:18:46Z

@@ -553,6 +555,7 @@ public void testInsertOnExternalDataSourceWithCatalogProjections(String contextN
        new AggregateProjectionMetadata.Schema(


add a builder to avoid the need for null filter everywhere?

gianm · 2025-07-31T07:23:31Z

      }
+
+      // if the projection has a filter, the query must contain this filter match
+      if (filter != null) {


IMO, put this logic and rewriteFilter in their own dedicated file. The logic will only grow more complex over time.

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 🤷

ok, locally i've moved most of the logic into Projections (and moved the ConstantTimeColumn stuff in Projections to its own file), and it seems nicer so far, more to do tho so havent pushed yet

gianm · 2025-07-31T07:55:44Z

+      }
+      // at least one child must have been rewritten to rewrite the AND
+      if (childRewritten) {
+        if (newChildren.size() > 1) {


can newChildren be empty, if all filters under the AND of queryFilter were part of projectionFilter?

i think not with the current rather weak equals based matching logic - either the parent AND would match a projection AND, or at most a single child of this AND can match. Doing an overlap of AND children would be an improvement, and run into the problem you're asking about I think. I'll see if I can wire that up tho since it seems important for a query with something like x = 'a' and y = 'b' and z = 'c' to match a projection with x = 'a' and y = 'b'

gianm · 2025-07-31T08:01:03Z

+  @Nullable
+  public static Filter rewriteFilter(@Nullable Filter projectionFilter, @Nullable Filter queryFilter)
+  {
+    if (projectionFilter == null || queryFilter == null) {


can queryFilter really be null? It seems like this method is only called for nonnull query filters.

oops, not anymore

cecemei · 2025-08-02T06:33:24Z

      this.timeColumnName = timeColumnName;
      this.timeColumnPosition = foundTimePosition;
-      this.granularity = granularity == null ? Granularities.ALL : granularity;
+      this.effectiveGranularity = granularity == null ? Granularities.ALL : granularity;


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.

also it seems like this effectiveGranularity and orderingWithTimeSubstitution logic is very similar to the computeOrdering function, maybe it could be merged?

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

cecemei · 2025-08-04T05:28:10Z

    }
    AggregateProjectionSpec that = (AggregateProjectionSpec) o;
    return Objects.equals(name, that.name)
+           && Objects.equals(filter, that.filter)


Is it intentional timeColumnName is not included in equals/ hashCode/ toString method, but ordering is? Both are derived so seems like not needed.

this is sort of aspirational - I think it would be possible someday for ordering to not be fixed to grouping if we ever open up the comparator used to sort data when building segments. However, no such plans for the time column.

(however the other part you noticed about relation between grouping and ordering maybe sort of breaks this)

gianm

Approved, with some thoughts on the tests.

gianm · 2025-08-07T08:40:22Z

+import java.util.Map;
+import java.util.Set;
+
+class ProjectionsTest


public class? not sure it matters, just odd not to see the keyword.

used intellij to generate a test file and saw it without the public, i looked into it and apparently in junit5 it is not necessary so have been rolling with it 🤷 https://docs.junit.org/current/user-guide/#writing-tests-classes-and-methods

gianm · 2025-08-07T08:41:38Z

+  }
+
+  @Test
+  void testSchemaMatchFilter()


We're going to need easier, more concise ways to write tests for filter matching. As the filter matching gets more sophisticated, there will be a lot of variations to test to ensure everything is working properly.

Maybe we can define a few "standard" datasources for the tests, each one with a base table and some projection(s). Then each test would just need to define a CursorBuildSpec and the expected ProjectionMatch.

ah yea, i was writing these tests as mainly synthetic to smoke test stuff as they are not backed by real data. I think for the bulk of tests I would agree we want something like what CursorFactoryProjectionTest is doing with a matrix of incremental and immutable segments but with a more cursor + filter focus and make it test that results are the same with and without the projection similar to what the native filter tests are doing

gianm · 2025-08-07T08:52:02Z

+        cursorBuildSpec,
+        new RowSignatureChecker(baseTable)
+    );
+    Projections.ProjectionMatch expected = new Projections.ProjectionMatch(


nit- moving ProjectionMatch out into its own class would help a little with making the test code more concise.

add support for filtered projections

58a4cbd

github-actions Bot added Area - Batch Ingestion Area - Segment Format and Ser/De Area - Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jul 30, 2025

clintropolis added 2 commits July 30, 2025 03:44

fix style

b47b5e5

remove mistake

280c8ce

gianm reviewed Jul 31, 2025

View reviewed changes

cecemei reviewed Aug 4, 2025

View reviewed changes

clintropolis added 6 commits August 4, 2025 12:45

move match logic into Projections instead of living in Schema, WIP

b591181

Merge remote-tracking branch 'upstream/master' into filtered-projections

4d4a748

move tests

af66e88

builders, improvements, fixes, etc

dcf8e2f

cleanup

2efc1a2

cleanup

ba255d9

gianm approved these changes Aug 7, 2025

View reviewed changes

move inner classes

7a78c7b

clintropolis merged commit fe57bc3 into apache:master Aug 7, 2025
95 of 96 checks passed

clintropolis deleted the filtered-projections branch August 7, 2025 18:16

cecemei added this to the 35.0.0 milestone Oct 21, 2025

		@@ -553,6 +555,7 @@ public void testInsertOnExternalDataSourceWithCatalogProjections(String contextN
		new AggregateProjectionMetadata.Schema(

Conversation

clintropolis commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clintropolis Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gianm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clintropolis commented Jul 30, 2025 •

edited

Loading

clintropolis Aug 7, 2025 •

edited

Loading