You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Jason Dere <jd...@hortonworks.com> on 2018/01/10 23:19:38 UTC

Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

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

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


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


Repository: hive-git


Description
-------

Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().

current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
  ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
  ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
  ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
  ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 


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


Testing
-------


Thanks,

Jason Dere


Re: Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.

> On Jan. 17, 2018, 5:55 a.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/65084/diff/2/?file=1939769#file1939769line72>
> >
> >     1) I think this should be _true_ by default, similar to _deterministic_. This change is in Hive 3.0, hence we just need to document it properly, but backwards compatibility should not be an issue.
> >     
> >     2) _deterministic_ functions should always be _runtimeConstant_, correct? Could this be enforced?
> 
> Jason Dere wrote:
>     No, runtime constants refer to functions like CURRENT_DATE/CURRENT_TIMESTAMP/CURRENT_DATABASE()/CURRENT_USER(), where the function value will remain the same during the life of a single query (which allows them to be constant folded). But these runtime constants are not determinstic, because these functions do not always return the same value (CURRENT_USER() returns a different value for each user, CURRENT_TIMESTAMP returns a different value every query).
>     
>     For that reason, I believe the runtimeConstant property should default to false - only the functions above should override this value as true. This property being true should distinguish this function from a deterministic function.
>     - Both determinstic functions and runtime constants can be constant folded.
>     - Only deterministic functions should be allowed for query materialization.

I think my question was not correctly formulated. A _deterministic_ function will always return the same value, thus it is _runtimeConstant_? Or in another way, is there any function for which _deterministic=true_ and _runtimeConstant=false_?


- Jesús


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


On Jan. 12, 2018, 7:33 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65084/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2018, 7:33 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez.
> 
> 
> Bugs: HIVE-18430
>     https://issues.apache.org/jira/browse/HIVE-18430
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().
> 
> current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
> Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b0a2da8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 
> 
> 
> Diff: https://reviews.apache.org/r/65084/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Re: Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

Posted by Jason Dere <jd...@hortonworks.com>.

> On Jan. 17, 2018, 5:55 a.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/65084/diff/2/?file=1939769#file1939769line72>
> >
> >     1) I think this should be _true_ by default, similar to _deterministic_. This change is in Hive 3.0, hence we just need to document it properly, but backwards compatibility should not be an issue.
> >     
> >     2) _deterministic_ functions should always be _runtimeConstant_, correct? Could this be enforced?

No, runtime constants refer to functions like CURRENT_DATE/CURRENT_TIMESTAMP/CURRENT_DATABASE()/CURRENT_USER(), where the function value will remain the same during the life of a single query (which allows them to be constant folded). But these runtime constants are not determinstic, because these functions do not always return the same value (CURRENT_USER() returns a different value for each user, CURRENT_TIMESTAMP returns a different value every query).

For that reason, I believe the runtimeConstant property should default to false - only the functions above should override this value as true. This property being true should distinguish this function from a deterministic function.
- Both determinstic functions and runtime constants can be constant folded.
- Only deterministic functions should be allowed for query materialization.


- Jason


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


On Jan. 12, 2018, 7:33 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65084/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2018, 7:33 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez.
> 
> 
> Bugs: HIVE-18430
>     https://issues.apache.org/jira/browse/HIVE-18430
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().
> 
> current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
> Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b0a2da8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 
> 
> 
> Diff: https://reviews.apache.org/r/65084/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Re: Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

Posted by Jason Dere <jd...@hortonworks.com>.

> On Jan. 17, 2018, 5:55 a.m., Jesús Camacho Rodríguez wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java
> > Lines 72 (patched)
> > <https://reviews.apache.org/r/65084/diff/2/?file=1939769#file1939769line72>
> >
> >     1) I think this should be _true_ by default, similar to _deterministic_. This change is in Hive 3.0, hence we just need to document it properly, but backwards compatibility should not be an issue.
> >     
> >     2) _deterministic_ functions should always be _runtimeConstant_, correct? Could this be enforced?
> 
> Jason Dere wrote:
>     No, runtime constants refer to functions like CURRENT_DATE/CURRENT_TIMESTAMP/CURRENT_DATABASE()/CURRENT_USER(), where the function value will remain the same during the life of a single query (which allows them to be constant folded). But these runtime constants are not determinstic, because these functions do not always return the same value (CURRENT_USER() returns a different value for each user, CURRENT_TIMESTAMP returns a different value every query).
>     
>     For that reason, I believe the runtimeConstant property should default to false - only the functions above should override this value as true. This property being true should distinguish this function from a deterministic function.
>     - Both determinstic functions and runtime constants can be constant folded.
>     - Only deterministic functions should be allowed for query materialization.
> 
> Jesús Camacho Rodríguez wrote:
>     I think my question was not correctly formulated. A _deterministic_ function will always return the same value, thus it is _runtimeConstant_? Or in another way, is there any function for which _deterministic=true_ and _runtimeConstant=false_?

Most of the Hive functions are set as deterministic=true and runtimeConstant=false. All arithmetic/mathematic functions, string functions, etc. Literally the only functions that should have runtimeConstant=true should be CURRENT_DATE/CURRENT_TIMESTAMP/CURRENT_DATABASE()/CURRENT_USER()


- Jason


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


On Jan. 12, 2018, 7:33 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65084/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2018, 7:33 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez.
> 
> 
> Bugs: HIVE-18430
>     https://issues.apache.org/jira/browse/HIVE-18430
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().
> 
> current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
> Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b0a2da8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 
> 
> 
> Diff: https://reviews.apache.org/r/65084/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Re: Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65084/#review195531
-----------------------------------------------------------




ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java
Lines 72 (patched)
<https://reviews.apache.org/r/65084/#comment274737>

    1) I think this should be _true_ by default, similar to _deterministic_. This change is in Hive 3.0, hence we just need to document it properly, but backwards compatibility should not be an issue.
    
    2) _deterministic_ functions should always be _runtimeConstant_, correct? Could this be enforced?


- Jesús Camacho Rodríguez


On Jan. 12, 2018, 7:33 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65084/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2018, 7:33 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez.
> 
> 
> Bugs: HIVE-18430
>     https://issues.apache.org/jira/browse/HIVE-18430
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().
> 
> current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
> Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b0a2da8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 
> 
> 
> Diff: https://reviews.apache.org/r/65084/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Re: Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

Posted by Jesús Camacho Rodríguez <jc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65084/#review195634
-----------------------------------------------------------


Ship it!




Ship It!

- Jesús Camacho Rodríguez


On Jan. 12, 2018, 7:33 p.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65084/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2018, 7:33 p.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez.
> 
> 
> Bugs: HIVE-18430
>     https://issues.apache.org/jira/browse/HIVE-18430
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().
> 
> current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
> Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b0a2da8 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
>   ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
>   ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
>   ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 
> 
> 
> Diff: https://reviews.apache.org/r/65084/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Re: Review Request 65084: HIVE-18430 Add new determinism category for runtime constants (current_date, current_timestamp)

Posted by Jason Dere <jd...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65084/
-----------------------------------------------------------

(Updated Jan. 12, 2018, 7:33 p.m.)


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


Changes
-------

Missed another use of the deterministic UDFType annotation in the constant propagation.


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


Repository: hive-git


Description
-------

Add a runtimeConstant field to UDFType. Also add a new method isConsistentWithinQuery() to FunctionRegistry/ExprNodeEvaluator, which indicates whether the UDF/expression returns a consistent value during the life of the query. Most existing calls to isDeterministic() should be switched to use isConsistentWithinQuery().

current_timestamp/current_date/current_user/current_database/logged_in_user are now tagged as non-determinstic, and runtime constants.
Jesus/Ashutosh: How does setting these functions to non-deterministic affect Calcite? It appears that for Calcite operators, there is a isDynamicFunction() which should return true in the case of runtime constants. Within Calcite should isDeterministic() return true or false for these functions?


Diffs (updated)
-----

  ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluator.java 375d65f 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeEvaluatorFactory.java cc40cae 
  ql/src/java/org/apache/hadoop/hive/ql/exec/ExprNodeGenericFuncEvaluator.java 8b9baa6 
  ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 7ca950d 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConstantPropagateProcFactory.java b0a2da8 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/PrunerExpressionOperatorFactory.java 306e714 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExceptRewriteRule.java 5b72dbd 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveIntersectRewriteRule.java a5d950a 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/PartitionPrune.java 0e5e2b9 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/SqlFunctionConverter.java 13ee4e5 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java 461dbe5 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ac37cc4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 5a88a96 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java 67ea32c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeDescUtils.java 067fbe0 
  ql/src/java/org/apache/hadoop/hive/ql/ppd/ExprWalkerProcFactory.java d4df1e8 
  ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java e265863 
  ql/src/java/org/apache/hadoop/hive/ql/udf/UDFType.java ac3ec58 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDF.java ef8dcf0 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentDate.java 1f027a2 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentTimestamp.java 2f13a22 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentUser.java d97583d 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFLoggedInUser.java 2915b86 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFMacro.java 3f505f2 
  ql/src/java/org/apache/hadoop/hive/ql/udf/generic/UDFCurrentDB.java a5bab4f 
  ql/src/test/org/apache/hadoop/hive/ql/exec/TestFunctionRegistry.java 3589938 


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

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


Testing
-------


Thanks,

Jason Dere