Consider deprecating spark.comet.schemaEvolution.enabled

[andygrove]

Background

spark.comet.schemaEvolution.enabled is an internal config that gates whether Comet's Parquet scan paths permit certain widening type promotions (INT32 -> INT64, FLOAT -> DOUBLE, and on Spark 4+ INT32 -> DOUBLE).

Defaults today (via ShimCometConf):

• Spark 3.x: false
• Spark 4.x: true

@mbutrovich raised the question in #4229 (comment):

why do we have spark.comet.schemaEvolution.enabled config anymore? Maybe we should deprecate that first and help us simplify the story. I think it's legacy from when Comet's Parquet decoder could be called from Iceberg, which has different schema evolution semantics.

Why this is worth investigating

The flag now exists primarily to model the Spark version's permissiveness rather than a user-tunable knob: Spark 3.x rejects these widenings, Spark 4.x accepts them. If that's its only purpose post Iceberg-decoder removal, the per-version default already encodes the right answer and a user-tunable internal config adds little besides surface area to reason about.

Scope of this issue

Investigate and report back:

1. Are there any code paths today that flip this flag away from the per-version default (Iceberg integration, tests, callers outside the Comet codebase)?
2. Does keeping the flag enable any correct behavior that we'd lose by hardcoding per-version defaults?
3. If neither (1) nor (2), propose a deprecation path: rename to a non-tunable internal constant, fold the check into the version-specific shim, and update the contributor docs.

No code changes required up front; a writeup on the above is enough to decide next steps.

Related

• fix: reject disallowed type promotions in native_datafusion scan #4229 (current PR, which uses this flag to gate the three widening-rejection cases on Spark 3.x)
• native_datafusion: silent wrong-answer paths for primitive Parquet type conversions Spark rejects #4297 (broader gap of unconditionally-rejected primitive conversions that this flag does not govern)

[andygrove]

Folded into #4229. The deprecation removes the public ConfigEntry, hardcodes per-Spark-version behavior via ShimCometConf, and rewrites the ParquetReadSuite tests that flipped the conf. Iceberg integration coupling turned out to be safe to remove (Java integration is dead per @andygrove; iceberg-rust native reader is what we use now). Stale spark.comet.schemaEvolution.enabled references in dev/diffs/iceberg/*.diff are no-ops post-merge — those diffs need a broader regen pass since they're stale against upstream Iceberg, but that's separate work.

[mbutrovich]

Folded into #4229. The deprecation removes the public ConfigEntry, hardcodes per-Spark-version behavior via ShimCometConf, and rewrites the ParquetReadSuite tests that flipped the conf. Iceberg integration coupling turned out to be safe to remove (Java integration is dead per @andygrove; iceberg-rust native reader is what we use now). Stale spark.comet.schemaEvolution.enabled references in dev/diffs/iceberg/*.diff are no-ops post-merge — those diffs need a broader regen pass since they're stale against upstream Iceberg, but that's separate work.

Why is the config added to the shim if we're removing it?

[parthchandra]

Afaik, the config was intended for native_comet and not just Iceberg. Either way, since both are deprecated, it should be okay to remove the config.
After removal are we going to continue enforcing Spark restrictions even when Comet/Datafusion permits it?

[mbutrovich]

After removal are we going to continue enforcing Spark restrictions even when Comet/Datafusion permits it?

Yep I think the goal is to only follow what Spark does.