Hisoka-X commented on code in PR #42810:
URL: https://github.com/apache/spark/pull/42810#discussion_r1315586396
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Util.scala:
##########
@@ -213,7 +213,9 @@ private[sql] object CatalogV2Util {
// enforced by the parser). On the other hand, commands that drop
the default value such
// as "ALTER TABLE t ALTER COLUMN c DROP DEFAULT" will set this
string to empty.
if (update.newDefaultValue().nonEmpty) {
- Some(field.withCurrentDefaultValue(update.newDefaultValue()))
+ val result =
field.withCurrentDefaultValue(update.newDefaultValue())
+ ResolveDefaultColumns.analyze(result, "ALTER TABLE ALTER COLUMN")
Review Comment:
Hi, I find the `ALTER TABLE ADD COLUMN` check default expression in
https://github.com/apache/spark/blob/6c885a7cf57df328b03308cff2eed814bda156e4/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statements.scala#L144
when invoke `changes`
https://github.com/apache/spark/blob/6c885a7cf57df328b03308cff2eed814bda156e4/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2AlterTableCommands.scala#L123
from `AlterTable` in `DataSourceV2Strategy`. I think add check default
expression for `ALTER TABLE ALTER COLUMN` into `changes` of `AlterColumn` look
more reasonable. After this, when we match `AlterTableCommand` in
`DataSourceV2Strategy` will throw an exception. What do you think about this
way?
I already updated the code, you can check it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
