andygrove opened a new issue, #5195: URL: https://github.com/apache/datafusion-comet/issues/5195
### Describe the bug `spark.comet.exceptionOnDatetimeRebase` is declared but never read. It publishes to the user-facing configs page and promises a safety guarantee that Comet does not provide. The config is declared at [`CometConf.scala:718-728`](https://github.com/apache/datafusion-comet/blob/main/spark/src/main/scala/org/apache/comet/CometConf.scala#L718), is not marked `.internal()`, and sits in `CATEGORY_EXEC`, so it appears in the generated configuration documentation. Its doc string says: > When this is true, Comet will throw exceptions when seeing these dates/timestamps that were > written by Spark version before 3.0. Grepping the tree, `COMET_EXCEPTION_ON_LEGACY_DATE_TIMESTAMP` has no reader outside its own declaration. `SparkParquetOptions::use_legacy_date_timestamp_or_ntz` ([`native/core/src/parquet/parquet_support.rs:78`](https://github.com/apache/datafusion-comet/blob/main/native/core/src/parquet/parquet_support.rs#L78)), which appears to be its intended destination, is only ever initialized to `false` (lines 111 and 127) and is never consulted. The consequence is worse than a no-op config. A user who reads the scan limitation in `compatibility/scans.md`, learns that Comet mis-reads legacy-calendar data, and then sets this config specifically to be protected from that, gets silence and wrong results instead of the promised exception. This is carved out of #5010 so that issue can stay focused on the underlying scan limitation (implementing rebase, or falling back per scan). The two have different fixes: this one is a small, self-contained decision about a published config, and does not depend on rebase support landing. ### Steps to reproduce ```scala val path = "/tmp/legacy_dates" // Written by Spark 4.1 in legacy-calendar mode. spark.conf.set("spark.sql.parquet.datetimeRebaseModeInWrite", "LEGACY") spark.sql("SELECT cast(s as date) as d FROM VALUES ('1000-01-01'),('1990-01-01') AS v(s)") .write.mode("overwrite").parquet(path) spark.conf.unset("spark.sql.parquet.datetimeRebaseModeInWrite") // Ask Comet to protect us from exactly this data. spark.conf.set("spark.comet.exceptionOnDatetimeRebase", "true") spark.read.parquet(path).show() // +----------+ // | d| // +----------+ // |1000-01-06| <-- silently shifted, no exception raised // |1990-01-01| // +----------+ ``` Setting the config to `false` produces identical output, which is the point: it has no effect in either direction. ### Expected behavior Either of: 1. Wire the config up, so that `true` raises when the native scan encounters a Parquet file carrying `org.apache.spark.legacyDateTime` metadata (or otherwise holding hybrid-calendar values), matching the doc string; or 2. Remove the config, so Comet stops advertising a guarantee it does not implement. The scan limitation itself is documented in [`compatibility/scans.md`](https://github.com/apache/datafusion-comet/blob/main/docs/source/user-guide/latest/compatibility/scans.md) and tracked by #5010, so removing the config loses no user-facing information. If option 2 is chosen, the sentence in `scans.md` that currently describes the config as dead code should be dropped along with it. ### Additional context * Verified on `apache/main` @ `ba21f02de`, default Maven profile (Spark 4.1.2), macOS aarch64, debug build. * Carved out of #5010. Found while re-running the config audit for #4180. -- 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]
