Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24001 )

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


Patch Set 4:

(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: "test\\%value" becomes "test%value"
It should become "test\%value", right? As '\\' amortizes to '\'.


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


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


http://gerrit.cloudera.org:8080/#/c/24001/4/fe/src/main/java/org/apache/impala/common/IcebergPredicateConverter.java@223
PS4, Line 223:     // Find first unescaped wildcard to extract prefix
             :     // Note: wildcards can be escaped with '\', e.g., 'asd\%' 
matches literal '%'
             :     int firstWildcard = -1;
             :     for (int i = 0; i < pattern.length(); i++) {
             :       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;
             :         int j = i - 1;
             :         while (j >= 0 && pattern.charAt(j) == '\\') {
             :           backslashCount++;
             :           j--;
             :         }
             :
             :         // If odd number of backslashes, the wildcard is escaped
             :         if (backslashCount % 2 == 0) {
             :           // Even number (including 0) - wildcard is not escaped
             :           firstWildcard = i;
             :           break;
             :         }
             :       }
             :     }
nit: please move it to a helper method.


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:

select * from iceberg_partitioned where action like "d%d" and event_time < 
"2022-01-01" and id < 10;

I want to make sure that like "d%d" will appear in "predicates:". And we 
shouldn't only see "action LIKE 'd%'" in "skipped Iceberg predicates:"



--
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: 4
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 09:55:11 +0000
Gerrit-HasComments: Yes

Reply via email to