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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
