subject:"Review Request 72423\: HIVE\-23089"

Re: Review Request 72423: HIVE-23089

2020-04-28 Thread Krisztian Kasa


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

(Updated April 28, 2020, 12:40 p.m.)


Review request for hive, Jesús Camacho Rodríguez and Vineet Garg.


Bugs: HIVE-23089
https://issues.apache.org/jira/browse/HIVE-23089


Repository: hive-git


Description
---

Add constraint checks to CBO plan


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java f94a9f9d70 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java 
3e3d331412 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java 
c9131ec018 
  ql/src/test/results/clientnegative/update_notnull_constraint.q.out 32905378e7 
  ql/src/test/results/clientpositive/llap/check_constraint.q.out bc5d361859 
  ql/src/test/results/clientpositive/llap/default_constraint.q.out a04da4e2b4 
  ql/src/test/results/clientpositive/llap/enforce_constraint_notnull.q.out 
e96363fe29 


Diff: https://reviews.apache.org/r/72423/diff/5/

Changes: https://reviews.apache.org/r/72423/diff/4-5/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver -Dqfile=check_constraint.q,sort_acid.q -pl 
itests/qtest -Pitests
mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestNegativeCliDriver 
-Dqfile=update_notnull_constraint.q -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa

Re: Review Request 72423: HIVE-23089

2020-04-27 Thread Krisztian Kasa


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

(Updated April 27, 2020, 5:29 p.m.)


Review request for hive, Jesús Camacho Rodríguez and Vineet Garg.


Bugs: HIVE-23089
https://issues.apache.org/jira/browse/HIVE-23089


Repository: hive-git


Description
---

Add constraint checks to CBO plan


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java f94a9f9d70 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java 
3e3d331412 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java 
c9131ec018 
  ql/src/test/results/clientnegative/update_notnull_constraint.q.out 32905378e7 
  ql/src/test/results/clientpositive/llap/check_constraint.q.out bc5d361859 
  ql/src/test/results/clientpositive/llap/default_constraint.q.out a04da4e2b4 
  ql/src/test/results/clientpositive/llap/enforce_constraint_notnull.q.out 
e96363fe29 


Diff: https://reviews.apache.org/r/72423/diff/4/

Changes: https://reviews.apache.org/r/72423/diff/3-4/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver -Dqfile=check_constraint.q,sort_acid.q -pl 
itests/qtest -Pitests
mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestNegativeCliDriver 
-Dqfile=update_notnull_constraint.q -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa

Re: Review Request 72423: HIVE-23089

2020-04-26 Thread Krisztian Kasa


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

(Updated April 27, 2020, 5:40 a.m.)


Review request for hive, Jesús Camacho Rodríguez and Vineet Garg.


Bugs: HIVE-23089
https://issues.apache.org/jira/browse/HIVE-23089


Repository: hive-git


Description
---

Add constraint checks to CBO plan


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java f94a9f9d70 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java 
3e3d331412 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java 
c9131ec018 
  ql/src/test/results/clientnegative/update_notnull_constraint.q.out 32905378e7 
  ql/src/test/results/clientpositive/llap/check_constraint.q.out bc5d361859 
  ql/src/test/results/clientpositive/llap/default_constraint.q.out a04da4e2b4 
  ql/src/test/results/clientpositive/llap/enforce_constraint_notnull.q.out 
e96363fe29 


Diff: https://reviews.apache.org/r/72423/diff/3/

Changes: https://reviews.apache.org/r/72423/diff/2-3/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver -Dqfile=check_constraint.q,sort_acid.q -pl 
itests/qtest -Pitests
mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestNegativeCliDriver 
-Dqfile=update_notnull_constraint.q -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa

Re: Review Request 72423: HIVE-23089

2020-04-24 Thread Krisztian Kasa


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

(Updated April 24, 2020, 7:03 a.m.)


Review request for hive, Jesús Camacho Rodríguez and Vineet Garg.


Bugs: HIVE-23089
https://issues.apache.org/jira/browse/HIVE-23089


Repository: hive-git


Description
---

Add constraint checks to CBO plan


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 7b2e201e5a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java 
3e3d331412 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java 
c9131ec018 
  ql/src/test/results/clientnegative/update_notnull_constraint.q.out 32905378e7 
  ql/src/test/results/clientpositive/llap/check_constraint.q.out bc5d361859 


Diff: https://reviews.apache.org/r/72423/diff/2/

Changes: https://reviews.apache.org/r/72423/diff/1-2/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver -Dqfile=check_constraint.q,sort_acid.q -pl 
itests/qtest -Pitests
mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestNegativeCliDriver 
-Dqfile=update_notnull_constraint.q -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa

Re: Review Request 72423: HIVE-23089

2020-04-23 Thread Jesús Camacho Rodríguez


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




ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 3593 (patched)


Can we add a comment to this method ('returns null if there are not 
constraints')?



ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
Lines 3594 (patched)


nit. Indentation seems off.



ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java
Lines 40 (patched)


Please, add a short comment to this class.



ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java
Lines 55 (patched)


nit. unnecessary newline



ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java
Lines 104 (patched)


Instead of creating a new method `createColumnRefExpr` in the 
`TypeCheckProcFactory` (we need to simplify that factory rather than continue 
adding to it), let's just call `typeCheckProcFactory.exprFactory.toExpr()` 
since we have access (and we have already parsed the columns).



ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java
Lines 80 (patched)


Instead of returning the generator, we should just return a ExprNodeDesc 
(to make it consistent with other methods in this class). Additionally, that 
way we do not need to expose the generator externally and the utility method 
would be more self contained.



ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java
Lines 71 (patched)


Instead of returning the generator, we should just return a RexNode (to 
make it consistent with other methods in this class).



ql/src/java/org/apache/hadoop/hive/ql/parse/type/TypeCheckProcFactory.java
Lines 1458 (patched)





- Jesús Camacho Rodríguez


On April 23, 2020, 1:47 p.m., Krisztian Kasa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72423/
> ---
> 
> (Updated April 23, 2020, 1:47 p.m.)
> 
> 
> Review request for hive, Jesús Camacho Rodríguez and Vineet Garg.
> 
> 
> Bugs: HIVE-23089
> https://issues.apache.org/jira/browse/HIVE-23089
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Add constraint checks to CBO plan
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 7b2e201e5a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> 0de3730351 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java 
> 3e3d331412 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java 
> c9131ec018 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/type/TypeCheckProcFactory.java 
> e16966e999 
>   ql/src/test/results/clientnegative/update_notnull_constraint.q.out 
> 32905378e7 
>   ql/src/test/results/clientpositive/llap/check_constraint.q.out bc5d361859 
> 
> 
> Diff: https://reviews.apache.org/r/72423/diff/1/
> 
> 
> Testing
> ---
> 
> mvn test -Dtest.output.overwrite -DskipSparkTests 
> -Dtest=TestMiniLlapLocalCliDriver -Dqfile=check_constraint.q,sort_acid.q -pl 
> itests/qtest -Pitests
> mvn test -Dtest.output.overwrite -DskipSparkTests 
> -Dtest=TestNegativeCliDriver -Dqfile=update_notnull_constraint.q -pl 
> itests/qtest -Pitests
> 
> 
> Thanks,
> 
> Krisztian Kasa
> 
>

Review Request 72423: HIVE-23089

2020-04-23 Thread Krisztian Kasa


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

Review request for hive, Jesús Camacho Rodríguez and Vineet Garg.


Bugs: HIVE-23089
https://issues.apache.org/jira/browse/HIVE-23089


Repository: hive-git


Description
---

Add constraint checks to CBO plan


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 7b2e201e5a 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0de3730351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ConstraintExprGenerator.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/ExprNodeTypeCheck.java 
3e3d331412 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/RexNodeTypeCheck.java 
c9131ec018 
  ql/src/java/org/apache/hadoop/hive/ql/parse/type/TypeCheckProcFactory.java 
e16966e999 
  ql/src/test/results/clientnegative/update_notnull_constraint.q.out 32905378e7 
  ql/src/test/results/clientpositive/llap/check_constraint.q.out bc5d361859 


Diff: https://reviews.apache.org/r/72423/diff/1/


Testing
---

mvn test -Dtest.output.overwrite -DskipSparkTests 
-Dtest=TestMiniLlapLocalCliDriver -Dqfile=check_constraint.q,sort_acid.q -pl 
itests/qtest -Pitests
mvn test -Dtest.output.overwrite -DskipSparkTests -Dtest=TestNegativeCliDriver 
-Dqfile=update_notnull_constraint.q -pl itests/qtest -Pitests


Thanks,

Krisztian Kasa

Re: Review Request 72423: HIVE-23089

Re: Review Request 72423: HIVE-23089

Re: Review Request 72423: HIVE-23089

Re: Review Request 72423: HIVE-23089

Re: Review Request 72423: HIVE-23089

Review Request 72423: HIVE-23089

6 matches

Site Navigation

Mail list logo

Footer information