andygrove opened a new issue, #4978: URL: https://github.com/apache/datafusion-comet/issues/4978
Tracker for [#4082](https://github.com/apache/datafusion-comet/issues/4082) "Review all configuration options and consider renaming some for consistency." An audit of the 54 keys in `spark/src/main/scala/org/apache/comet/CometConf.scala` surfaced six categories of inconsistency. This issue is the umbrella; individual rename PRs should each cover one cluster and use the `.withAlternative(...)` mechanism to keep old keys working as deprecated aliases (see the [Configuration Conventions](https://datafusion.apache.org/comet/contributor-guide/config_conventions.html) contributor-guide page). ## Category 1 — Boolean flags missing the `.enabled` suffix Current convention: boolean flags end in `.enabled`. Exceptions to rename: - [ ] `spark.comet.nativeLoadRequired` → `spark.comet.nativeLoadRequired.enabled` - [ ] `spark.comet.exceptionOnDatetimeRebase` → `spark.comet.datetimeRebase.exceptionOnRead.enabled` (or similar; the current name is also very long) - [x] `spark.comet.exec.replaceSortMergeJoin` → `spark.comet.exec.forceShuffleHashJoin.enabled` (done as the pilot rename) ## Category 2 — Word separator inside a segment Most segments use `camelCase`; a handful use dots or dashes to separate words. Fix: - [ ] `spark.comet.columnar.shuffle.async.max.thread.num` → `spark.comet.columnar.shuffle.async.maxThreadNum` - [ ] `spark.comet.columnar.shuffle.async.thread.num` → `spark.comet.columnar.shuffle.async.threadNum` - [ ] `spark.comet.parquet.read.parallel.io.thread-pool.size` → `spark.comet.parquet.read.parallel.io.threadPoolSize` - [ ] `spark.comet.exec.pyarrowUdf.enabled` vs `spark.comet.exec.scalaUDF.codegen.enabled` — pick one casing for UDF and apply consistently. ## Category 3 — `.explain.*` group has orphaned siblings `spark.comet.explain.*` already exists (`format`, `native.enabled`, `rules`). These related keys live at the top level: - [ ] `spark.comet.explainCodegen.enabled` → `spark.comet.explain.codegen.enabled` - [ ] `spark.comet.explainFallback.enabled` → `spark.comet.explain.fallback.enabled` - [ ] `spark.comet.logFallbackReasons.enabled` → related concept, current name is orphaned; discuss placement (`spark.comet.explain.fallback.log.enabled`? `spark.comet.fallback.log.enabled`?). ## Category 4 — Two shuffle prefixes - `spark.comet.columnar.shuffle.*` (async/batch/memory/spill) - `spark.comet.native.shuffle.*` (partitioning) Users likely expect `spark.comet.shuffle.columnar.*` and `spark.comet.shuffle.native.*`. This is the largest rename cluster and is worth its own thread. ## Category 5 — `sparkToColumnar` uses camelCase as a top-level category `spark.comet.sparkToColumnar.*` is the only categorical prefix using camelCase (others are lowercase words: `exec`, `scan`, `parquet`, `shuffle`). Options: - Fold into the existing `spark.comet.convert.*` family: `spark.comet.convert.spark.enabled`. - Rename the category segment: `spark.comet.sparkColumnar.*`. ## Category 6 — Top-level scalars that could join a category - `spark.comet.batchSize` - `spark.comet.memoryOverhead` (candidate: `spark.comet.memory.overhead`) - `spark.comet.maxTempDirectorySize` (candidate: `spark.comet.tempDirectory.maxSize`) Lower priority — these are top-level scalars that pre-date the current category convention. ## Process Each rename PR: 1. Adds `.withAlternative("oldKey")` on the `conf(...)` call for the new key. 2. Renames the Scala `val` (grep `CometConf.` for callsites). 3. Updates every documented reference (grep `docs/source/` for the old key string). 4. Extends `CometConfSuite` if the rename exercises a new alias pattern. Alias removals happen no earlier than the next Comet major after the rename first ships (see the [versioning policy](https://datafusion.apache.org/comet/about/versioning_policy.html)). -- 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]
