Skip to content

Commit

Permalink
fix filtering bug in filtering unnest cols and dim cols
Browse files Browse the repository at this point in the history
  • Loading branch information
@pranavbhole
pranavbhole committed Jul 18, 2023
1 parent 0dcb19f commit 7d9518e
Show file tree
Hide file tree
Showing 3 changed files with 325 additions and 25 deletions.
84 changes: 59 additions & 25 deletions processing/src/main/java/org/apache/druid/segment/UnnestStorageAdapter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.apache.druid.segment.column.ValueType;
import org.apache.druid.segment.data.Indexed;
import org.apache.druid.segment.data.ListIndexed;
import org.apache.druid.segment.filter.AndFilter;
import org.apache.druid.segment.filter.BoundFilter;
import org.apache.druid.segment.filter.Filters;
import org.apache.druid.segment.filter.LikeFilter;
Expand Down Expand Up @@ -362,31 +363,19 @@ void addPreFilter(@Nullable final Filter filter)
final FilterSplitter filterSplitter = new FilterSplitter();

if (queryFilter != null) {
List<Filter> preFilterList = new ArrayList<>();
final int origFilterSize;
if (queryFilter.getRequiredColumns().contains(outputColumnName)) {
// outside filter contains unnested column
// requires check for OR
if (queryFilter instanceof OrFilter) {
origFilterSize = ((OrFilter) queryFilter).getFilters().size();
for (Filter filter : ((OrFilter) queryFilter).getFilters()) {
if (filter.getRequiredColumns().contains(outputColumnName)) {
final Filter newFilter = rewriteFilterOnUnnestColumnIfPossible(
filter,
inputColumn,
inputColumnCapabilites
);
if (newFilter != null) {
preFilterList.add(newFilter);
}
} else {
preFilterList.add(filter);
}
}
if (preFilterList.size() == origFilterSize) {
// there has been successful rewrites
final OrFilter preOrFilter = new OrFilter(preFilterList);
filterSplitter.addPreFilter(preOrFilter);
// requires check for OR and And filters, disqualify rewrite for non-unnest filters
if (queryFilter instanceof BooleanFilter) {
List<Filter> preFilterList = recursiveRewriteOnUnnestFilters(
(BooleanFilter) queryFilter,
inputColumn,
inputColumnCapabilites
);
if (queryFilter instanceof AndFilter) {
filterSplitter.addPreFilter(new AndFilter(preFilterList));
} else if (queryFilter instanceof OrFilter) {
filterSplitter.addPreFilter(new OrFilter(preFilterList));
}
// add the entire query filter to unnest filter to be used in Value matcher
filterSplitter.addPostFilterWithPreFilterIfRewritePossible(queryFilter, true);
Expand All @@ -408,7 +397,53 @@ void addPreFilter(@Nullable final Filter filter)
);
}


/**
* handles the nested rewrite for unnest columns in recursive way,
* it loops through all and/or filters and rewrite only required filters in the child and skip others
*
* @param queryFilter query filter passed to makeCursors
* @param inputColumn input column to unnest if it's a direct access; otherwise null
* @param inputColumnCapabilites input column capabilities if known; otherwise null
*/
private List<Filter> recursiveRewriteOnUnnestFilters(
BooleanFilter queryFilter,
final String inputColumn,
final ColumnCapabilities inputColumnCapabilites
)
{
final List<Filter> preFilterList = new ArrayList<>();
for (Filter filter : queryFilter.getFilters()) {
if (filter.getRequiredColumns().contains(outputColumnName)) {
if (filter instanceof AndFilter) {
preFilterList.add(new AndFilter(recursiveRewriteOnUnnestFilters(
(BooleanFilter) filter,
inputColumn,
inputColumnCapabilites
)));
} else if (filter instanceof OrFilter) {
preFilterList.add(new OrFilter(recursiveRewriteOnUnnestFilters(
(BooleanFilter) filter,
inputColumn,
inputColumnCapabilites
)));
} else {
final Filter newFilter = rewriteFilterOnUnnestColumnIfPossible(
filter,
inputColumn,
inputColumnCapabilites
);
if (newFilter != null) {
preFilterList.add(newFilter);
} else {
preFilterList.add(filter);
}
}
} else {
preFilterList.add(filter);
}
}
return preFilterList;
}
/**
* Returns the input of {@link #unnestColumn}, if it's a direct access; otherwise returns null.
*/
Expand Down Expand Up @@ -460,7 +495,6 @@ private static boolean filterMapsOverMultiValueStrings(final Filter filter)
return false;
}
}

return true;
} else if (filter instanceof NotFilter) {
return filterMapsOverMultiValueStrings(((NotFilter) filter).getBaseFilter());
Expand Down
50 changes: 50 additions & 0 deletions processing/src/test/java/org/apache/druid/segment/UnnestStorageAdapterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.apache.druid.query.filter.SelectorDimFilter;
import org.apache.druid.segment.column.ColumnCapabilities;
import org.apache.druid.segment.column.ValueType;
import org.apache.druid.segment.filter.AndFilter;
import org.apache.druid.segment.filter.OrFilter;
import org.apache.druid.segment.generator.GeneratorBasicSchemas;
import org.apache.druid.segment.generator.GeneratorSchemaInfo;
Expand Down Expand Up @@ -300,7 +301,56 @@ public void test_pushdown_or_filters_unnested_and_original_dimension_with_unnest
return null;
});
}
@Test
public void test_nested_filters_unnested_and_original_dimension_with_unnest_adapters()
{
final UnnestStorageAdapter unnestStorageAdapter = new UnnestStorageAdapter(
new TestStorageAdapter(INCREMENTAL_INDEX),
new ExpressionVirtualColumn(OUTPUT_COLUMN_NAME, "\"" + COLUMNNAME + "\"", null, ExprMacroTable.nil()),
null
);

final VirtualColumn vc = unnestStorageAdapter.getUnnestColumn();

final String inputColumn = unnestStorageAdapter.getUnnestInputIfDirectAccess(vc);

final OrFilter baseFilter = new OrFilter(ImmutableList.of(
new SelectorDimFilter(OUTPUT_COLUMN_NAME, "1", null).toFilter(),
new AndFilter(ImmutableList.of(
new SelectorDimFilter(inputColumn, "2", null).toFilter(),
new SelectorDimFilter(OUTPUT_COLUMN_NAME, "10", null).toFilter()
))
));

final OrFilter expectedPushDownFilter = new OrFilter(ImmutableList.of(
new SelectorDimFilter(inputColumn, "1", null).toFilter(),
new AndFilter(ImmutableList.of(
new SelectorDimFilter(inputColumn, "2", null).toFilter(),
new SelectorDimFilter(inputColumn, "10", null).toFilter()
))
));

final Sequence<Cursor> cursorSequence = unnestStorageAdapter.makeCursors(
baseFilter,
unnestStorageAdapter.getInterval(),
VirtualColumns.EMPTY,
Granularities.ALL,
false,
null
);

final TestStorageAdapter base = (TestStorageAdapter) unnestStorageAdapter.getBaseAdapter();
final Filter pushDownFilter = base.getPushDownFilter();

Assert.assertEquals(expectedPushDownFilter, pushDownFilter);
cursorSequence.accumulate(null, (accumulated, cursor) -> {
Assert.assertEquals(cursor.getClass(), PostJoinCursor.class);
final Filter postFilter = ((PostJoinCursor) cursor).getPostJoinFilter();
// OR-case so base filter should match the postJoinFilter
Assert.assertEquals(baseFilter, postFilter);
return null;
});
}
@Test
public void test_pushdown_filters_unnested_dimension_with_unnest_adapters()
{
Expand Down
Loading

0 comments on commit 7d9518e

Please sign in to comment.