Skip to content

Enable partial filter pushdown for conjunctive predicates in Iceberg datasets #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shishir-kuet wants to merge 1 commit into
base: master
Choose a base branch
from
+64 −4
Open

Enable partial filter pushdown for conjunctive predicates in Iceberg datasets #45

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 64 additions & 4 deletions .../org/apache/asterix/optimizer/rules/pushdown/processor/ColumnFilterPushdownProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,51 @@ protected boolean pushdownFilter(ScanDefineDescriptor scanDescriptor) throws Alg
protected void putFilterInformation(ScanDefineDescriptor scanDefineDescriptor, ILogicalExpression inlinedExpr)
throws AlgebricksException {
if (checkerVisitor.containsMultipleArrayPaths(paths.values())) {
// Cannot pushdown a filter with multiple unnest
// TODO allow rewindable column readers for filters
// TODO this is a bit conservative (maybe too conservative) as we can push part of expression down
return;
// Conservative fallback: attempt to push a subset of top-level AND operands
// that do not collectively introduce multiple array paths.
if (inlinedExpr instanceof AbstractFunctionCallExpression
&& BuiltinFunctions.AND.equals(((AbstractFunctionCallExpression) inlinedExpr).getFunctionIdentifier())) {
List<Mutable<ILogicalExpression>> args = ((AbstractFunctionCallExpression) inlinedExpr)
.getArguments();
List<Mutable<ILogicalExpression>> selectedArgs = new ArrayList<>();
List<ARecordType> selectedPaths = new ArrayList<>();

for (Mutable<ILogicalExpression> argRef : args) {
ILogicalExpression arg = argRef.getValue();
List<ARecordType> argPaths = collectPathsForExpression(arg);
// Test if adding this arg's paths would cause multiple array paths
checkerVisitor.beforeVisit();
List<ARecordType> merged = new ArrayList<>(selectedPaths);
merged.addAll(argPaths);
if (!checkerVisitor.containsMultipleArrayPaths(merged)) {
selectedArgs.add(new MutableObject<>(arg));
selectedPaths.addAll(argPaths);
}
Comment on lines +168 to +181
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subset-selection logic checks containsMultipleArrayPaths(merged) using only selectedPaths + argPaths, but it doesn't include any paths that may already have been pushed down earlier for this scan (i.e., scanDefineDescriptor.getFilterPaths() from prior putFilterInformation calls). Since paths is accumulated across multiple candidate filters during pushdownFilter(...), this can still select a conjunct that introduces a second array path when combined with previously accepted predicates, defeating the constraint you’re trying to preserve. Consider seeding selectedPaths (or the merged list) with the already-pushed paths from the descriptor before evaluating each new conjunct.

Copilot uses AI. Check for mistakes.
}

if (selectedArgs.isEmpty()) {
// nothing pushable
return;
}

// Build new inlined expression from selectedArgs
AbstractFunctionCallExpression newExpr;
if (selectedArgs.size() == 1) {
// single argument
ILogicalExpression single = selectedArgs.get(0).getValue();
putFilterInformation(scanDefineDescriptor, single);
return;
Comment on lines +192 to +195
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When selectedArgs.size() == 1, this calls putFilterInformation(scanDefineDescriptor, single) recursively while leaving paths unchanged. Since paths.values() still contains the full set of paths (including the ones that triggered containsMultipleArrayPaths), the recursive call will typically hit the same containsMultipleArrayPaths(...) branch and then return early (because single is not an AND), resulting in no predicate being pushed down. Instead, avoid recursion here: set inlinedExpr to the single selected predicate and also restrict paths to only the entries needed by that selected predicate (and re-check/reset the visitor state if needed).

Suggested change
// single argument
ILogicalExpression single = selectedArgs.get(0).getValue();
putFilterInformation(scanDefineDescriptor, single);
return;
// Single argument: avoid recursion. Use it directly as the inlined expression
// and restrict 'paths' to only those needed by this predicate.
ILogicalExpression single = selectedArgs.get(0).getValue();
inlinedExpr = single;
// Keep only entries whose ARecordType is among the selected paths
paths.entrySet().removeIf(e -> !selectedPaths.contains(e.getValue()));

Copilot uses AI. Check for mistakes.
} else {
IFunctionInfo fInfo = context.getMetadataProvider().lookupFunction(AlgebricksBuiltinFunctions.AND);
newExpr = new ScalarFunctionCallExpression(fInfo, selectedArgs);
inlinedExpr = newExpr;
}
} else {
// Cannot pushdown a filter with multiple unnest
// TODO allow rewindable column readers for filters
// TODO this is a bit conservative (maybe too conservative) as we can push part of expression down
return;
}
}

ILogicalExpression filterExpr = scanDefineDescriptor.getFilterExpression();
Expand All @@ -181,6 +222,25 @@ protected IExpectedSchemaNode getPathNode(AbstractFunctionCallExpression express
return expression.accept(exprToNodeVisitor, null);
}

private List<ARecordType> collectPathsForExpression(ILogicalExpression expr) {
List<ARecordType> result = new ArrayList<>();
collectPathsForExpressionInternal(expr, result);
return result;
}

private void collectPathsForExpressionInternal(ILogicalExpression expr, List<ARecordType> out) {
if (expr instanceof AbstractFunctionCallExpression) {
AbstractFunctionCallExpression f = (AbstractFunctionCallExpression) expr;
ARecordType t = paths.get(f);
if (t != null) {
out.add(t);
}
for (Mutable<ILogicalExpression> argRef : f.getArguments()) {
collectPathsForExpressionInternal(argRef.getValue(), out);
}
}
}

protected final AbstractFunctionCallExpression andExpression(ILogicalExpression filterExpr,
ILogicalExpression inlinedExpr) {
AbstractFunctionCallExpression funcExpr = (AbstractFunctionCallExpression) filterExpr;
Expand Down
Loading