andygrove commented on PR #5048:
URL:
https://github.com/apache/datafusion-comet/pull/5048#issuecomment-5152922785
Thanks for this @peterxcli — the diagnosis is right, and the Spark 2.4.5 "no
`org.apache.spark.version` key at all" case is a good catch.
My concern is where the check runs. `requiresDatetimeRebase` opens every
selected file's footer on the driver at plan time, for every query with a date
or timestamp column. Three things compound: `selectedPartitions` lists with
`partitionFilters.filterNot(isDynamicPruningFilter)`, so it's the *pre-DPP*
file set; `ParquetFileReader.open` reads the full footer including all
row-group metadata rather than `ParquetFooterReader.readFooter(...,
SKIP_ROW_GROUPS)`; and it's serial. On a table with thousands of files that is
significant added planning latency and driver heap, paid per query, even when
every file turns out to be fine.
I opened #5202 as an alternative that does the same detection in the native
Parquet reader factory, where the footer has already been fetched and cached
for the read itself — so it costs no additional I/O. Being at that point also
lets it consult row-group statistics, which matters because Spark stamps
`legacyDateTime` on a whole file whenever the write mode was LEGACY, whether or
not any value is actually old enough to rebase.
The tradeoff is the outcome: yours falls back to Spark and always returns
correct results, mine raises. I don't think fallback is reachable without
paying the plan-time cost, but that's worth other opinions.
One thing #5202 should take from here regardless: you read the rebase modes
from `new ParquetOptions(r.options, conf)`, which honors
`.option("datetimeRebaseMode", ...)`. #5202 reads the session conf only and
misses it. I'll fix that and credit you.
--
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]
