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 3:

(6 comments)

Thanks for the review!

http://gerrit.cloudera.org:8080/#/c/24001/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/24001/2//COMMIT_MSG@23
PS2, Line 23: - '_prefix%' - starts with wildcard
> We should pushdown it with Expressions.equal().
Thanks for the suggestion. Done


http://gerrit.cloudera.org:8080/#/c/24001/1/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/1/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@159
PS1, Line 159:    * Only processes up to firstWildcard position if specified,
> Note for the reviewers, suggest some better alternative if available!
Done


http://gerrit.cloudera.org:8080/#/c/24001/2/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/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@160
PS2, Line 160:    * otherwise the entire string.
             :    */
             :   private String unescapeLikePattern(String pattern, int endPos) 
{
             :     int len = (endPos == -1) ? pattern.length() : endPos;
             :     S
> Yes, we can make it public.
Done


http://gerrit.cloudera.org:8080/#/c/24001/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@213
PS2, Line 213: ing pattern = ((StringLiteral) literal).getUnesca
> As mentioned in the commit message, we should just do this for the user: st
Done


http://gerrit.cloudera.org:8080/#/c/24001/2/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@227
PS2, Line 227:       char c = pattern.charAt(i);
             :
             :       // Check if this character is a wildcard
             :       if (c == '%' || c == '_') {
             :         // Check if it's escaped by counting preceding 
backslashes
             :         int backslashCount = 0;
             :
> Please note that wildcards can be escaped via '\':
Great catch! Modified the logic to take care of the escaping.


http://gerrit.cloudera.org:8080/#/c/24001/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-like-pushdown.test
File 
testdata/workloads/functional-query/queries/QueryTest/iceberg-like-pushdown.test:

http://gerrit.cloudera.org:8080/#/c/24001/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-like-pushdown.test@194
PS2, Line 194: ---- QUERY
> OK, no problem. Thanks for trying it out.
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: 3
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: Fri, 20 Feb 2026 18:52:30 +0000
Gerrit-HasComments: Yes

Reply via email to