andygrove commented on code in PR #5763:
URL: https://github.com/apache/datafusion-comet/pull/5763#discussion_r3999917133
##########
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:
##########
@@ -513,6 +541,19 @@ case class CometExecRule(session: SparkSession)
}
}
+ // `WriteFilesExec` does not carry the write's output path, but
CometWriteFiles needs it to
+ // decide whether the target filesystem is supported. Record it from the
enclosing command
+ // before the bottom-up walk reaches the write node. The absence of the
tag also tells
+ // CometWriteFiles that the write is not an
InsertIntoHadoopFsRelationCommand and must be
+ // declined. Only the Spark 4.0+ path consults this tag.
+ if (isSpark40Plus) {
+ plan.foreach {
+ case DataWritingCommandExec(cmd: InsertIntoHadoopFsRelationCommand, w:
WriteFilesExec) =>
+ w.setTagValue(CometExecRule.WRITE_OUTPUT_PATH,
cmd.outputPath.toString)
Review Comment:
Yes, and it is better. Done in 268fdb9d2: the pre-pass is gone and the
conversion happens at the command.
```scala
case d @ DataWritingCommandExec(cmd: InsertIntoHadoopFsRelationCommand, w:
WriteFilesExec)
if isSpark40Plus =>
w.setTagValue(CometExecRule.WRITE_OUTPUT_PATH, cmd.outputPath.toString)
d.withNewChildren(Seq(convertToComet(w, CometWriteFiles).getOrElse(w)))
```
The tag itself stays, because `CometOperatorSerde` only ever sees the
operator and there is nowhere else to put the path, but it is now set and read
inside one expression. That drops both of the invariants @comphead pointed at
in the same round: that `withNewChildren` copies tags, and that nothing hands
`CometExecRule` a `WriteFilesExec` without its enclosing command. Neither is
Comet's to guarantee, and if the second one ever stopped holding the write now
just stays on Spark instead of being converted against a missing tag.
`WriteFilesExec` moves to the unconditional "never tagged as a fallback"
list as a result, since on both version paths the conversion decision is made
at the command above 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]
