----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68897/#review209179 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java Line 1376 (original), 1376 (patched) <https://reviews.apache.org/r/68897/#comment293503> Can you add a comment on what this for loop is suppose to do? ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java Lines 1409 (patched) <https://reviews.apache.org/r/68897/#comment293504> replaceDefaultKeywordForMerge() is based on index. That is it assumes values list is in same order as in column list in target table which was true till now, but now with this change columns can be in any order and this may not work. ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java Line 1388 (original), 1410 (patched) <https://reviews.apache.org/r/68897/#comment293505> valuesClause may need to reorder column which are in columnListNode order to match them to order of target table. See, my ask to add new tests in later comments. ql/src/test/queries/clientpositive/sqlmerge_stats.q Line 34 (original), 34 (patched) <https://reviews.apache.org/r/68897/#comment293506> Can you please add following tests: 1. This should throw an error, since values clause cardinality need to match columnlist cardinality. MERGE into t as t using upd_t as u ON t.a = u.a WHEN MATCHED THEN DELETE WHEN NOT MATCHED THEN INSERT (a) VALUES(u.a); 2. merge into t as t using upd_t as u ON t.a = u.a WHEN MATCHED THEN DELETE WHEN NOT MATCHED THEN INSERT (b, a) VALUES(u.a, u.b); 3. Assuming t's schema is create table t (a int, b default 1) then merge into t as t using upd_t as u ON t.a = u.a WHEN MATCHED THEN DELETE WHEN NOT MATCHED THEN INSERT (b, a) VALUES(default, u.b); 4. Assuming t's schema is create table t (a int, b default 1) then merge into t as t using upd_t as u ON t.a = u.a WHEN MATCHED THEN update set b = default WHEN NOT MATCHED THEN INSERT (b, a) VALUES(default, u.b); - Ashutosh Chauhan On Oct. 2, 2018, 1:26 p.m., Miklos Gergely wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68897/ > ----------------------------------------------------------- > > (Updated Oct. 2, 2018, 1:26 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-20590 > https://issues.apache.org/jira/browse/HIVE-20590 > > > Repository: hive-git > > > Description > ------- > > Allow merge statement to have column schema. > > Also removed some unused code, and made the rewritten query more consistent > (upper case SQL keywords everywhere) > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 78bc87c > > ql/src/java/org/apache/hadoop/hive/ql/parse/UpdateDeleteSemanticAnalyzer.java > e8823e1 > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCardinalityViolation.java > b688447 > ql/src/test/queries/clientpositive/sqlmerge_stats.q c480eb6 > ql/src/test/results/clientpositive/llap/sqlmerge_stats.q.out 02aa87a > > > Diff: https://reviews.apache.org/r/68897/diff/1/ > > > Testing > ------- > > Tested on local cluster using the new syntax. Also modified a q file to use > the new syntax. > > > Thanks, > > Miklos Gergely > >