----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66349/#review200203 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 647 (patched) <https://reviews.apache.org/r/66349/#comment280849> Extract the information from the UDF instead of hardcoding the name: final String inlineFunctionName = GenericUDTFInline.class.getAnnotation(Description.class).name(); ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 650 (patched) <https://reviews.apache.org/r/66349/#comment280850> Same for array function. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 683 (patched) <https://reviews.apache.org/r/66349/#comment280853> No need for check, if colName is not present it will return null. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 693 (patched) <https://reviews.apache.org/r/66349/#comment280852> No need for check, if fs.getName is not present it will return null. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 717 (patched) <https://reviews.apache.org/r/66349/#comment280854> You can inline instantiation _newNode = new ParseDriver().parseExpression(newValue);_ ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 736 (patched) <https://reviews.apache.org/r/66349/#comment280856> We can be more aggressive here to prevent any undesired state in the future. Pseudo: if (selectExprs.getChild(0) != "ROW_ID") { throw new SemanticException("Unexpected element when replacing default keyword for update. Expected ROW_ID, found: " + selectExprs.getChild(0)); } ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 746 (patched) <https://reviews.apache.org/r/66349/#comment280860> Can we leave a LOG.debug message "inserted default for x: y" or something alike? ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 763 (patched) <https://reviews.apache.org/r/66349/#comment280857> We can be aggressive here too. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 772 (patched) <https://reviews.apache.org/r/66349/#comment280861> Can we leave a LOG.debug message "inserted default for x: y" or something alike? ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 779 (patched) <https://reviews.apache.org/r/66349/#comment280859> Can we refactor this so we call _isValueClause_ and _updating_ functions only once (trying to avoid some overhead)? - Jesús Camacho Rodríguez On March 29, 2018, 12:14 a.m., Vineet Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66349/ > ----------------------------------------------------------- > > (Updated March 29, 2018, 12:14 a.m.) > > > Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez. > > > Bugs: HIVE-19059 > https://issues.apache.org/jira/browse/HIVE-19059 > > > Repository: hive-git > > > Description > ------- > > This patch adds support for specifying DEFAULT keyword with INSERT and UPDATE > e.g. > INSERT INTO T1 values(DEFAULT, DEFAULT..) > > > Diffs > ----- > > itests/src/test/resources/testconfiguration.properties 5985dcfab9 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 26f20f2e05 > ql/src/test/queries/clientpositive/insert_into_default_keyword.q > PRE-CREATION > ql/src/test/results/clientpositive/llap/insert_into_default_keyword.q.out > PRE-CREATION > > > Diff: https://reviews.apache.org/r/66349/diff/2/ > > > Testing > ------- > > Added more tests > Existing pcommit tests > > > Thanks, > > Vineet Garg > >
