apache / apache/datafusion-comet

Triage the `spark.sql.legacy.*` configs dropped from the session-wide fallback in #4799

Open
#5,087 1 comment 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 6h
Merged PRs (30d)
190

Description

### What is the problem the feature request solves?

PR #4799 originally added a session-wide fallback (`spark.comet.legacyConfFallback.enabled`) that disabled Comet whenever any config in a curated `spark.sql.legacy.*` list was set to a non-default value. During review the author agreed to remove the session-wide fallback from that PR and address the configs separately (https://github.com/apache/datafusion-comet/pull/4799#issuecomment-5097107986).

That leaves most of the curated list with no verified decision. The audit in #4180 only covers two of them:

- `spark.sql.legacy.charVarcharAsString`: verified respected (write-side and read-side, including padding-sensitive predicates)
- `spark.sql.legacy.typeCoercion.datetimeToString.enabled`: safe to ignore, resolved in the analyzer before Comet sees the plan

The remaining configs from the curated list have not been audited and are not covered by the #4180 inventory:

- `spark.sql.legacy.decimal.retainFractionDigitsOnTruncate`
- `spark.sql.legacy.literal.pickMinimumPrecision`
- `spark.sql.legacy.allowParameterlessCount`
- `spark.sql.legacy.doLooseUpcast`
- `spark.sql.legacy.duplicateBetweenInput`
- `spark.sql.legacy.inSubqueryNullability`
- `spark.sql.legacy.scalarSubqueryCountBugBehavior`
- `spark.sql.legacy.disableMapKeyNormalization`
- `spark.sql.legacy.setopsPrecedence.enabled`
- `spark.sql.legacy.viewSchemaCompensation`
- `spark.sql.legacy.readFileSourceTableCacheIgnoreOptions`

Several of these are consumed by the parser, analyzer, or optimizer and are therefore likely safe by construction (Comet executes the plan Spark hands it), but "likely safe" is exactly the state the audit process in #4180 is meant to replace with a verified decision. At least one, `spark.sql.legacy.disableMapKeyNormalization`, is consumed at execution time by map-building code paths that Comet runs natively, so it needs an actual differential check.

### Describe the potential solution

For each config above, follow the method from the #4180 audit (differential run over Parquet-backed columns with Comet operators asserted in the plan) and record one of the three states: respected natively, fallback trigger, or known-safe to ignore. File a correctness bug for any verified divergence. Capture the per-config decision in the compatibility guide alongside the existing legacy-config notes so a reader can tell "considered and safe" from "not yet looked at".

### Additional context

Related: #4180 (config semantics audit umbrella), #4799 (per-expression and per-scan legacy config handling).

Contributor guide

Open the contributing guide

Research direction

Start with the audit method and inventory in #4180, then compare the remaining configs against the session-wide fallback changes in #4799. Run differential checks over Parquet-backed columns with Comet operators asserted in the plan; done means recording each config as natively respected, a fallback trigger, or safe to ignore, and updating the compatibility guide.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, scala
Domain
data-engineering
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.