You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/24 09:38:26 UTC
(datafusion-comet) 01/01: docs: exempt testing-category and internal configs from the versioning policy (#6089)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6089-29d5d4f939bd340ab7107bcf6d7ca00f541e8dfe
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
commit 8166586fa767008a45665358d200ce23ec955f64
Author: Andy Grove <ag...@apache.org>
AuthorDate: Thu Sep 24 09:38:02 2026 +0000
docs: exempt testing-category and internal configs from the versioning policy (#6089)
* docs: exempt testing-category configs from the versioning policy
* docs: extend the config exemption to internal() keys
* docs: key the config exemption on the declaration, not on configs.md
The exemption was phrased as an exhaustive rule over the configuration
reference: a key was covered only if configs.md published it as a
production setting. That over-reached. Two families of covered key have
no row on that page.
The per-expression spark.comet.expression.<Name>.allowIncompatible
opt-ins have no ConfigEntry at all - CometExecRule and QueryPlanSerde
read them by string - so GenerateDocs never emits them. They are
documented in the compatibility guide instead, and users set them in
production to accept a known divergence from Spark. A deprecated alias
left behind by withAlternative has no row either, because
generateConfigReference writes conf.key and nothing else, yet this same
policy says an alias may only be dropped in a major release.
So state the rule over the two declarations that actually mark a key,
.category(CATEGORY_TESTING) and .internal(), and say outright that
absence from configs.md is not the test. The contributor guide gains the
corollary that a key read by string cannot carry either mark and is
therefore covered, which closes the obvious way around the guarantee.
---
docs/source/about/versioning_policy.md | 73 +++++++++++++++++++++-
.../source/contributor-guide/config_conventions.md | 48 +++++++++++++-
docs/source/user-guide/latest/configs.md | 13 ++++
3 files changed, 130 insertions(+), 4 deletions(-)
diff --git a/docs/source/about/versioning_policy.md b/docs/source/about/versioning_policy.md
index e5cb832f44..7b6e4974c8 100644
--- a/docs/source/about/versioning_policy.md
+++ b/docs/source/about/versioning_policy.md
@@ -48,7 +48,14 @@ interface that vendors implement. It is small on purpose, and everything outside
The following are covered by this versioning policy:
- **Configuration keys under `spark.comet.*`**: their names, types, accepted values, default
- values, and semantics.
+ values, and semantics. Two classes of key are excluded — those in the `testing` category, and
+ those marked internal; see
+ [Testing and Internal Configurations Are Exempt](#testing-and-internal-configurations-are-exempt).
+ Every other key is covered wherever Comet documents it, including keys the
+ [configuration reference](../user-guide/latest/configs.md) does not list in its own right: the
+ per-expression `spark.comet.expression.<Name>.allowIncompatible` opt-ins, which the
+ [compatibility guide](../user-guide/latest/compatibility/index.md) documents, and the deprecated
+ alias a rename leaves behind.
- **A small, enumerated public Java and Scala API**: the class names users write into Spark config
properties, and the S3 credential provider SPI that vendors implement. The full list is in
[Public Scala and Java API](#public-scala-and-java-api).
@@ -59,6 +66,9 @@ The following are covered by this versioning policy:
The following are internal implementation details. They are not covered by this policy and may
change in any release:
+- Configuration keys in the `testing` category, and keys marked internal. They exist to let Comet's
+ own suites and maintainers reach internal behavior, and they expose that behavior directly. See
+ [Testing and Internal Configurations Are Exempt](#testing-and-internal-configurations-are-exempt).
- The protobuf format used to serialize query plans between the JVM and the native library. The
JVM jar and the native library ship together and are versioned together; see
[Native Library Coupling](#native-library-coupling).
@@ -71,6 +81,52 @@ 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 what puts a key outside is an explicit
+mark on its declaration in `CometConf.scala`: **a key is exempt only if it is in the `testing`
+category or is marked `internal()`.** Every other `spark.comet.*` key is covered.
+
+Absence from the [configuration reference](../user-guide/latest/configs.md) is not the test,
+because that page is not an exhaustive list of covered keys. The per-expression
+`spark.comet.expression.<Name>.allowIncompatible` opt-ins are read as plain strings with no
+`ConfigEntry` behind them, so no generated table ever lists them; the
+[compatibility guide](../user-guide/latest/compatibility/index.md) is where they are documented
+instead. A deprecated alias left behind by a rename has no row of its own either. Both are fully
+covered.
+
+The two mechanisms that do exempt a key are set independently of each other:
+
+- **The `testing` category.** Every key declares a category, and `testing` routes it into the
+ Development & Testing Settings table rather than in with the production settings. These keys
+ exist so that Comet's own suites, and contributors chasing a bug, can reach a state the rest of
+ the code is not built to support: disabling native scans to isolate a planner problem, running
+ Comet in on-heap mode, making a declined operator throw instead of quietly reporting itself, or
+ running a partial aggregate without its final counterpart. Setting one is a debugging step, not a
+ deployment choice.
+- **`internal()`.** A key marked internal is left out of the configuration reference entirely, so
+ Comet never publishes its name, its default, or what it does. A user has no supported way to
+ learn that it exists, which is the point: these are escape hatches a maintainer reaches for while
+ working on Comet itself.
+
+For a key caught by either mechanism, the name, type, accepted values, default value, and semantics
+may change in any release, including a patch release, and the key may be removed outright. None of
+the machinery the rest of this policy requires applies: no `spark.comet.legacy.*` escape hatch for a
+behavior change, no deprecated alias for a rename, no deprecation cycle before removal, and no
+upgrade guide entry.
+
+The exemption exists because these keys point at Comet's internals by construction. Guaranteeing
+them across releases would pin the implementation details they expose, which is the thing the rest
+of this section deliberately leaves free to change.
+
+The corollary binds contributors: **neither mechanism may be the only way to reach a behavior that
+production users need.** If a knob turns out to be one that deployments legitimately set, it belongs
+in a non-`testing` category and must not be marked internal, and the guarantees come with it.
+Choosing a category, and deciding whether to mark a key internal, are therefore policy decisions
+rather than routing details; see
+[Categories and Visibility](../contributor-guide/config_conventions.md#categories-and-visibility)
+in the contributor guide.
+
## What Each Version Component Means
### Major Releases
@@ -99,8 +155,11 @@ A minor release may:
### Patch Releases
A patch release contains bug fixes only. It adds no configuration keys and makes no behavior
-changes, with one exception: correctness fixes, which are covered in
-[Correctness Fixes Are Not Breaking Changes](#correctness-fixes-are-not-breaking-changes).
+changes, with two exceptions: correctness fixes, which are covered in
+[Correctness Fixes Are Not Breaking Changes](#correctness-fixes-are-not-breaking-changes), and
+configuration keys in the `testing` category or marked internal, which are outside the policy
+altogether and may be added, changed, or removed in any release. See
+[Testing and Internal Configurations Are Exempt](#testing-and-internal-configurations-are-exempt).
## Behavior Changes and Legacy Configurations
@@ -125,6 +184,10 @@ Behavior changes that require this treatment include changing the default value
configuration key, changing what an existing key's values mean, and changing the semantics of an
`Incompatible` expression or operator whose divergence from Spark users may have come to depend on.
+Changing a `testing` or internal key is not a behavior change for this purpose, and needs no escape
+hatch. See
+[Testing and Internal Configurations Are Exempt](#testing-and-internal-configurations-are-exempt).
+
### Lifetime of a Legacy Configuration
A `spark.comet.legacy.*` key is deprecated from the moment it is added. Its purpose is to buy users
@@ -147,6 +210,10 @@ The alias may only be dropped in a major release.
Removing a configuration key outright requires a deprecation cycle: the key must remain available,
with a deprecation warning, for at least one minor release before it is removed in a major release.
+Neither rule applies to a `testing` or internal key, which may be renamed without an alias and
+removed in any release. See
+[Testing and Internal Configurations Are Exempt](#testing-and-internal-configurations-are-exempt).
+
## Correctness Fixes Are Not Breaking Changes
Comet's contract is to produce the results that Apache Spark produces. When an expression or
diff --git a/docs/source/contributor-guide/config_conventions.md b/docs/source/contributor-guide/config_conventions.md
index 5bcad087ea..6c198f740c 100644
--- a/docs/source/contributor-guide/config_conventions.md
+++ b/docs/source/contributor-guide/config_conventions.md
@@ -66,19 +66,60 @@ The symbol name is what appears in code; the key is what appears in user configu
The two do not have to match segment-for-segment — brevity in the symbol is fine as long
as the key remains descriptive.
-## Categories
+## Categories and Visibility
Every `ConfigEntry` must call `.category(...)`. The category is used to route the key into
the right table in the user guide's `configs.md`. Available categories are declared as
`CATEGORY_*` constants at the top of `CometConf.scala`. If a new config does not fit an
existing category, discuss adding a new one before landing the config.
+A second, independent choice is whether to call `.internal()`, which keeps the key out of
+`configs.md` altogether.
+
+Neither choice is cosmetic. Together they decide whether Comet's
+[versioning policy](../about/versioning_policy.md) covers the key. The policy's rule keys on these
+two marks and nothing else: a key is exempt only if it is in the `testing` category or is marked
+`internal()`, and every other `spark.comet.*` key is covered.
+
+| Category | `internal()` | Appears in `configs.md` | Covered by the versioning policy |
+| --------- | ------------ | -------------------------------- | -------------------------------- |
+| any other | no | yes, in its category's table | **yes** |
+| `testing` | no | yes, under Development & Testing | no |
+| any | yes | no | no |
+
+Note that a key read by string rather than through a registered `ConfigEntry` — as the
+per-expression `spark.comet.expression.<Name>.allowIncompatible` opt-ins are — has no way to carry
+either mark, so it is covered. Reaching for a dynamic key is therefore not a way to avoid the
+guarantee; if the key is a debugging aid, give it a real `ConfigEntry` in the `testing` category.
+
+Being covered commits the project to the key's name, type, accepted values, default, and semantics
+across minor releases. Being
+[exempt](../about/versioning_policy.md#testing-and-internal-configurations-are-exempt) means the key
+may be renamed, retyped, redefaulted, or removed in any release, including a patch release, with no
+alias, no deprecation cycle, and no upgrade guide entry.
+
+So reach for `testing` or `internal()` only when the key exists to let Comet's own suites, or
+someone debugging Comet itself, reach a state the rest of the code is not built to support —
+disabling native scans, running in on-heap mode, making a declined operator throw. Use `internal()`
+in particular when Comet should not publish the key at all, so that nobody can adopt it in the first
+place.
+
+If a deployment would have a legitimate reason to set the key, it belongs in a non-`testing`
+category and must not be `internal()`, and the guarantees come with it. Do not use either mechanism
+as a way to ship a production knob without committing to it.
+
## Renaming an Existing Config
Configs under `spark.comet.*` are stable across minor releases: users may have set them in
production `spark-defaults.conf` files, Spark job submissions, or notebooks. Renaming a key
must not silently break those deployments.
+Keys in the `testing` category, and keys marked `internal()`, are
+[exempt from the versioning policy](../about/versioning_policy.md#testing-and-internal-configurations-are-exempt)
+and may be renamed outright: skip the `withAlternative` call in step 1, and step 5 with it. The
+rest of the checklist still applies, because a stale key string left behind in code or docs is a
+bug either way.
+
Use the `withAlternative` builder on `ConfigBuilder` to keep the old key working as a
deprecated alias:
@@ -159,5 +200,10 @@ Two cases do **not** need a legacy config:
- **Changes to which expressions and operators run natively.** Falling back to Spark, or ceasing
to, changes performance rather than results.
+Nor does a change to a key in the `testing` category or a key marked `internal()`, both
+[exempt from the versioning policy](../about/versioning_policy.md#testing-and-internal-configurations-are-exempt):
+the default and the meaning may change in any release, with no legacy config and no upgrade guide
+entry. Update the suites that set it in the same PR.
+
Removing a legacy config is a major-release change, handled the same way as removing a deprecated
alias.
diff --git a/docs/source/user-guide/latest/configs.md b/docs/source/user-guide/latest/configs.md
index 49ee6dd48d..59d5c711fa 100644
--- a/docs/source/user-guide/latest/configs.md
+++ b/docs/source/user-guide/latest/configs.md
@@ -74,6 +74,19 @@ shuffle, including applications that rely only on remote reliable storage or dec
## Development & Testing Settings
+These settings exist for Comet's own test suites and for debugging. They are **not covered by the
+[versioning policy](../../about/versioning_policy.md#testing-and-internal-configurations-are-exempt)**:
+their names, defaults, accepted values, and meanings may change in any release, including a patch
+release, and any of them may be removed without a deprecation cycle. Do not set them in
+production.
+
+Comet also marks a handful of keys internal and deliberately leaves them off this page entirely.
+They are maintainer escape hatches, not settings, and carry no guarantee of any kind. Absence from
+this page does not by itself mean that, though: the per-expression
+`spark.comet.expression.<Name>.allowIncompatible` opt-ins are documented in the
+[compatibility guide](compatibility/index.md) rather than here, and the versioning policy covers
+them like any other production setting.
+
<!--BEGIN:CONFIG_TABLE[testing]-->
<!--END:CONFIG_TABLE-->
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org