Arnab Karmakar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24001 )

Change subject: IMPALA-14737: Push down LIKE predicates to Iceberg
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/24001/4/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java
File fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java:

http://gerrit.cloudera.org:8080/#/c/24001/4/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@158
PS4, Line 158:  be escaped with '\', e.g., 'asd\%'
> It should become "test\%value", right? As '\\' amortizes to '\'.
When SQL has LIKE 'test\%value', the getUnescapedValue() method preserves the 
LIKE escape sequence and returns "test\\%value". Our unescapeLikePattern() 
method then removes this escape to produce "test%value" (literal %) for 
Iceberg. Documented it in the docstring.


http://gerrit.cloudera.org:8080/#/c/24001/4/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@162
PS4, Line 162: ttern.charAt(i);
> Can you add a few unit tests for this function?
Done


http://gerrit.cloudera.org:8080/#/c/24001/4/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@162
PS4, Line 162:     cha
> can be static as well
Done


http://gerrit.cloudera.org:8080/#/c/24001/4/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@223
PS4, Line 223:
             :   protected Expression convert(LikePredicate predicate)
             :       throws ImpalaRuntimeException {
             :     // Only LIKE operator is supported, not RLIKE, REGEXP, etc.
             :     if (predicate.getOp() != LikePredicate.Operator.LIKE) {
             :       throw new ImpalaRuntimeException(String.format(
             :           "Only LIKE operator is supported for Iceberg pushdown, 
got: %s",
             :           predicate.getOp()));
             :     }
             :
             :     Term term = getTerm(predicate.getChild(0));
             :     IcebergColumn column = term.referencedColumn_;
             :
             :     // Check if the column is a string type
             :     if (!column.getType().isStringType()) {
             :       throw new ImpalaRuntimeException(String.format(
             :           "LIKE predicate pushdown only supports string columns, 
got: %s",
             :           column.getType()));
             :     }
             :
             :     LiteralExpr literal = getSecondChildAsLiteralExpr(predicate);
             :     checkNullLiteral(literal);
             :
             :     i
> nit: please move it to a helper method.
Done


http://gerrit.cloudera.org:8080/#/c/24001/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test:

http://gerrit.cloudera.org:8080/#/c/24001/4/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-predicates.test@122
PS4, Line 122: select * from iceberg_partitioned where action like "d%" and 
event_time < "2022-01-01" and id < 10
> Can you please add a test for the following query:
Done



--
To view, visit http://gerrit.cloudera.org:8080/24001
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I548834126540bcc8d22efc872c2571293b8b7ec4
Gerrit-Change-Number: 24001
Gerrit-PatchSet: 5
Gerrit-Owner: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Mon, 23 Feb 2026 14:21:36 +0000
Gerrit-HasComments: Yes

Reply via email to