sunchao commented on code in PR #7053:
URL: https://github.com/apache/datafusion-comet/pull/7053#discussion_r4458640616
##########
spark/src/main/scala/org/apache/comet/serde/operator/NativeWriteUtils.scala:
##########
@@ -317,6 +319,39 @@ object NativeWriteUtils {
// scalastyle:on classforname
}.getOrElse(false)
+ /**
+ * A fallback reason when the session asks for a LEGACY datetime rebase on
write, or `None`.
+ *
+ * The native writer always writes proleptic Gregorian (corrected) datetime
values and stamps
+ * `org.apache.spark.version` with no legacy markers. Honoring a LEGACY
write rebase mode would
+ * require rebasing the values and stamping
`org.apache.spark.legacyDateTime` /
+ * `org.apache.spark.legacyINT96`, so fall back to Spark rather than
silently ignoring the
+ * requested mode and letting readers trust a "corrected" marker over
legacy-intent data.
+ * TIMESTAMP_NTZ is exempt because Spark never rebases NTZ values on write.
+ */
+ def legacyDatetimeRebaseWriteReason(output: Seq[Attribute]): Option[String]
= {
+ val hasDate = output.exists(a => SupportLevel.containsType(a.dataType,
classOf[DateType]))
+ val hasTimestamp =
+ output.exists(a => SupportLevel.containsType(a.dataType,
classOf[TimestampType]))
+ // getConf also reads the 3.x legacy alias keys. Both modes default to
EXCEPTION on 3.x and
+ // to CORRECTED on 4.0+.
+ def isLegacy(mode: Any): Boolean = mode.toString == "LEGACY"
+ val datetimeLegacy = (hasDate || hasTimestamp) &&
+ isLegacy(SQLConf.get.getConf(SQLConf.PARQUET_REBASE_MODE_IN_WRITE))
+ val int96Legacy =
+ hasTimestamp &&
isLegacy(SQLConf.get.getConf(SQLConf.PARQUET_INT96_REBASE_MODE_IN_WRITE))
Review Comment:
Could the INT96-specific guard check the requested output timestamp
encoding? With a timestamp column,
`spark.sql.parquet.outputTimestampType=TIMESTAMP_MICROS` and
`datetimeRebaseModeInWrite=CORRECTED`, changing only `int96RebaseModeInWrite`
from CORRECTED to LEGACY makes this exact helper return a fallback reason
instead of `None`. Spark writes INT64 TIMESTAMP(MICROS) in both cases; for
`1500-01-01 12:34:56.123456` my two files contained the identical value
`-14831724303876544`. Thus an unrelated legacy INT96 setting now disables
acceleration for a supported corrected microsecond write. Restrict the INT96
condition to the encoding it governs, and cover a non-INT96 timestamp output in
the regression test.
--
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]
