andygrove commented on code in PR #7053:
URL: https://github.com/apache/datafusion-comet/pull/7053#discussion_r4459252052
##########
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:
Agreed that the INT96 guard should only apply when the requested output
timestamp type is INT96. The fallback is conservative (it costs acceleration,
not correctness), and #7053 is already in the merge queue, so a push here would
send it back through the full queue tier. Filed as a follow-on: #7463, covering
the guard change, a regression test with a non-INT96 timestamp output, and the
matching docs line.
--
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]
