HIVE-28917: NPE in merge statement when checking nullability of joini… #5781
Conversation
| merge into target as t using source as s on t.a = s.a | ||
| when matched and t.a is null then delete; |
There was a problem hiding this comment.
From a SQL perspective the query is perfectly valid so there is no reason to throw an error or exception. The query is probably useless cause indeed the when matched and t.a is null is always false but this is not a SQL error. The expected result is to simply leave the target table unchanged.
|
|
||
| int tokFromIdx = query.getFirstChildWithType(HiveParser.TOK_FROM).getChildIndex(); | ||
| ASTNode from = (ASTNode) query.getFirstChildWithType(HiveParser.TOK_FROM); | ||
| assert from != null : "Couldn't find a child of type FROM in the AST"; |
There was a problem hiding this comment.
Assertions should never be used for user-level errors. Despite the fact that they are disabled in production environments (and thus a user will never encounter) stack-traces and other information associated with the assertion/exception are for developers to help diagnose and fix a problem. Problems with the SQL syntax should be user-level errors with appropriate error codes and localization.
However, as I mentioned previously the MERGE statement addressed here is valid SQL so it shouldn't lead to failures or exceptions of any kind.
asf-ci-hive
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
…ng column in matched clause
soumyakanti3578
force-pushed
the
HIVE-28917
branch
from
5efdfe4 to
ef385f5
Compare
|
…ng column in matched clause
What changes were proposed in this pull request?
When TOK_FROM is not present in the AST for MERGE statements, return
DestClausePrefix.MERGE.Why are the changes needed?
Without this we could get an NPE as explained in HIVE-28917
Does this PR introduce any user-facing change?
No
How was this patch tested?