You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Ashutosh Chauhan <ha...@apache.org> on 2016/04/07 00:37:03 UTC

Re: Review Request 45274: HIVE-13290 : Support PK/FK constraint as part of create table command

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45274/#review127479
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java (line 658)
<https://reviews.apache.org/r/45274/#comment190821>

    PrimaryKeys should not be part of Table object.



ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g (lines 321 - 328)
<https://reviews.apache.org/r/45274/#comment190815>

    We should not reserve these KWs, as this may cause backward compatibility issue.



ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java (line 324)
<https://reviews.apache.org/r/45274/#comment190820>

    Constraint name should be computed at Metastore server, not in client.


- Ashutosh Chauhan


On March 24, 2016, 7:08 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45274/
> -----------------------------------------------------------
> 
> (Updated March 24, 2016, 7:08 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
> -----
> 
>   hcatalog/core/src/test/java/org/apache/hive/hcatalog/common/TestHCatUtil.java 102d6d2 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java 6caf3fe 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggrStatsCacheIntegration.java 51d96dd 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseImport.java af60660 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseSchemaTool.java e5833b8 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreIntegration.java 2cc1373 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/hbase/TestStorageDescriptorSharing.java c29e46a 
>   metastore/if/hive_metastore.thrift 6a55962 
>   metastore/scripts/upgrade/derby/034-HIVE-13076.derby.sql PRE-CREATION 
>   metastore/scripts/upgrade/derby/hive-schema-2.1.0.derby.sql 42f4eb6 
>   metastore/scripts/upgrade/derby/upgrade-2.0.0-to-2.1.0.derby.sql a0bac3c 
>   metastore/scripts/upgrade/mssql/hive-schema-2.1.0.mssql.sql cf5a662 
>   metastore/scripts/upgrade/mssql/upgrade-2.0.0-to-2.1.0.mssql.sql f25daf2 
>   metastore/scripts/upgrade/mysql/034-HIVE-13076.mysql.sql PRE-CREATION 
>   metastore/scripts/upgrade/mysql/hive-schema-2.1.0.mysql.sql 6fd3209 
>   metastore/scripts/upgrade/mysql/upgrade-2.0.0-to-2.1.0.mysql.sql e790636 
>   metastore/scripts/upgrade/oracle/034-HIVE-13076.oracle.sql PRE-CREATION 
>   metastore/scripts/upgrade/oracle/hive-schema-2.1.0.oracle.sql 774f6be 
>   metastore/scripts/upgrade/oracle/upgrade-2.0.0-to-2.1.0.oracle.sql 8368d08 
>   metastore/scripts/upgrade/postgres/033-HIVE-13076.postgres.sql PRE-CREATION 
>   metastore/scripts/upgrade/postgres/hive-schema-2.1.0.postgres.sql 7463a37 
>   metastore/scripts/upgrade/postgres/upgrade-2.0.0-to-2.1.0.postgres.sql 6172407 
>   metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 6e5de20 
>   metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h 97c07a5 
>   metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp 2695ffa 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDynamicPartitions.java bb6e584 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsRequest.java 083d340 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsResult.java 9004457 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AggrStats.java bfd1206 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java 657bb7b 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ColumnStatistics.java ba059fe 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Database.java 2769845 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/DropPartitionsResult.java adef415 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/EnvironmentContext.java de588e2 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java 6b08234 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Function.java 5f8ce0d 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java f88e279 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java 0236b4a 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java 89eb819 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java 2408ad1 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java 8946635 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsInfoResponse.java 629c042 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsResponse.java 9f57a4a 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPrincipalsInRoleResponse.java 02c652a 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetRoleGrantsForPrincipalResponse.java c013e0e 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HeartbeatTxnRangeResponse.java b00fb9c 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HiveObjectRef.java d7dd8ec 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Index.java 44d99c7 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java 488d3a0 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/LockRequest.java f39f582 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java fcbbd18 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/OpenTxnsResponse.java e11a2b3 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Partition.java a247221 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionListComposingSpec.java e8232b9 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionSpecWithSharedSD.java 2c41912 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionWithoutSD.java f0c5ce1 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsByExprResult.java 12ae66d 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsRequest.java 8416369 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsResult.java 2e903f1 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrincipalPrivilegeSet.java e1792bd 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PrivilegeBag.java bba2d40 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java ab151b1 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RequestPartsSpec.java 7f8a044 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Schema.java 75d0ebb 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SerDeInfo.java d5a8d99 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SetPartitionsStatsRequest.java 6e334f6 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java afa832c 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowLocksResponse.java b9b7f3c 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SkewedInfo.java c32f50c 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/StorageDescriptor.java 938f06b 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Table.java 5d683fb 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsRequest.java d0daee5 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsResult.java 78d4250 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java 13e30db 
>   metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Type.java 14cd42e 
>   metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php 05a0749 
>   metastore/src/gen/thrift/gen-php/metastore/Types.php 488a920 
>   metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py ac8d8a4 
>   metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py 10eaf4a 
>   metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb 1cf40ae 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java d51f58d 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java ec24fe9 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MForeignKeyRel.java PRE-CREATION 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MPKFKMapping.java PRE-CREATION 
>   metastore/src/model/org/apache/hadoop/hive/metastore/model/MTable.java 2a78ce9 
>   metastore/src/model/package.jdo 7385a13 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStorePartitionSpecs.java 922a4bf 
>   metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java 2e1f5f4 
>   metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsCache.java 6cd3a46 
>   metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseAggregateStatsCacheWithBitVector.java 36c7984 
>   metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStore.java e4723f6 
>   metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreBitVector.java b1dc542 
>   metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStoreCached.java cfe9cd0 
>   ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java f091f67 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 5a72af1 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java f6ba521 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ForeignKeyInfo.java PRE-CREATION 
>   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/ImportSemanticAnalyzer.java 549d24f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseUtils.java 5f13277 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java d9db1d5 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java 03b4d8b 
> 
> Diff: https://reviews.apache.org/r/45274/diff/
> 
> 
> Testing
> -------
> 
> Local testing done
> 
> 
> Thanks,
> 
> Hari Sankar Sivarama Subramaniyan
> 
>


Re: Review Request 45274: HIVE-13290 : Support PK/FK constraint as part of create table command

Posted by pengcheng xiong <px...@hortonworks.com>.

> On April 6, 2016, 10:37 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, lines 321-328
> > <https://reviews.apache.org/r/45274/diff/1/?file=1313165#file1313165line321>
> >
> >     We should not reserve these KWs, as this may cause backward compatibility issue.

According to http://www.postgresql.org/docs/9.2/static/sql-keywords-appendix.html, primary, foreign, REFERENCES, CONSTRAINT are a reserved key word. So we can reserve all of them and add them under "sql11ReservedKeywordsUsedAsIdentifier" in IdenfiersParser.g for backward compatibility. For the rest of them, KEY, NOVALIDATE, RELY NORELY, we should put them under "nonReserved" in IdenfiersParser.g


- pengcheng


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45274/#review127479
-----------------------------------------------------------


On April 23, 2016, 8:20 p.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, 8:20 p.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_duplicate_name.q PRE-CREATION 
>   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_duplicate_name.q.out 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
> 
>