[SPARK-51119][SQL][FOLLOW-UP] Add fallback to ResolveDefaultColumnsUtil existenceDefaultValues #49962
Conversation
…il.existenceDefaultValues
szehon-ho
force-pushed
the
SPARK-51119-follow-2
branch
from
6f13efb to
64b62ca
Compare
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/ResolveDefaultColumnsUtil.scala
Outdated
Show resolved
Hide resolved
| log"with ${MDC(COLUMN_DATA_TYPE_SOURCE, field.dataType)}, " + | ||
| log"falling back to full analysis.") | ||
|
|
||
| field.getExistenceDefaultValue().map { text: String => |
There was a problem hiding this comment.
is this just the passed in defaultSQL?
There was a problem hiding this comment.
You are right, I guess I can re-use it or re-derive the redundant argument, i just changed to re-derive it as its a bit easier to get the method signature, but lmk if you prefer the first.
|
I re-ran the failing test and its green on the build link, but somehow still shows red on the PR. |
| s" field name: ${field.name}, value: $defaultSQL") | ||
| } | ||
| // sanity check | ||
| if (!literal.resolved) { |
There was a problem hiding this comment.
I think def analyze already did this check?
There was a problem hiding this comment.
Yea the original code for resolving existenceDefaultValues, which called analyze(), went through this check. It's even indicated as extra in the comments.
So I was just keeping the original behavior, do you suggest to remove?
There was a problem hiding this comment.
Removed, lets me know if its better
|
thanks, merging to master/4.0! |
…il existenceDefaultValues ### What changes were proposed in this pull request? The original change in #49840 was too optimistic and assumes that all column EXISTS_DEFAULT are already resolved and column folded. However, if there is bad EXISTS_DEFAULT metadata (an unresolved expression is persisted) it will break. Add fallback to use the original logic in that case. ### Why are the changes needed? There are some cases where bad EXISTS_DEFAULT metadata is persisted by external catalogs, due to some bugs such as #49942 or other problems. ### Does this PR introduce _any_ user-facing change? No, it should handle bad metadata better. ### How was this patch tested? Add unit test ### Was this patch authored or co-authored using generative AI tooling? No Closes #49962 from szehon-ho/SPARK-51119-follow-2. Authored-by: Szehon Ho <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> (cherry picked from commit 4ffc398) Signed-off-by: Wenchen Fan <[email protected]>
What changes were proposed in this pull request?
The original change in #49840 was too optimistic and assumes that all column EXISTS_DEFAULT are already resolved and column folded. However, if there is bad EXISTS_DEFAULT metadata (an unresolved expression is persisted) it will break. Add fallback to use the original logic in that case.
Why are the changes needed?
There are some cases where bad EXISTS_DEFAULT metadata is persisted by external catalogs, due to some bugs such as #49942 or other problems.
Does this PR introduce any user-facing change?
No, it should handle bad metadata better.
How was this patch tested?
Add unit test
Was this patch authored or co-authored using generative AI tooling?
No