-
Notifications
You must be signed in to change notification settings - Fork 28.3k
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-45075][SQL] Fix alter table with invalid default value will not report error #42810
[SPARK-45075][SQL] Fix alter table with invalid default value will not report error #42810
Conversation
checkError( | ||
exception = intercept[AnalysisException] { | ||
sql("alter table t add column s bigint default badvalue") | ||
}, | ||
errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION", | ||
parameters = Map( | ||
"statement" -> "ALTER TABLE", | ||
"colName" -> "`s`", | ||
"defaultValue" -> "badvalue")) |
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.
This test case will report error normally before this PR, but we don't have any test case to prove it.
checkError( | ||
exception = intercept[AnalysisException] { | ||
sql("alter table t alter column s set default badvalue") | ||
}, | ||
errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION", | ||
parameters = Map( | ||
"statement" -> "ALTER TABLE ALTER COLUMN", | ||
"colName" -> "`s`", | ||
"defaultValue" -> "badvalue")) |
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.
Add this test case to prove the new change worked. But the more negative case in sql/core/src/test/scala/org/apache/spark/sql/sources/InsertSuite.scala
and only worked for V1, after this PR merged, I will move it to sql/core/src/test/scala/org/apache/spark/sql/SQLInsertTestSuite.scala
in #42802
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala
Outdated
Show resolved
Hide resolved
...talyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
Outdated
Show resolved
Hide resolved
...talyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
Show resolved
Hide resolved
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.
Thanks for the fix!
...talyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
Outdated
Show resolved
Hide resolved
...talyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala
Show resolved
Hide resolved
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.
+1, LGTM. Thank you, @Hisoka-X , @dtenedor , @cloud-fan .
…t report error ### What changes were proposed in this pull request? This PR make sure ALTER TABLE ALTER COLUMN with invalid default value on DataSource V2 will report error, before this PR it will alter sucess. ### Why are the changes needed? Fix the error behavior on DataSource V2 with ALTER TABLE statement. ### Does this PR introduce _any_ user-facing change? Yes, the invalid default value will report error. ### How was this patch tested? Add new test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #42810 from Hisoka-X/SPARK-45075_alter_invalid_default_value_on_v2. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 4dd4737) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
…t report error ### What changes were proposed in this pull request? This PR make sure ALTER TABLE ALTER COLUMN with invalid default value on DataSource V2 will report error, before this PR it will alter sucess. ### Why are the changes needed? Fix the error behavior on DataSource V2 with ALTER TABLE statement. ### Does this PR introduce _any_ user-facing change? Yes, the invalid default value will report error. ### How was this patch tested? Add new test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #42810 from Hisoka-X/SPARK-45075_alter_invalid_default_value_on_v2. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 4dd4737) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Merged to master/3.5/3.4. |
Thanks @cloud-fan @dongjoon-hyun @dtenedor |
Oh, I realized that branch-3.4 fails. It seems that there is a behavior difference in branch-3.4. Let me revert this from branch-3.4. Could you make a backporting PR to branch-3.4, @Hisoka-X ? |
This is reverted via e3b8402 from branch-3.4 for now. |
…ll not report error ### What changes were proposed in this pull request? This is a backporting PR to branch-3.4 from #42810 Changed the way of assert the error to adapt to 3.4 ### Why are the changes needed? Fix bug on 3.4 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? add new test. ### Was this patch authored or co-authored using generative AI tooling? No Closes #42876 from Hisoka-X/SPARK-45075_followup_3.4_alter_column. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
…t report error ### What changes were proposed in this pull request? This PR make sure ALTER TABLE ALTER COLUMN with invalid default value on DataSource V2 will report error, before this PR it will alter sucess. ### Why are the changes needed? Fix the error behavior on DataSource V2 with ALTER TABLE statement. ### Does this PR introduce _any_ user-facing change? Yes, the invalid default value will report error. ### How was this patch tested? Add new test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#42810 from Hisoka-X/SPARK-45075_alter_invalid_default_value_on_v2. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 4dd4737) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
…ll not report error ### What changes were proposed in this pull request? This is a backporting PR to branch-3.4 from apache#42810 Changed the way of assert the error to adapt to 3.4 ### Why are the changes needed? Fix bug on 3.4 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? add new test. ### Was this patch authored or co-authored using generative AI tooling? No Closes apache#42876 from Hisoka-X/SPARK-45075_followup_3.4_alter_column. Authored-by: Jia Fan <fanjiaeminem@qq.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
What changes were proposed in this pull request?
This PR make sure ALTER TABLE ALTER COLUMN with invalid default value on DataSource V2 will report error, before this PR it will alter sucess.
Why are the changes needed?
Fix the error behavior on DataSource V2 with ALTER TABLE statement.
Does this PR introduce any user-facing change?
Yes, the invalid default value will report error.
How was this patch tested?
Add new test.
Was this patch authored or co-authored using generative AI tooling?
No.