zabetak commented on code in PR #5781:
URL: https://github.com/apache/hive/pull/5781#discussion_r2053456664


##########
iceberg/iceberg-handler/src/test/queries/negative/merge_with_null_check_on_joining_col.q:
##########
@@ -0,0 +1,5 @@
+create table target(a int, b int) stored by iceberg 
tblproperties('format-version'='2', 'write.merge.mode'='copy-on-write');
+create table source(a int, b int) stored by iceberg 
tblproperties('format-version'='2', 'write.merge.mode'='copy-on-write');
+
+merge into target as t using source as s on t.a = s.a
+when matched and t.a is null then delete;

Review 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. 



##########
ql/src/java/org/apache/hadoop/hive/ql/Context.java:
##########
@@ -361,8 +361,10 @@ private DestClausePrefix getMergeDestClausePrefix(ASTNode 
curNode) {
     assert insert != null && insert.getType() == HiveParser.TOK_INSERT;
     ASTNode query = (ASTNode) insert.getParent();
     assert query != null && query.getType() == HiveParser.TOK_QUERY;
-
-    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";

Review 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. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to