andygrove opened a new pull request, #5026:
URL: https://github.com/apache/datafusion-comet/pull/5026
## Summary
Part of #4978 (Category 3).
Three explain-related boolean flags lived at the top level while their close
siblings (`format`, `native.enabled`, `rules`) already sat under
`spark.comet.explain.*`. This PR moves them under the group so the config
namespace is discoverable and consistent.
## Rename table
| Old key | New key |
| --- | --- |
| `spark.comet.explainFallback.enabled` |
`spark.comet.explain.fallback.enabled` |
| `spark.comet.explainCodegen.enabled` |
`spark.comet.explain.codegen.enabled` |
| `spark.comet.logFallbackReasons.enabled` |
`spark.comet.explain.fallback.log.enabled` |
For the placement question raised by the issue on `logFallbackReasons`
(either `explain.fallback.log.enabled` or `fallback.log.enabled`), I went
with
`explain.fallback.log.enabled`. `logFallbackReasons` and `explainFallback`
are
the two fallback-reason reporters — one aggregated WARN vs one WARN per
reason — so nesting the "log every reason" variant under the same
`.explain.fallback.*` parent makes the pairing visible in tab-completion and
docs.
Every renamed key is registered via `.withAlternative(...)`, so existing user
configurations keep working (a one-time deprecation warning is logged per old
key). No behavior change.
## Not in this PR
- The `ENABLE_COMET_LOG_FALLBACK_REASONS` env var is preserved as-is. There
is
no alias mechanism for env vars, and renaming it would silently break users
who set it in their launcher configs.
- The `dev/diffs/iceberg/*.diff` files still reference the old string keys.
They keep working through aliases, and the shuffle rename PR (#4986) took
the same approach — regenerating those diffs is out of scope here.
## Val rename
`val COMET_LOG_FALLBACK_REASONS` is renamed to
`COMET_EXPLAIN_FALLBACK_LOG_ENABLED` to match the new key (the two sibling
vals already had matching names, so only this one moved).
## Test plan
- [x] `./mvnw -Pspark-3.5 test -pl spark
-Dsuites="org.apache.comet.CometConfSuite"` — 10 tests pass (including 3 new
alias tests)
- [x] `./mvnw -Pspark-3.5 -pl spark spotless:check` — clean
- [x] `./mvnw -Pspark-3.5 -pl spark test-compile` — clean
- [ ] CI runs the broader test matrix
--
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]
