andygrove opened a new issue, #5087:
URL: https://github.com/apache/datafusion-comet/issues/5087

   ### 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).
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to