Skip to content

spark.comet.exceptionOnDatetimeRebase is dead code: wire it up or remove it #5195

Description

@andygrove

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,
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),
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

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
    and tracked by Datetime rebase: track the documented scan limitation, and spark.comet.exceptionOnDatetimeRebase is dead code #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

Metadata

Metadata

Assignees

Labels

area:scanParquet scan / data readingbugSomething isn't workingpriority:mediumFunctional bugs, performance regressions, broken featuresrequires-triage

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions