Re: Review Request 66183: HIVE-18953 Implement CHECK constraint

2018-03-21 Thread Vineet Garg

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

(Updated March 22, 2018, 12:05 a.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 (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 e2244a1d4e 
  itests/src/test/resources/testconfiguration.properties 08de4a79f3 
  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/IdentifiersParser.g 2bba33f6bd 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 36f6bcd069 
  
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_qual_name.q PRE-CREATION 
  ql/src/test/queries/clientnegative/check_constraint_subquery.q PRE-CREATION 
  ql/src/test/queries/clientnegative/check_constraint_temporary_udf.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/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_nonboolean_expr.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_temporary_udf.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 
  

Re: Review Request 66183: HIVE-18953 Implement CHECK constraint

2018-03-21 Thread Vineet Garg

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

(Updated March 21, 2018, 10:33 p.m.)


Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez.


Changes
---

Addressed review comments


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 (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 e2244a1d4e 
  itests/src/test/resources/testconfiguration.properties 08de4a79f3 
  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/IdentifiersParser.g 2bba33f6bd 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 36f6bcd069 
  
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_qual_name.q PRE-CREATION 
  ql/src/test/queries/clientnegative/check_constraint_subquery.q PRE-CREATION 
  ql/src/test/queries/clientnegative/check_constraint_temporary_udf.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/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_nonboolean_expr.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_temporary_udf.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 
  

Re: Review Request 66183: HIVE-18953 Implement CHECK constraint

2018-03-20 Thread Ashutosh Chauhan

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


Can a single constraint name refer to multiple constraints?



ql/src/java/org/apache/hadoop/hive/ql/metadata/CheckConstraint.java
Lines 52 (patched)


I suppose you mean check expression?



ql/src/java/org/apache/hadoop/hive/ql/metadata/CheckConstraint.java
Lines 69 (patched)


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)


Add this in IdentifiersParser.g to make it non-reserved keyword.



ql/src/test/queries/clientnegative/check_constraint_permanent_udf.q
Lines 1 (patched)


you want to rename this test file.



ql/src/test/queries/clientnegative/create_external_with_default_constraint.q
Line 1 (original), 1 (patched)


This is already covered in previous test. you may want to undo this.



ql/src/test/queries/clientpositive/check_constraint.q
Lines 1 (patched)


Any reason for this?



ql/src/test/queries/clientpositive/check_constraint.q
Lines 41 (patched)


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)


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)


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)


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)


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 
>   

Review Request 66183: HIVE-18953 Implement CHECK constraint

2018-03-20 Thread Vineet Garg

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

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