hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Wilfong" <kevinwilf...@fb.com>
Subject Re: Review Request: Warn user that precision is lost when bigint is implicitly cast to double.
Date Wed, 17 Aug 2011 18:34:44 GMT

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

(Updated 2011-08-17 18:34:44.253584)


Review request for hive and Siying Dong.


Changes
-------

This diff is in response to Siying's comments.

I'm a little hesitant to move the check into the method getFuncExprNodeDesc for two reasons:

1) if the name of the UDF the method is called on corresponds to a comparison operator the
code I modified (and hence the check in GenericUDFBaseCompare.ObjectInspector.initialize())
is already called 
    DefaultExprProcessor.getFuncExprNodeDesc -> ExprNodeGenericFuncDesc.newInstance ->
GenericUDFBaseCompare.ObjectInspector.initialize

2) I would have to write new code in getFuncExprNodeDesc to check if  the GenericUDF is of
type GenericUDFBaseCompare, and if so, get the TypeInfo for from each of its arguments.  Admittedly,
this is only a few lines, but we get this for free by putting the check in GenericUDFBaseCompare.ObjectInspector.initialize

If you still feel strongly about this, or you have other concerns, let me know and I will
move the check, I just wanted to make those points before doing it.

I didn't realize we wanted to print the warning to the console.  There is a function LogHelper.printError
which checks uses SessionState's error output stream if possible and System.err otherwise
and logs the message, which I have decided to use.


Summary
-------

I added a check in the code for equality expressions (includes inequalities) with operands
of different types, that throws an error or logs a warning, depending on strict mode, if one
operand is a string or double and the other is a bigint.


This addresses bug HIVE-2378.
    https://issues.apache.org/jira/browse/HIVE-2378


Diffs (updated)
-----

  trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/ErrorMsg.java 1158835 
  trunk/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBaseCompare.java 1158835

  trunk/ql/src/test/queries/clientnegative/compare_double_bigint.q PRE-CREATION 
  trunk/ql/src/test/queries/clientnegative/compare_string_bigint.q PRE-CREATION 
  trunk/ql/src/test/results/clientnegative/compare_double_bigint.q.out PRE-CREATION 
  trunk/ql/src/test/results/clientnegative/compare_string_bigint.q.out PRE-CREATION 

Diff: https://reviews.apache.org/r/1515/diff


Testing
-------

I added two tests (one for strings and one for doubles) to record the issue.

I also verified the unit tests still run.


Thanks,

Kevin


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message