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