Avoid pushdown IS (NOT) NULL expression on numeric types to Glue#13138
Avoid pushdown IS (NOT) NULL expression on numeric types to Glue#13138
IS (NOT) NULL expression on numeric types to Glue#13138Conversation
plugin/trino-hive/src/main/java/io/trino/plugin/hive/metastore/glue/GlueExpressionUtil.java
Outdated
Show resolved
Hide resolved
ebyhr
force-pushed
the
ebi/hive-glue-null
branch
from
593c944 to
89c29f1
Compare
| } | ||
|
|
||
| // Glue throws an exception (e.g. input string: "__HIVE_D" is not an integer) | ||
| // for column <> '__HIVE_DEFAULT_PARTITION__' or column = '__HIVE_DEFAULT_PARTITION__' expression on numeric types |
There was a problem hiding this comment.
Is there is any specific reason on not using NOT or IS NULL logical operator provided by Glue ? It was mentioned here - but any reason on not using it https://docs.aws.amazon.com/glue/latest/dg/aws-glue-api-catalog-partitions.html#aws-glue-api-catalog-partitions-GetPartitions
There was a problem hiding this comment.
@findinpath and I confirmed the expression returns incorrect result. Let me leave a code comment.
There was a problem hiding this comment.
Similar behavior is shown even for numeric type ?
There was a problem hiding this comment.
Yes, it behaves same regardless of the types. (We already shared this issue with AWS engineers)
There was a problem hiding this comment.
@Praveen2112 Added a comment. Please take another look.
Glue throws an exception. e.g. input string: "__HIVE_D" is not an integer
ebyhr
force-pushed
the
ebi/hive-glue-null
branch
from
89c29f1 to
93bc28e
Compare
Praveen2112
left a comment
Praveen2112
left a comment
There was a problem hiding this comment.
Additionally can we track the issue i.e pushdown is not being applied when we try to fetch the partitions for IS (NOT) NULL on numeric types. So once AWS fixes the issue we could use NULL operator.
|
Filed #13150 |
| // https://docs.aws.amazon.com/glue/latest/dg/aws-glue-api-catalog-partitions.html#aws-glue-api-catalog-partitions-GetPartitions | ||
| if ((domain.getValues().isAll() || domain.getValues().isNone()) && !isQuotedType(domain.getType())) { | ||
| return Optional.empty(); | ||
| } |
There was a problem hiding this comment.
The code below doesn't handle the IS NULL OR IS some value case correctly.
-- It's apparent from the fact it "recognizes" IS NULL case by inspecting valueSet.isNone() (which is quite implicit), instead of checking domain.isNullAllowed(). Thus, it doesn't discern "some values" and "some values OR NULL" cases.
#13214 should make it clear (it doesn't solve the problem just yet)
There was a problem hiding this comment.
Thanks for your explanation. I was going to handle IS NULL OR IS some value case in #13122 after this PR.
4 participants
Description
Avoid pushdown
IS (NOT) NULLexpression on numeric types to GlueFixes #13124
Documentation
(x) No documentation is needed.
Release notes
(x) Release notes entries required with the following suggested text: