----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66183/#review199614 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/metadata/CheckConstraint.java Lines 47 (patched) <https://reviews.apache.org/r/66183/#comment279938> Can a single constraint name refer to multiple constraints? ql/src/java/org/apache/hadoop/hive/ql/metadata/CheckConstraint.java Lines 52 (patched) <https://reviews.apache.org/r/66183/#comment279939> I suppose you mean check expression? ql/src/java/org/apache/hadoop/hive/ql/metadata/CheckConstraint.java Lines 69 (patched) <https://reviews.apache.org/r/66183/#comment279944> Should this be assert instead of if() ? Will any caller pass in diff db/tbl name? Also, does this method should even accept db/tbl name as args? ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g Lines 363 (patched) <https://reviews.apache.org/r/66183/#comment279950> Add this in IdentifiersParser.g to make it non-reserved keyword. ql/src/test/queries/clientnegative/check_constraint_permanent_udf.q Lines 1 (patched) <https://reviews.apache.org/r/66183/#comment279958> you want to rename this test file. ql/src/test/queries/clientnegative/create_external_with_default_constraint.q Line 1 (original), 1 (patched) <https://reviews.apache.org/r/66183/#comment279959> This is already covered in previous test. you may want to undo this. ql/src/test/queries/clientpositive/check_constraint.q Lines 1 (patched) <https://reviews.apache.org/r/66183/#comment279960> Any reason for this? ql/src/test/queries/clientpositive/check_constraint.q Lines 41 (patched) <https://reviews.apache.org/r/66183/#comment279961> can you also add test for alter table add check constraint enable novalidate rely? ql/src/test/results/clientnegative/check_constraint_nonboolean_expr.q.out Lines 1 (patched) <https://reviews.apache.org/r/66183/#comment279962> Improved err msg: Only boolean type is supported. Found: int standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java Lines 2369 (patched) <https://reviews.apache.org/r/66183/#comment279963> There will always be db name and tbl name, no? standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java Lines 4498 (patched) <https://reviews.apache.org/r/66183/#comment279965> This method shares quite a bit of code with addDefaultConstraints() Might be a good idea to refactor 2 methods for code reuse? standalone-metastore/src/main/thrift/hive_metastore.thrift Lines 109 (patched) <https://reviews.apache.org/r/66183/#comment279964> Since user can't name a check constraint, does this field make sense? - Ashutosh Chauhan On March 20, 2018, 10:15 p.m., Vineet Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66183/ > ----------------------------------------------------------- > > (Updated March 20, 2018, 10:15 p.m.) > > > Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez. > > > Bugs: HIVE-18953 > https://issues.apache.org/jira/browse/HIVE-18953 > > > Repository: hive-git > > > Description > ------- > > This patch provides implementation for column level CHECK constraint > > > Diffs > ----- > > > itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java > e2244a1d4e > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 4eafcde7fb > ql/src/java/org/apache/hadoop/hive/ql/metadata/CheckConstraint.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java da690b4aa4 > > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java > da82f68f73 > > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java > bfc7b38ceb > > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java > 6309bfdc2b > > ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java > 006584839a > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > cc783cc4c8 > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 1605d7dd3f > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g a1ec96cff9 > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 3abc75248b > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > aa2d060618 > > ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/AddNotNullConstraintHandler.java > 7889e03573 > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java bf86cec488 > ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java ff9df3d87a > ql/src/java/org/apache/hadoop/hive/ql/plan/ImportTableDesc.java 7a955bc981 > ql/src/test/queries/clientnegative/check_constraint_aggregate.q > PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_max_length.q > PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_nonboolean_expr.q > PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_permanent_udf.q > PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_qual_name.q > PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_subquery.q PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_udtf.q PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_violation.q > PRE-CREATION > ql/src/test/queries/clientnegative/check_constraint_window_fun.q > PRE-CREATION > ql/src/test/queries/clientnegative/create_external_with_check_constraint.q > PRE-CREATION > > ql/src/test/queries/clientnegative/create_external_with_default_constraint.q > 4690c2cb0b > ql/src/test/queries/clientpositive/check_constraint.q PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_aggregate.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_max_length.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_non_permanent_udf.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_nonboolean_expr.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_permanent_udf.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_qual_name.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_subquery.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_udtf.q.out PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_violation.q.out > PRE-CREATION > ql/src/test/results/clientnegative/check_constraint_window_fun.q.out > PRE-CREATION > > ql/src/test/results/clientnegative/create_external_with_check_constraint.q.out > PRE-CREATION > ql/src/test/results/clientpositive/llap/check_constraint.q.out PRE-CREATION > ql/src/test/results/clientpositive/llap/default_constraint.q.out 72ff7af046 > ql/src/test/results/clientpositive/llap/enforce_constraint_notnull.q.out > eeb6a7a0b5 > standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h > 7206e296fd > standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp > b7a3b929be > > standalone-metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp > 8d9ad254a3 > standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h > 05a7a29fcc > standalone-metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp > 620d6ef6ba > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AbortTxnsRequest.java > 699f27b74e > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddCheckConstraintRequest.java > PRE-CREATION > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDefaultConstraintRequest.java > 6fe0cae3e6 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddDynamicPartitions.java > 2144dc0a37 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddForeignKeyRequest.java > 6bff65d90e > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddNotNullConstraintRequest.java > 35f1a56977 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsRequest.java > 02f0dfa1c3 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPartitionsResult.java > 347c267d5b > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddPrimaryKeyRequest.java > c2f12aa3cb > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AddUniqueConstraintRequest.java > 5cd7677cd8 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AllocateTableWriteIdsRequest.java > b143812dc7 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AllocateTableWriteIdsResponse.java > d9ed20d631 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CheckConstraintsRequest.java > PRE-CREATION > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CheckConstraintsResponse.java > PRE-CREATION > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java > 78d9255bb8 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java > c27792f61f > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionRequest.java > 18581f3983 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CreationMetadata.java > ed89b2e938 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/DropPartitionsResult.java > 5237c643f0 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java > 01bc32d70b > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java > 64335903ff > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Function.java > 5e785df67b > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java > ce90d65e76 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java > 025a04a33c > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java > 2ba496fb11 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java > e12211f867 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java > a375723f12 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsInfoResponse.java > 8950b81929 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetOpenTxnsResponse.java > b7e7d4b532 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java > 8d8ce6dda7 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java > 10b1d41175 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetValidWriteIdsRequest.java > c22778d6d1 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetValidWriteIdsResponse.java > 90d0b9db0b > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/HeartbeatTxnRangeResponse.java > e069524048 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java > 2b823a0909 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/LockRequest.java > 5a9a0e892b > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Materialization.java > dc6dc0d560 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java > 2037590a12 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/OpenTxnsResponse.java > de60105a71 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionValuesRequest.java > bb9bc516f5 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionValuesResponse.java > a98f748d9e > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionValuesRow.java > b8a55e96d4 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsByExprResult.java > 521b68d202 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsRequest.java > c1d93371d6 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PartitionsStatsResult.java > 2bf7fc541d > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java > 0b0da1453c > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RequestPartsSpec.java > 5639a98d86 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SQLCheckConstraint.java > PRE-CREATION > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SchemaVersion.java > fb0be40fba > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowCompactResponse.java > a189d0ab25 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ShowLocksResponse.java > 37155095a4 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsRequest.java > ccab0e166f > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableStatsResult.java > 4e8c5b25b4 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/TableValidWriteIds.java > aab0aa38bc > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java > 8c5ceafb25 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java > 2fc0e00d6c > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetAllResourcePlanResponse.java > 0ddb2b2dff > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetTriggersForResourePlanResponse.java > 93fa2b7ee9 > > standalone-metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMValidateResourcePlanResponse.java > 97d33c1da4 > > standalone-metastore/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php > efe693a65e > standalone-metastore/src/gen/thrift/gen-php/metastore/Types.php 6e3ec622cc > > standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote > a8e83863f7 > > standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py > 30214d8df8 > standalone-metastore/src/gen/thrift/gen-py/hive_metastore/ttypes.py > 486f0612b9 > standalone-metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb > dd7467c503 > standalone-metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb > e6787c1e02 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 3a0e7608e6 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 1755700b0e > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > f1d5066657 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java > 6ead20aeaf > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java > 88d88ed4df > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java > ad4af1a9df > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java > d37b201424 > > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MConstraint.java > 8c7f57fdc1 > standalone-metastore/src/main/thrift/hive_metastore.thrift ef63eabe44 > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java > e9527c72ef > > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java > 8fc0c83788 > > > Diff: https://reviews.apache.org/r/66183/diff/1/ > > > Testing > ------- > > > Thanks, > > Vineet Garg > >
