Skip to content

Commit d1a5a61

Browse files
[Enhancement] Support partial pushdown for AND compound predicates in Iceberg connector (backport #70293) (#73085)
Signed-off-by: Jiwon Park <jpark92@outlook.kr> Co-authored-by: Jiwon Park <jpark92@outlook.kr>
1 parent 3897f52 commit d1a5a61

2 files changed

Lines changed: 126 additions & 2 deletions

File tree

fe/fe-core/src/main/java/com/starrocks/connector/iceberg/ScalarOperatorToIcebergExpr.java

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,12 @@ public Expression convertStrict(List<ScalarOperator> operators, IcebergContext c
110110

111111
public Expression convert(List<ScalarOperator> operators, IcebergContext context, boolean strict) {
112112
IcebergExprVisitor visitor = new IcebergExprVisitor();
113+
IcebergContext effectiveContext = strict && !context.isStrict()
114+
? new IcebergContext(context.getSchema(), context.isInsideNot(), true)
115+
: context;
113116
List<Expression> expressions = Lists.newArrayList();
114117
for (ScalarOperator operator : operators) {
115-
Expression filterExpr = operator.accept(visitor, context);
118+
Expression filterExpr = operator.accept(visitor, effectiveContext);
116119
if (filterExpr == null) {
117120
if (strict) {
118121
LOG.debug("Strict mode: cannot convert operator {}", operator.debugString());
@@ -141,14 +144,34 @@ public Expression convert(List<ScalarOperator> operators, IcebergContext context
141144

142145
public static class IcebergContext {
143146
private final Types.StructType schema;
147+
private final boolean insideNot;
148+
private final boolean strict;
144149

145150
public IcebergContext(Types.StructType schema) {
151+
this(schema, false, false);
152+
}
153+
154+
public IcebergContext(Types.StructType schema, boolean insideNot, boolean strict) {
146155
this.schema = schema;
156+
this.insideNot = insideNot;
157+
this.strict = strict;
147158
}
148159

149160
public Types.StructType getSchema() {
150161
return schema;
151162
}
163+
164+
public boolean isInsideNot() {
165+
return insideNot;
166+
}
167+
168+
public boolean isStrict() {
169+
return strict;
170+
}
171+
172+
public IcebergContext withInsideNot() {
173+
return new IcebergContext(schema, true, strict);
174+
}
152175
}
153176

154177
private static class IcebergExprVisitor extends ScalarOperatorVisitor<Expression, IcebergContext> {
@@ -185,7 +208,7 @@ public Expression visitCompoundPredicate(CompoundPredicateOperator operator, Ice
185208
if (operator.getChild(0) instanceof LikePredicateOperator) {
186209
return null;
187210
}
188-
Expression expression = operator.getChild(0).accept(this, context);
211+
Expression expression = operator.getChild(0).accept(this, context.withInsideNot());
189212

190213
if (expression != null) {
191214
return not(expression);
@@ -196,6 +219,21 @@ public Expression visitCompoundPredicate(CompoundPredicateOperator operator, Ice
196219
if (left != null && right != null) {
197220
return (op == CompoundPredicateOperator.CompoundType.OR) ? or(left, right) : and(left, right);
198221
}
222+
// For AND predicates outside of NOT, allow partial pushdown.
223+
// If only one side converts successfully, push down that side alone.
224+
// This is safe because AND(a, b) is more restrictive than just a (or just b),
225+
// so pushing down one side still correctly filters data.
226+
// This must NOT be done inside NOT, because NOT(AND(a, b)) = OR(NOT(a), NOT(b)),
227+
// and pushing down NOT(a) alone would over-filter.
228+
if (!context.isStrict() && !context.isInsideNot()
229+
&& op == CompoundPredicateOperator.CompoundType.AND) {
230+
if (left != null) {
231+
return left;
232+
}
233+
if (right != null) {
234+
return right;
235+
}
236+
}
199237
}
200238
return null;
201239
}

fe/fe-core/src/test/java/com/starrocks/connector/iceberg/IcebergExprVisitorTest.java

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,92 @@ public void testToIcebergExpressionDotColumn() {
451451
Assertions.assertEquals(Expression.Operation.TRUE, convertedExpr.op());
452452
}
453453

454+
@Test
455+
public void testAndPartialPushdown() {
456+
ScalarOperatorToIcebergExpr.IcebergContext context = new ScalarOperatorToIcebergExpr.IcebergContext(SCHEMA.asStruct());
457+
ScalarOperatorToIcebergExpr converter = new ScalarOperatorToIcebergExpr();
458+
459+
// Build a convertible predicate: k1 > 10
460+
BinaryPredicateOperator convertible = new BinaryPredicateOperator(
461+
BinaryType.GT, K1, ConstantOperator.createInt(10));
462+
463+
// Build an unconvertible predicate: CAST(k6 AS INT) < 5
464+
// CastOperator(INT, K6) where K6 is a string column causes getLiteralValue to return null
465+
CastOperator cast = new CastOperator(IntegerType.INT, K6);
466+
BinaryPredicateOperator unconvertible = new BinaryPredicateOperator(
467+
BinaryType.LT, cast, ConstantOperator.createInt(5));
468+
469+
// Verify that the unconvertible predicate alone produces alwaysTrue
470+
Expression unconvertibleExpr = converter.convert(Lists.newArrayList(unconvertible), context);
471+
Assertions.assertEquals(Expression.Operation.TRUE, unconvertibleExpr.op(),
472+
"Unconvertible predicate should produce alwaysTrue");
473+
474+
Expression convertedExpr;
475+
Expression expectedExpr;
476+
477+
// AND(convertible, unconvertible) -> returns the convertible side
478+
convertedExpr = converter.convert(Lists.newArrayList(
479+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.AND,
480+
convertible, unconvertible)), context);
481+
expectedExpr = Expressions.greaterThan("k1", 10);
482+
Assertions.assertEquals(expectedExpr.toString(), convertedExpr.toString(),
483+
"AND(convertible, unconvertible) should push down the convertible side");
484+
485+
// AND(unconvertible, convertible) -> returns the convertible side
486+
convertedExpr = converter.convert(Lists.newArrayList(
487+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.AND,
488+
unconvertible, convertible)), context);
489+
expectedExpr = Expressions.greaterThan("k1", 10);
490+
Assertions.assertEquals(expectedExpr.toString(), convertedExpr.toString(),
491+
"AND(unconvertible, convertible) should push down the convertible side");
492+
493+
// OR(convertible, unconvertible) -> returns alwaysTrue (no partial pushdown for OR)
494+
convertedExpr = converter.convert(Lists.newArrayList(
495+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.OR,
496+
convertible, unconvertible)), context);
497+
Assertions.assertEquals(Expression.Operation.TRUE, convertedExpr.op(),
498+
"OR(convertible, unconvertible) should NOT do partial pushdown");
499+
500+
// NOT(AND(convertible, unconvertible)) -> returns alwaysTrue (no partial pushdown inside NOT)
501+
CompoundPredicateOperator andOp = new CompoundPredicateOperator(
502+
CompoundPredicateOperator.CompoundType.AND, convertible, unconvertible);
503+
convertedExpr = converter.convert(Lists.newArrayList(
504+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.NOT, andOp)), context);
505+
Assertions.assertEquals(Expression.Operation.TRUE, convertedExpr.op(),
506+
"NOT(AND(convertible, unconvertible)) should NOT do partial pushdown");
507+
508+
// AND(convertible1, convertible2) -> returns and(left, right) (regression test)
509+
BinaryPredicateOperator convertible2 = new BinaryPredicateOperator(
510+
BinaryType.LT, K2, ConstantOperator.createInt(20));
511+
convertedExpr = converter.convert(Lists.newArrayList(
512+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.AND,
513+
convertible, convertible2)), context);
514+
expectedExpr = Expressions.and(
515+
Expressions.greaterThan("k1", 10),
516+
Expressions.lessThan("k2", 20));
517+
Assertions.assertEquals(expectedExpr.toString(), convertedExpr.toString(),
518+
"AND(convertible1, convertible2) should return and(left, right)");
519+
520+
// Nested AND: AND(AND(convertible, unconvertible), convertible2) -> returns and(convertible, convertible2)
521+
CompoundPredicateOperator innerAnd = new CompoundPredicateOperator(
522+
CompoundPredicateOperator.CompoundType.AND, convertible, unconvertible);
523+
convertedExpr = converter.convert(Lists.newArrayList(
524+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.AND,
525+
innerAnd, convertible2)), context);
526+
expectedExpr = Expressions.and(
527+
Expressions.greaterThan("k1", 10),
528+
Expressions.lessThan("k2", 20));
529+
Assertions.assertEquals(expectedExpr.toString(), convertedExpr.toString(),
530+
"AND(AND(convertible, unconvertible), convertible2) should return and(convertible, convertible2)");
531+
532+
// Strict mode: AND(convertible, unconvertible) -> returns null (no partial pushdown)
533+
convertedExpr = converter.convertStrict(Lists.newArrayList(
534+
new CompoundPredicateOperator(CompoundPredicateOperator.CompoundType.AND,
535+
convertible, unconvertible)), context);
536+
Assertions.assertNull(convertedExpr,
537+
"Strict mode: AND(convertible, unconvertible) should return null");
538+
}
539+
454540
@Test
455541
public void testConvertLastUpdatedSequenceNumberPredicate() {
456542
ScalarOperatorToIcebergExpr.IcebergContext context = new ScalarOperatorToIcebergExpr.IcebergContext(SCHEMA.asStruct());

0 commit comments

Comments
 (0)