You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/09/21 19:16:19 UTC
[PR] docs: exempt testing-category configs from the versioning policy [datafusion-comet]
andygrove opened a new pull request, #6089:
URL: https://github.com/apache/datafusion-comet/pull/6089
## Which issue does this PR close?
Closes #6088.
## Rationale for this change
The versioning policy currently puts every `spark.comet.*` key inside the compatibility surface, with no carve-out. Read literally, that applies the full contract to configs declared with `.category(CATEGORY_TESTING)`: renaming one would need a `withAlternative` alias, changing a default would need a `spark.comet.legacy.*` escape hatch plus an upgrade guide entry, and removing one would need a deprecation cycle and a major release.
Those keys are not a deployment surface. They exist so Comet's own suites, and contributors chasing a bug, can reach a state the rest of the code is not built to support — `spark.comet.scan.enabled` to isolate a planner problem, `spark.comet.explain.fallback.strict.enabled` to make a declined operator throw rather than quietly report itself, `spark.comet.testing.aggregate.partialMode.enabled` to run a partial aggregate without its final counterpart, and the on-heap family because Spark's own SQL suite needs on-heap mode. Guaranteeing them across releases would pin the internals they exist to expose, which is exactly what the rest of the policy leaves free to change.
So the policy now says the `testing` category is exempt, and says what follows from that in both directions: maintainers owe those keys nothing, and contributors may not use the category as a way to ship a production knob without committing to it.
## What changes are included in this PR?
Documentation only; no code changes.
- `docs/source/about/versioning_policy.md`: a new `Testing Configurations Are Exempt` subsection under the compatibility surface, stating that a `testing` key's name, type, accepted values, default, and semantics may change in any release, with no alias, no legacy config, no deprecation cycle, and no upgrade guide entry — and that such a key must not be the only way to reach a behavior production users need. The covered/not-covered bullet lists are updated, and the three sections that previously implied the opposite now cross-reference it: patch releases (which said they add no configuration keys), behavior changes, and renames.
- `docs/source/contributor-guide/config_conventions.md`: the `Categories` section now explains that the choice of category is what decides whether the key is covered, with guidance on when `testing` is the right answer. The rename checklist and the behavior-change section note the exemption.
- `docs/source/user-guide/latest/configs.md`: a note above the Development & Testing Settings table so users see the exemption where they would set one of these.
One thing worth a reviewer's attention: `internal()` is an orthogonal axis that the policy still does not address. Three keys are `internal()` but not in the `testing` category — `spark.comet.exec.broadcast.enabled`, `spark.comet.shuffle.jvm.spillThreshold`, and `spark.comet.shuffle.jvm.memoryWaitTimeout`. They are hidden from `configs.md` yet, under the policy as written before and after this PR, still covered. I left that alone rather than widen the change, but it is probably worth a second carve-out or a recategorization.
## How are these changes tested?
Not applicable — documentation only. `prettier --check` passes on the three files, and every relative link and heading anchor added here resolves against an existing heading.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: exempt testing-category and internal configs from the versioning policy [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #6089:
URL: https://github.com/apache/datafusion-comet/pull/6089
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: exempt testing-category and internal configs from the versioning policy [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #6089:
URL: https://github.com/apache/datafusion-comet/pull/6089#discussion_r4066480001
##########
docs/source/about/versioning_policy.md:
##########
@@ -71,6 +76,46 @@ change in any release:
fall back to Spark. An expression that ran natively in one release may fall back in the next,
and vice versa. The results stay the same; only the speed changes.
+### Testing and Internal Configurations Are Exempt
+
+Two kinds of configuration key sit outside this policy, and one rule identifies both: **a key is
+covered only if the [configuration reference](../user-guide/latest/configs.md) lists it outside the
+Development & Testing Settings table.** Everything the reference publishes as a production setting
+is guaranteed; nothing else is.
Review Comment:
### Correctness
**[P2] Preserve documented expression opt-ins in the compatibility surface**
Could the exemption be defined by an explicit `testing` category or `internal()` designation, while retaining public keys documented outside `configs.md`? For example, `spark.comet.expression.RLike.allowIncompatible` is documented in the compatibility guide and read by `CometConf.isExprAllowIncompat`, but it has no `ConfigEntry` and is absent from the generated configuration table. `GenerateDocs` emits per-expression `.enabled` keys there and emits native opt-ins on the compatibility pages instead. This rule therefore exempts an existing user-facing config family that is neither testing nor internal, permitting its keys/defaults to change without the normal compatibility path. The same exhaustive statement at `configs.md:83-85` needs to use the narrower definition too. Deprecated public aliases are also omitted from the generated key rows even though this policy retains them until a major release.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] docs: exempt testing-category and internal configs from the versioning policy [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #6089:
URL: https://github.com/apache/datafusion-comet/pull/6089#discussion_r4072978967
##########
docs/source/about/versioning_policy.md:
##########
@@ -71,6 +76,46 @@ change in any release:
fall back to Spark. An expression that ran natively in one release may fall back in the next,
and vice versa. The results stay the same; only the speed changes.
+### Testing and Internal Configurations Are Exempt
+
+Two kinds of configuration key sit outside this policy, and one rule identifies both: **a key is
+covered only if the [configuration reference](../user-guide/latest/configs.md) lists it outside the
+Development & Testing Settings table.** Everything the reference publishes as a production setting
+is guaranteed; nothing else is.
Review Comment:
Good catch, and you're right on both counts. I checked before changing anything: `GenerateDocs.generateConfigReference` writes `conf.key` and nothing else, so a deprecated alias never gets a row even though this same policy says the alias may only be dropped in a major release — the rule as I wrote it contradicted the rename section two screens further down. And the per-expression `allowIncompatible` keys have no `ConfigEntry` at all; `CometConf.isExprAllowIncompat` reads them by string, so nothing generates them into a table. They are documented in the compatibility guide, and users set them in production to accept a known divergence from Spark, which is exactly the kind of key the policy should be pinning.
The mistake was reaching for a documentation artifact as the test when the thing I actually meant is declared in the source. So the rule is now stated over the two marks directly — a key is exempt only if it is in the `testing` category or is marked `internal()`, and every other `spark.comet.*` key is covered — with an explicit paragraph saying absence from `configs.md` is not the test, naming the `allowIncompatible` opt-ins and the deprecated alias as the cases that prove it. The covered bullet at the top now says the same thing positively, and the note at `configs.md:83-85` narrows to internal keys and points at the compatibility guide for the opt-ins.
One consequence worth making explicit, so I added it to the contributor guide: a key read by string cannot carry either mark, so under the new rule it is covered by default. That closes the obvious way around the guarantee, which the old phrasing left wide open — a dynamic key was automatically exempt.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org