----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45274/#review130263 -----------------------------------------------------------
metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (line 1050) <https://reviews.apache.org/r/45274/#comment193980> You are already inside transaction as part of dropTable(), no need for opening/closing Txn here. ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java (lines 646 - 649) <https://reviews.apache.org/r/45274/#comment193984> Please update comments as per latest grammar. ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java (lines 720 - 724) <https://reviews.apache.org/r/45274/#comment193985> Update comments. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 2029) <https://reviews.apache.org/r/45274/#comment193986> I wonder if you could just have this rule as KW_PRIMARY KW_KEY columnParenthesesList.... is there a reason not to do it that way? Otherwise, in what condition else part of production is valid? ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 2038) <https://reviews.apache.org/r/45274/#comment193987> same comment as previous. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 2047) <https://reviews.apache.org/r/45274/#comment193988> here as well. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g (line 2056) <https://reviews.apache.org/r/45274/#comment193989> and here. ql/src/test/queries/clientnegative/create_with_constraints_validate.q (line 1) <https://reviews.apache.org/r/45274/#comment193981> Can you add a -ve test case where table is created with existing constriant name? ql/src/test/queries/clientpositive/create_with_constraints.q (line 2) <https://reviews.apache.org/r/45274/#comment193983> Please add describe statement for all tables. They should list all constraints of table. ql/src/test/queries/clientpositive/create_with_constraints.q (line 3) <https://reviews.apache.org/r/45274/#comment193982> Can you add a test case for rely/no-rely? Both should succeed. - Ashutosh Chauhan On April 23, 2016, 7:17 a.m., Hari Sankar Sivarama Subramaniyan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45274/ > ----------------------------------------------------------- > > (Updated April 23, 2016, 7:17 a.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Repository: hive-git > > > Description > ------- > > Support primary keys/foreign keys constraint as part of create table command > in Hive > > > Diffs > ----- > > metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java > ae6f084 > ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f091f67 > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java b26f09d > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4c9acce > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > 19342a8 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g dd997f0 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 50c53db > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g a192fa7 > ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java > 549d24f > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 005b53f > ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java 03b4d8b > ql/src/test/queries/clientnegative/create_with_constraints_enable.q > PRE-CREATION > ql/src/test/queries/clientnegative/create_with_constraints_validate.q > PRE-CREATION > ql/src/test/queries/clientpositive/create_with_constraints.q PRE-CREATION > ql/src/test/results/clientnegative/create_with_constraints_enable.q.out > PRE-CREATION > ql/src/test/results/clientnegative/create_with_constraints_validate.q.out > PRE-CREATION > ql/src/test/results/clientpositive/create_with_constraints.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/45274/diff/ > > > Testing > ------- > > Local testing done > > > Thanks, > > Hari Sankar Sivarama Subramaniyan > >
