[SPARK-48081][SQL][3.4] Fix ClassCastException in NTile.checkInputDataTypes() when argument is non-foldable or of wrong type #46337
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hen argument is non-foldable or of wrong type ### What changes were proposed in this pull request? While migrating the `NTile` expression's type check failures to the new error class framework, PR apache#38457 removed a pair of not-unnecessary `return` statements and thus caused certain branches' values to be discarded rather than returned. As a result, invalid usages like ``` select ntile(99.9) OVER (order by id) from range(10) ``` trigger internal errors like errors like ``` java.lang.ClassCastException: class org.apache.spark.sql.types.Decimal cannot be cast to class java.lang.Integer (org.apache.spark.sql.types.Decimal is in unnamed module of loader 'app'; java.lang.Integer is in module java.base of loader 'bootstrap') at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99) at org.apache.spark.sql.catalyst.expressions.NTile.checkInputDataTypes(windowExpressions.scala:877) ``` instead of clear error framework errors like ``` org.apache.spark.sql.catalyst.ExtendedAnalysisException: [DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE] Cannot resolve "ntile(99.9)" due to data type mismatch: The first parameter requires the "INT" type, however "99.9" has the type "DECIMAL(3,1)". SQLSTATE: 42K09; line 1 pos 7; 'Project [unresolvedalias(ntile(99.9) windowspecdefinition(id#0L ASC NULLS FIRST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())))] +- Range (0, 10, step=1, splits=None) at org.apache.spark.sql.catalyst.analysis.package$AnalysisErrorAt.dataTypeMismatch(package.scala:73) at org.apache.spark.sql.catalyst.analysis.CheckAnalysis.$anonfun$checkAnalysis0$7(CheckAnalysis.scala:315) ``` ### Why are the changes needed? Improve error messages. ### Does this PR introduce _any_ user-facing change? Yes, it improves an error message. ### How was this patch tested? Added a new test case to AnalysisErrorSuite. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#46333 from JoshRosen/SPARK-48081. Authored-by: Josh Rosen <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun
approved these changes
May 2, 2024
Member
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM (Pending CIs).
Thank you!
dongjoon-hyun
pushed a commit
that referenced
this pull request
May 2, 2024
…aTypes() when argument is non-foldable or of wrong type branch-3.4 pick of PR #46333 , fixing test issue due to difference in expected error message parameter formatting across branches; original description follows below: --- ### What changes were proposed in this pull request? While migrating the `NTile` expression's type check failures to the new error class framework, PR #38457 removed a pair of not-unnecessary `return` statements and thus caused certain branches' values to be discarded rather than returned. As a result, invalid usages like ``` select ntile(99.9) OVER (order by id) from range(10) ``` trigger internal errors like errors like ``` java.lang.ClassCastException: class org.apache.spark.sql.types.Decimal cannot be cast to class java.lang.Integer (org.apache.spark.sql.types.Decimal is in unnamed module of loader 'app'; java.lang.Integer is in module java.base of loader 'bootstrap') at scala.runtime.BoxesRunTime.unboxToInt(BoxesRunTime.java:99) at org.apache.spark.sql.catalyst.expressions.NTile.checkInputDataTypes(windowExpressions.scala:877) ``` instead of clear error framework errors like ``` org.apache.spark.sql.catalyst.ExtendedAnalysisException: [DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE] Cannot resolve "ntile(99.9)" due to data type mismatch: The first parameter requires the "INT" type, however "99.9" has the type "DECIMAL(3,1)". SQLSTATE: 42K09; line 1 pos 7; 'Project [unresolvedalias(ntile(99.9) windowspecdefinition(id#0L ASC NULLS FIRST, specifiedwindowframe(RowFrame, unboundedpreceding$(), currentrow$())))] +- Range (0, 10, step=1, splits=None) at org.apache.spark.sql.catalyst.analysis.package$AnalysisErrorAt.dataTypeMismatch(package.scala:73) at org.apache.spark.sql.catalyst.analysis.CheckAnalysis.$anonfun$checkAnalysis0$7(CheckAnalysis.scala:315) ``` ### Why are the changes needed? Improve error messages. ### Does this PR introduce _any_ user-facing change? Yes, it improves an error message. ### How was this patch tested? Added a new test case to AnalysisErrorSuite. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46337 from JoshRosen/SPARK-48081-branch-3.4. Authored-by: Josh Rosen <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
Member
|
Merged to branch-3.4. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
branch-3.4 pick of PR #46333 , fixing test issue due to difference in expected error message parameter formatting across branches; original description follows below:
What changes were proposed in this pull request?
While migrating the
NTileexpression's type check failures to the new error class framework, PR #38457 removed a pair of not-unnecessaryreturnstatements and thus caused certain branches' values to be discarded rather than returned.As a result, invalid usages like
trigger internal errors like errors like
instead of clear error framework errors like
Why are the changes needed?
Improve error messages.
Does this PR introduce any user-facing change?
Yes, it improves an error message.
How was this patch tested?
Added a new test case to AnalysisErrorSuite.
Was this patch authored or co-authored using generative AI tooling?
No.