sunchao commented on code in PR #5293:
URL: https://github.com/apache/datafusion-comet/pull/5293#discussion_r3807844496
##########
spark/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -870,8 +870,13 @@ object CometConf extends ShimCometConf {
.booleanConf
.createWithEnvVarOrDefault("ENABLE_COMET_STRICT_TESTING", false)
- val COMET_OPERATOR_DATA_WRITING_COMMAND_ALLOW_INCOMPAT: ConfigEntry[Boolean]
=
- createOperatorIncompatConfig("DataWritingCommandExec")
+ val COMET_OPERATOR_WRITE_FILES_ALLOW_INCOMPAT: ConfigEntry[Boolean] =
+ // Native writes used to replace the whole DataWritingCommandExec; they
now replace only
+ // WriteFilesExec, so the opt-in moved with it. Keep the old key working
for anyone who had
+ // already enabled the experimental writer.
+ createOperatorIncompatConfig(
+ "WriteFilesExec",
+ Some(getOperatorAllowIncompatConfigKey("DataWritingCommandExec")))
Review Comment:
[P2] Could the planner read the registered
`COMET_OPERATOR_WRITE_FILES_ALLOW_INCOMPAT` entry?
`CometExecRule.isOperatorEnabled` calls
`isOperatorAllowIncompat("WriteFilesExec")`, which reads only the new key
through `SQLConf.getConfString`. That bypasses the alternative declared here.
On Spark 4, an existing job with `spark.comet.parquet.write.enabled=true`
and only `spark.comet.operator.DataWritingCommandExec.allowIncompatible=true`
now falls back to Spark's writer. I reproduced this with the exact PR's
`CometConf` and Spark 4.0.4: the registered config entry returns `true`, while
the planner's check returns `false`.
Could you preserve the old-key-only behavior and add a regression test that
also checks that an explicitly set new key takes precedence?
--
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]
