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:25 UTC
(datafusion-comet) branch gh-readonly-queue/main/pr-6089-29d5d4f939bd340ab7107bcf6d7ca00f541e8dfe created (now 8166586fa7)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-6089-29d5d4f939bd340ab7107bcf6d7ca00f541e8dfe
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
at 8166586fa7 docs: exempt testing-category and internal configs from the versioning policy (#6089)
This branch includes the following new commits:
new 8166586fa7 docs: exempt testing-category and internal configs from the versioning policy (#6089)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion-comet) 01/01: docs: exempt testing-category and internal configs from the versioning policy (#6089)
Posted by gi...@apache.org.
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