sunchao commented on code in PR #4821:
URL: https://github.com/apache/datafusion-comet/pull/4821#discussion_r4117479834
##########
spark/src/main/scala/org/apache/comet/DataTypeSupport.scala:
##########
@@ -53,6 +54,8 @@ trait DataTypeSupport {
BinaryType | StringType | _: DecimalType | DateType | TimestampType
| TimestampNTZType |
CalendarIntervalType =>
true
+ case dt if isTimeType(dt) =>
Review Comment:
[P2] Preserve fallback for TIME CSV scans with custom formats. This shared
allowance also enables `CometBatchScanExec`, whereas the new TIME rejection
only covers `CometScanTypeChecker`. On Spark 4.2, enable TIME, set
`spark.sql.sources.useV1SourceList=""` and
`spark.comet.scan.csv.v2.enabled=true`, then read a file containing `12-34-56`
with `.schema("t TIME").option("timeFormat", "HH-mm-ss").csv(path).collect()`.
Spark returns `12:34:56`, but the newly selected native CSV path omits
`timeFormat` and fails parsing the row. Previously this schema fell back to
Spark. Please reject TIME in the CSV checker until its parsing options are
supported, or scope this allowance to the row/Arrow paths.
Evidence: Compiled the actual base and head DataTypeSupport implementations
against Spark 4.1.3: the base returned `TIME accepted=false`; the head returned
`TIME accepted=true`. Spark 4.2's UnivocityParser uses
`TimeFormatter(options.timeFormatInRead, true)`. Its TimeFormatter source is
identical to 4.1.3, and a freshly compiled formatter probe parsed `12-34-56` as
45296000000000 nanoseconds. A standalone probe using the PR-pinned arrow-csv
59.3.0 with Time64(Nanosecond) accepted `12:34:56` but returned `Parser error:
Error while parsing value '12-34-56'` for the custom format. Source tracing
confirms CometScanRule uses CometBatchScanExec's inherited gate and
csvOptions2Proto does not transmit timeFormat. The complete Spark 4.2 query was
not executed locally.
--
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]
