[ 
https://issues.apache.org/jira/browse/HIVE-24778?focusedWorklogId=553520&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-553520
 ]

ASF GitHub Bot logged work on HIVE-24778:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 17/Feb/21 10:37
            Start Date: 17/Feb/21 10:37
    Worklog Time Spent: 10m 
      Work Description: zabetak commented on a change in pull request #1982:
URL: https://github.com/apache/hive/pull/1982#discussion_r577501389



##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFTimestamp.java
##########
@@ -84,12 +84,12 @@ public ObjectInspector initialize(ObjectInspector[] 
arguments) throws UDFArgumen
           "The function TIMESTAMP takes only primitive types");
     }
 
-    if (ss != null && 
ss.getConf().getBoolVar(ConfVars.HIVE_STRICT_TIMESTAMP_CONVERSION)) {
+    if (ss != null && 
ss.getConf().getBoolVar(ConfVars.HIVE_STRICT_CHECKS_TYPE_SAFETY)) {
       PrimitiveCategory category = argumentOI.getPrimitiveCategory();
       PrimitiveGrouping group = 
PrimitiveObjectInspectorUtils.getPrimitiveGrouping(category);
       if (group == PrimitiveGrouping.NUMERIC_GROUP) {
         throw new UDFArgumentException(
-            "Casting NUMERIC types to TIMESTAMP is prohibited (" + 
ConfVars.HIVE_STRICT_TIMESTAMP_CONVERSION + ")");
+            "Casting NUMERIC types to TIMESTAMP is prohibited (" + 
ConfVars.HIVE_STRICT_CHECKS_TYPE_SAFETY + ")");
       }

Review comment:
       Should this be here or rather in 
`TypeCheckProcFactory.DefaultExprProcessor#validateUDF`?

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/udf/TimestampCastRestrictorResolver.java
##########
@@ -45,7 +45,7 @@
   public TimestampCastRestrictorResolver(UDFMethodResolver parentResolver) {
     this.parentResolver = parentResolver;
     SessionState ss = SessionState.get();
-    if (ss != null && 
ss.getConf().getBoolVar(ConfVars.HIVE_STRICT_TIMESTAMP_CONVERSION)) {
+    if (ss != null && 
ss.getConf().getBoolVar(ConfVars.HIVE_STRICT_CHECKS_TYPE_SAFETY)) {

Review comment:
       If I understand well this class is used to restrict casting 
timestamp/date to boolean, double, byte, float, integer, long, short values. I 
am not sure why we should deal with these checks at this point but I if we 
really need this then I guess it makes sense to extend it so that we apply the 
same checks for all types under `hive.strict.checks.type.safety` property. 
Should we create another JIRA for this?

##########
File path: 
ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java
##########
@@ -166,10 +166,10 @@ protected void checkConversionAllowed(ObjectInspector 
argOI, ObjectInspector com
       return;
     }
     SessionState ss = SessionState.get();
-    if (ss != null && 
ss.getConf().getBoolVar(ConfVars.HIVE_STRICT_TIMESTAMP_CONVERSION)) {
+    if (ss != null && 
ss.getConf().getBoolVar(ConfVars.HIVE_STRICT_CHECKS_TYPE_SAFETY)) {
       if (primitiveGroupOf(compareOI) == PrimitiveGrouping.NUMERIC_GROUP) {
         throw new UDFArgumentException(
-            "Casting DATE/TIMESTAMP to NUMERIC is prohibited (" + 
ConfVars.HIVE_STRICT_TIMESTAMP_CONVERSION + ")");
+            "Casting DATE/TIMESTAMP to NUMERIC is prohibited (" + 
ConfVars.HIVE_STRICT_CHECKS_TYPE_SAFETY + ")");

Review comment:
       Why do we need this `checkConversionAllowed` method? If conversion is 
incompatible/dangerous shouldn't this be caught by 
`TypeCheckProcFactory.DefaultExprProcessor#validateUDF`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 553520)
    Time Spent: 20m  (was: 10m)

> Unify hive.strict.timestamp.conversion and hive.strict.checks.type.safety 
> properties
> ------------------------------------------------------------------------------------
>
>                 Key: HIVE-24778
>                 URL: https://issues.apache.org/jira/browse/HIVE-24778
>             Project: Hive
>          Issue Type: Sub-task
>    Affects Versions: 4.0.0
>            Reporter: Stamatis Zampetakis
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 4.0.0
>
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> The majority of strict type checks can be controlled by 
> {{hive.strict.checks.type.safety}} property. HIVE-24157 introduced another 
> property, namely  {{hive.strict.timestamp.conversion}}, to control the 
> implicit comparisons between numerics and timestamps.
> The name and description of {{hive.strict.checks.type.safety}} imply that the 
> property covers all strict checks so having others for specific cases appears 
> confusing and can easily lead to unexpected behavior.
> The goal of this issue is to unify those properties to facilitate 
> configuration and improve code reuse.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to