-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-3501] [SQL] Fix the bug of Hive SimpleUDF creates unnecessary type cast #2368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tion in constant folding
9bb678c to
b834ed4
Compare
|
test this please. |
|
QA tests have started for PR 2368 at commit
|
|
QA tests have finished for PR 2368 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'd structure this as a guard to call out the two cases more clearly:
case (e, t) if (e.dataType == t) => e
case (e, t) => Cast(e, t)|
Thanks for fixing this! While I think eliminating the problem is great, it also seems like a bug to me that casting a timestamp to a timestamp throws an exception since none of the other datatypes do that. Mind adding a no-op case to the |
|
QA tests have started for PR 2368 at commit
|
|
QA tests have started for PR 2368 at commit
|
|
Tests timed out after a configured wait of |
|
Tests timed out after a configured wait of |
|
retest this please. |
|
QA tests have started for PR 2368 at commit
|
|
Tests timed out after a configured wait of |
3f15731 to
49dfc50
Compare
|
QA tests have started for PR 2368 at commit
|
|
QA tests have finished for PR 2368 at commit
|
|
You're right @marmbrus . the expression should work even without being optimized. I've updated the no-op case in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps:
private[this] lazy val cast: Any => Any = dataType match {
case dt if dt == child.dataType => identity[Any] _
case StringType => castToString
case BinaryType => castToBinary
case DecimalType => castToDecimal
case TimestampType => castToTimestamp
case BooleanType => castToBoolean
case ByteType => castToByte
case ShortType => castToShort
case IntegerType => castToInt
case FloatType => castToFloat
case LongType => castToLong
case DoubleType => castToDouble
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @marmbrus , it's more clean. :)
|
Minor style suggestion, otherwise LGTM. Thanks! |
|
QA tests have started for PR 2368 at commit
|
|
QA tests have finished for PR 2368 at commit
|
|
Thanks! Merged to master. |
When do the query like:
SparkSQL will raise exception: