-
Notifications
You must be signed in to change notification settings - Fork 28k
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
[SPARK-42326][SQL] Integrate _LEGACY_ERROR_TEMP_2099
into UNSUPPORTED_DATATYPE
#39979
Conversation
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala
Outdated
Show resolved
Hide resolved
@@ -79,7 +79,7 @@ private[sql] object ArrowUtils { | |||
case ArrowType.Null.INSTANCE => NullType | |||
case yi: ArrowType.Interval if yi.getUnit == IntervalUnit.YEAR_MONTH => YearMonthIntervalType() | |||
case di: ArrowType.Duration if di.getUnit == TimeUnit.MICROSECOND => DayTimeIntervalType() | |||
case _ => throw QueryExecutionErrors.unsupportedDataTypeError(dt.toString) | |||
case _ => throw QueryExecutionErrors.unsupportedDataTypeError(dt.asInstanceOf[DataType]) |
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.
hmm, ArrowType
and DataType
are unrelated types, this fails in runtime I guess.
Just in case, do we have a test for the case.
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.
Yeah, you're right.
I added test case and new error class UNSUPPORTED_ARROWTYPE
to handle the ArrowType separately since I think they're technically different objects.
+1, LGTM. Merging to master/3.4. |
…TED_DATATYPE` ### What changes were proposed in this pull request? This PR proposes to integrate `_LEGACY_ERROR_TEMP_2099` into `UNSUPPORTED_DATATYPE`. And also introduce new error class `UNSUPPORTED_ARROWTYPE`. ### Why are the changes needed? We should assign proper name for LEGACY errors. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Updated UT. Closes #39979 from itholic/LEGACY_2099. Authored-by: itholic <haejoon.lee@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com> (cherry picked from commit 9855b13) Signed-off-by: Max Gekk <max.gekk@gmail.com>
…TED_DATATYPE` ### What changes were proposed in this pull request? This PR proposes to integrate `_LEGACY_ERROR_TEMP_2099` into `UNSUPPORTED_DATATYPE`. And also introduce new error class `UNSUPPORTED_ARROWTYPE`. ### Why are the changes needed? We should assign proper name for LEGACY errors. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Updated UT. Closes apache#39979 from itholic/LEGACY_2099. Authored-by: itholic <haejoon.lee@databricks.com> Signed-off-by: Max Gekk <max.gekk@gmail.com> (cherry picked from commit 9855b13) Signed-off-by: Max Gekk <max.gekk@gmail.com>
What changes were proposed in this pull request?
This PR proposes to integrate
_LEGACY_ERROR_TEMP_2099
intoUNSUPPORTED_DATATYPE
.And also introduce new error class
UNSUPPORTED_ARROWTYPE
.Why are the changes needed?
We should assign proper name for LEGACY errors.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Updated UT.