andygrove opened a new pull request, #5063: URL: https://github.com/apache/datafusion-comet/pull/5063
> [!IMPORTANT] > This PR is stacked on #4986 (`refactor: unify shuffle configs under spark.comet.shuffle.* prefix`) and must not be merged until that one lands. The shuffle config references here use the new `spark.comet.shuffle.*` names introduced by #4986. Once #4986 merges, this branch will be rebased and the diff will show only the docs commit. ## Which issue does this PR close? No issue filed; this is a docs cleanup found by sweeping every tracked markdown file for `spark.comet.*` keys and validating them against `CometConf.scala`. ## Rationale for this change Several docs reference configuration keys that do not exist. A user copying these into a `spark-submit` command gets a silently ignored setting, which is worse than an error because the doc implies the setting took effect. The largest group is the expression opt-in prefix. The real namespace is `spark.comet.expression.*` (`COMET_EXPR_CONFIG_PREFIX` in `CometConf.scala`), but the contributor guide, the versioning policy, and two expression-audit pages all use `spark.comet.expr.*`. Those same places also describe a global `allowIncompatible` key, which has never existed: the opt-in is always per-expression or per-operator. While verifying the remaining `spark.comet.expression.<Name>.*` references against the serde registry, three of them turned out to name something that is not a valid expression config, and the surrounding prose was stale in the same way, so those claims were corrected too. ## What changes are included in this PR? Wrong config prefix, corrected to `spark.comet.expression.*`: - `contributor-guide/adding_a_new_expression.md` (3 references, plus removal of the nonexistent global key from the `Incompatible` description) - `about/versioning_policy.md` - `.claude/skills/audit-comet-expression/SKILL.md` - `contributor-guide/benchmarking_macos.md`, which passed `spark.comet.expression.allowIncompatible=true` Explode is gated at the operator level, not the expression level, so `expression-audits/generator_funcs.md` now points at `spark.comet.operator.GenerateExec.allowIncompatible`. Config keys naming an expression that does not exist: - `spark.comet.expression.regexp.allowIncompatible` (`predicate_funcs.md`, `string_funcs.md`). There is no `regexp` expression; the real keys are `RLike` and `RegExpReplace`. - `spark.comet.expression.Upper.allowIncompatible` / `Lower.allowIncompatible` (`string_funcs.md`). Case conversion overrides the opt-in key via `nativeOptInConfigKeyOverride`, so the key is `spark.comet.caseConversion.enabled`. Correcting those four also meant correcting the claims around them, which predate the codegen dispatcher: `rlike`, `regexp_replace`, `upper` and `lower` are `Compatible` by default now, with the native regexp / case-mapping path opt-in, not "unconditionally `Incompatible`". The `regexp_replace` entry also cited `RegExp.isSupportedPattern`, a symbol that no longer exists. Renamed or never-registered keys: - `spark.comet.metrics.detailed` (`user-guide/latest/metrics.md`) was never registered; those metrics are always reported - `spark.comet.memoryPool.fraction` to `spark.comet.exec.memoryPool.fraction` (`benchmark-results/tpc-ds.md`, `tpc-h.md`) - `spark.comet.shuffle.write.buffer.size` to `spark.comet.shuffle.native.writeBufferSize` (`native_shuffle.md`) - `spark.comet.exec.shuffle.enableFastEncoding` and `spark.comet.exec.shuffle.fallbackToColumnar`, both removed, dropped from `benchmarking_macos.md` - `spark.comet.exec.all.enabled`, removed, dropped from `benchmarking_spark_sql_perf.md` and `benchmarks/README.md` - `spark.comet.columnar.shuffle.enabled` to `spark.comet.shuffle.mode=jvm` (`benchmarking_spark_sql_perf.md`) - `spark.comet.cast.allowIncompatible` to `spark.comet.expression.Cast.allowIncompatible` (`benchmarks/README.md`) Deliberately left alone: `docs/source/changelog/*`, which records historical PR titles mentioning since-removed keys, the `docs/comet-0.1x/` version archives, and placeholder forms such as `spark.comet.exec.<yourOperator>.enabled`. ## How are these changes tested? Docs only, no code changes. Verified by extracting every `spark.comet.*` token from all tracked `.md` and `.rst` files and diffing that set against the keys registered in `CometConf.scala`, expanded for the dynamic `expression.<Name>.*`, `operator.<Name>.*` and `exec.<name>.enabled` patterns. Every expression name appearing in a config key was checked against the `classOf[...]` entries in `QueryPlanSerde.scala`, and each corrected claim was read back against the relevant serde. After the change the only unmatched keys are the three changelog entries noted above. `prettier` reports no formatting changes. -- 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]
