You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "szehon-ho (via GitHub)" <gi...@apache.org> on 2025/04/23 19:00:51 UTC
[PR] [SPARK-51119][SQL][FOLLOW-UP] Fix missing fallback case for parsing corrupt exists_default value [spark]
szehon-ho opened a new pull request, #50685:
URL: https://github.com/apache/spark/pull/50685
### What changes were proposed in this pull request?
Add another fallback for broken (non-resolved) exists_default values for SPARK-51119 original fix.
### Why are the changes needed?
https://github.com/apache/spark/pull/49962 added a fallback in case there were already broken (ie, non-resolved) persisted default values in catalogs. A broken one is something like 'current_database, current_user, current_timestamp' , these are non-deterministic and will bring wrong results in EXISTS_DEFAULT, where user expects the value resolved when they set the default.
However, this fallback missed one case when the current_xxx is in a cast. This fixes it.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Add to existing unit test in StructTyepSuite
### Was this patch authored or co-authored using generative AI tooling?
No
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51119][SQL][FOLLOW-UP] Fix missing fallback case for parsing corrupt exists_default value [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #50685:
URL: https://github.com/apache/spark/pull/50685#issuecomment-2825931766
thanks, merging to master/4.0!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51119][SQL][FOLLOW-UP] Fix missing fallback case for parsing corrupt exists_default value [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #50685:
URL: https://github.com/apache/spark/pull/50685#discussion_r2056939147
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala:
##########
@@ -376,7 +376,7 @@ object ResolveDefaultColumns extends QueryErrorsBase
val expr = Literal.fromSQL(defaultSQL) match {
// EXISTS_DEFAULT will have a cast from analyze() due to coerceDefaultValue
Review Comment:
Yes in the happy path.
There are unfortunately bugs from before though, where column exists_defaults were persisted as unresolved, hence the fallback mechanism.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51119][SQL][FOLLOW-UP] Fix missing fallback case for parsing corrupt exists_default value [spark]
Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on code in PR #50685:
URL: https://github.com/apache/spark/pull/50685#discussion_r2056924586
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala:
##########
@@ -376,7 +376,7 @@ object ResolveDefaultColumns extends QueryErrorsBase
val expr = Literal.fromSQL(defaultSQL) match {
// EXISTS_DEFAULT will have a cast from analyze() due to coerceDefaultValue
Review Comment:
Don't we expect the cast to be evaluated before the exists default is persisted?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-51119][SQL][FOLLOW-UP] Fix missing fallback case for parsing corrupt exists_default value [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #50685: [SPARK-51119][SQL][FOLLOW-UP] Fix missing fallback case for parsing corrupt exists_default value
URL: https://github.com/apache/spark/pull/50685
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org