dongjoon-hyun commented on code in PR #59895:
URL: https://github.com/apache/spark/pull/59895#discussion_r4413482451
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -1176,6 +1179,27 @@ class Analyzer(
})
}
+ // DELETE, UPDATE and MERGE on a table that supports row-level operations
are rewritten into
+ // plans that read the metadata columns they need (see
RewriteRowLevelCommand). The rewrite
+ // takes the output of the target table as the data columns to write, so
metadata columns must
+ // not be added to it, even if the command is resolved only in a later
iteration. This does not
+ // apply to UPDATE and MERGE on a table that accepts any schema, which
Spark leaves to the rules
Review Comment:
Thank you. You're right. Spark leaves only the assignments of these commands
to the injected rules, and `RewriteUpdateTable` and `RewriteMergeIntoTable`
still rewrite them once they are aligned, including a `MERGE` with only
`DELETE` actions. In 07d03a07f72, I updated the comment and kept the behavior
for such tables as it is. For the record, `ResolveReferences` does not resolve
a `MERGE` on such a table at all, so this case happens only when the injected
rules resolve it.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -1139,9 +1139,12 @@ class Analyzer(
if (metaCols.isEmpty) {
node
} else {
- val newNode = node.mapChildren(
- addMetadataCol(_, metaCols.map(_.exprId).toSet,
onlyUniqueAndNecessaryMetadataColumns)
- )
+ val requiredAttrIds = metaCols.map(_.exprId).toSet
+ val newNode = node.mapChildren {
+ case child if isRowLevelCommandTarget(node, child) => child
Review Comment:
#59900 is merged, and I rebased this PR on it. In 07d03a07f72, I added your
`id < 1.5` form to `GroupBasedNoMetadataDeleteFromTableSuite` and
`DeltaBasedNoMetadataDeleteFromTableSuite`, next to the `id < 2` tests from
#59900. Both forms succeed now, and the tests check that the scan reads
`_partition` while neither the write schema nor the write log includes 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]
