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

Reply via email to