HIVE-29377: Fix inconsistency for partition filter LIKE clause#6240
HIVE-29377: Fix inconsistency for partition filter LIKE clause#6240wecharyu wants to merge 1 commit intoapache:masterfrom
Conversation
|
|
@dengzhhu653 @zhangbutao @deniskuzZ could you please take a look? |
| POSTHOOK: Input: default@test_tbl | ||
| POSTHOOK: Input: default@test_tbl@b=d_%5C%25ae | ||
| #### A masked pattern was here #### | ||
| PREHOOK: query: select * from test_tbl where b like 'a.*' |
There was a problem hiding this comment.
The conversion rule for this PR is: if the character .* is encountered in the partition predicate of Hive SQL, it is mapped to % in MySQL (matching zero or more characters), then the partition predicate a.* should be converted to a% in MySQL, and the direct SQL would query two partition entries, namely abc and af%.
However, why is the query result from q.out empty here?
There was a problem hiding this comment.
The POSTHOOK Input shows that it got matching partitions from HMS, but hive optimizer PartitionConditionRemover will filter partitions again by original pattern like a.*, and does not convert to a% so it will filter out the matching partitions.
POSTHOOK: Input: default@test_tbl
POSTHOOK: Input: default@test_tbl@b=abc
POSTHOOK: Input: default@test_tbl@b=af%25
There was a problem hiding this comment.
So there is an issue here. The HMS can return partition information based on like .*, but the PartitionConditionRemover considers these partitions invalid. This seems inconsistent—we need to maintain consistency. cc @dengzhhu653 @deniskuzZ
Additionally, I am not sure if .* is a commonly used expression in SQL LIKE filtering. Could you give examples of similar LIKE matching usages in other sql engines?
There was a problem hiding this comment.
SQL engines usually not use .* as a filter, but HMS thrift APIs support it as a pattern, I think it's because HMS uses JDO matches() for like filter.
And Spark is using the .* pattern for partition filter like here:
https://github.com/apache/spark/blob/8cc2e4677d596bada67d05792f6fd896e5fc640f/sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveShim.scala#L827-L834
There was a problem hiding this comment.
per my understanding, we don't have a single pattern for both jdo and the direct sql, jdo accepts .* (Java-type pattern) for any sequences and the sql allows _% for the same game. When comes to HMS API, the user needs to take care of the pattern, whether it should be a Java-type pattern or sql pattern.
Not sure why PartitionConditionRemover would remove the partition condition, given the pruned partitions from Metastore is not empty.
There was a problem hiding this comment.
The jdo like is much more complex than the sql type. In the direct sql we assume the like is a Jdo pattern, and the sql pattern in the jdo query for the same API, ObjectStore#getPartitionsByFilter for example, this could make things worse, if we disable the direct sql, it could give us a different answer for the same input.
do you think we should not support java pattern filter?
We use the Java filter somewhere, the tablePattern in getTableObjectsByName for example. If an ObjectStore API has implemented the direct sql or going to be, then for this API, I would suggest the sql type pattern for the filter.
There was a problem hiding this comment.
if we disable the direct sql, it could give us a different answer for the same input.
The result could be different before this patch just like the updated ppd_like_filter.q test shows, we are fixing it here. Pls correct me if I miss something.
If an ObjectStore API has implemented the direct sql or going to be, then for this API, I would suggest the sql type pattern for the filter.
HMS users may not know the API details, it's a bit difficult for them to know which api should use sql filter and which one should use java pattern. In another word, such difference seems not friendly to users.
I think we have 2 choices to keep consistency:
- Declare in the user document that filter is SQL only, and escape the java pattern in JDO implement of each API.
- Both direct implement and JDO implement supports java pattern and sql filter, which is what we do here.
There was a problem hiding this comment.
The result could be different before this patch just like the updated ppd_like_filter.q test shows, we are fixing it here. Pls correct me if I miss something.
I don't get it here, why would it cause different result without this PR? in case both the jdo and direct sql accept the SQL type pattern.
Both direct implement and JDO implement supports java pattern and sql filter, which is what we do here.
The reason is the java pattern is much more complex, imaging a java regex for IP: ^(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})$, changing it to a sql filter looks quite hard and not easy to test.
Declare in the user document that filter is SQL only, and escape the java pattern in JDO implement of each API.
We can add some annotations in IMetastoreClient to tell which pattern type is accepted
There was a problem hiding this comment.
The result could be different before this patch just like the updated ppd_like_filter.q test shows, we are fixing it here. Pls correct me if I miss something.
I somehow get the point, if the input is a java pattern which the user thinks this API should accept, then the direct sql might return a different result compared to the jdo, however this also might be a problem if another user insists that his input is a sql type pattern.
There was a problem hiding this comment.
if the input is a java pattern which the user thinks this API should accept, then the direct sql might return a different result compared to the jdo
Yep, that's the motivation of this patch.
I think user should know it supports both java pattern and sql filter for now, otherwise it's also a problem if a user think he is input a java pattern filter.
4 participants
What changes were proposed in this pull request?
ESCAPE '\\\\'for MySQL db..*and.pattern for Direct SQL query.Why are the changes needed?
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Add test in
ppd_like_filter.q:mvn test -pl itests/qtest -Pitests -Dtest=TestMiniLlapLocalCliDriver -Dtest.output.overwrite=true -Dqfile=ppd_like_filter.q -Dtest.metastore.db=mysqlThe result will be a little different in different dbs because in derby the ESCAPE is '\' but in MySQL the ESCAPE is '\\'.