Resolve UpCast expressions introduced in Delta DMLs #1998
Closed
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
| @@ -107,6 +107,11 @@ class DeltaSparkSessionExtension extends (SparkSessionExtensions => Unit) { | |||
| extensions.injectPostHocResolutionRule { session => | |||
| new PreprocessTableDelete(session.sessionState.conf) | |||
| } | |||
| // Resolve new UpCast expressions that might have been introduced by [[PreprocessTableUpdate]] | |||
| // and [[PreprocessTableMerge]]. | |||
There was a problem hiding this comment.
@johanl-db, in DeltaMergeBuilder we explicitly call PostHocResolveUpCast. The inject Rule only runs automatically for UPDATES and not for MERGE?
There was a problem hiding this comment.
MERGE using the Scala Delta table API doesn't go through the analyzer but instead resolves the merge command manually. UPDATE (SQL + Scala/Java/Python) and MERGE (SQL) do run through the regular path and injected rules will be applied.
I'm not very familiar with why we need to resolve the MERGE command manually when using the Scala API, this may be something we can improve in the future
vkorukanti
pushed a commit
to vkorukanti/delta
that referenced
this pull request
Oct 26, 2023
Follow-up from delta-io#1998 to run the rule `PostHocResolveUpCast` multiple times until all `UpCast` expressions are resolved. With the previous change, it only ran once, failing to resolve nested UpCast expressions. Added a test covering a problematic case with multiple `UpCast` expressions. GitOrigin-RevId: de56850c6013ad2b7c4a134917778069ee2f5915
xupefei
pushed a commit
to xupefei/delta
that referenced
this pull request
Oct 31, 2023
Follow-up from delta-io#1998 to run the rule `PostHocResolveUpCast` multiple times until all `UpCast` expressions are resolved. With the previous change, it only ran once, failing to resolve nested UpCast expressions. Added a test covering a problematic case with multiple `UpCast` expressions. GitOrigin-RevId: de56850c6013ad2b7c4a134917778069ee2f5915
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Which Delta project/connector is this regarding?
Description
#1938 changed the casting behavior in MERGE and UPDATE to follow the value of the
storeAssignmentPolicyconfig instead of theansiEnabledone, making the behavior consistent with INSERT.This change breaks MERGE and UPDATE operations that contain a cast when
storeAssignmentPolicyis set toSTRICT, throwing an internal error during analysis.The cause is the
UpCastexpression added byPreprocessTableMergeandPreprocessTableUpdatewhen processing assignments.UpCastis meant to be replaced by a regular cast after performing checks by theResolveUpCastrule that runs during the resolution phase beforePreprocessTableMergeandPreprocessTableUpdateintroduce the expression, leaving the cast unresolved.The fix is to run the
ResolveUpCastrule once more afterPreprocessTableMergeandPreprocessTableUpdatehave run.How was this patch tested?
Missing tests covering cast behavior for the different values of
storeAssignmentPolicyfor UPDATE and MERGE are added, covering: