dwsmith1983 commented on PR #5365:
URL: 
https://github.com/apache/datafusion-comet/pull/5365#issuecomment-5522958178

   Both P2s addressed in 053ec14b4, plus one related bug your cases flushed out.
   
   Session read modes: verified the exact getRebaseSpec semantics against the 
3.5.9 and 4.0.4 sources first. The conf applies only when 
org.apache.spark.version is absent from the footer; with a version present, 
policy is purely the version comparison plus the legacy flag. The serde now 
resolves the effective modes the way ParquetFileFormat does (ParquetOptions 
over relation options and session conf, so the 3.5 EXCEPTION and 4.0 CORRECTED 
defaults come from the session rather than being hardcoded), carries them in 
two new DeltaSparkScanCommon fields, and the native resolver uses them in the 
version-absent arm. Your two cases are regression tests built on raw parquet-mr 
files with no Spark metadata, converted to Delta: corrected 1500-01-01 and the 
pre-1970 microsecond timestamp now read verbatim natively under CORRECTED, 
rebase under LEGACY, and fail loudly under EXCEPTION. One nuance mirrored 
deliberately: conf LEGACY resolves the writer zone from file metadata; Spark 
falls back to t
 he JVM default zone, which native cannot replicate, so non UTC zones keep the 
refuse-ancient posture for timestamps while dates rebase fully since they are 
zone free.
   
   Nested types: the wrapper now checks the applicable leaf policies before 
rejecting, recursing through struct, list, map, and dictionary. Your STRUCT 
with a DATE leaf under corrected dates plus legacy INT96 stays native and 
correct, tested with modern and null values; a nested column with a genuinely 
affected leaf still declines.
   
   The bonus bug: with no pushed predicate and identical logical and physical 
schemas, the DataFusion opener skips the expression adapter entirely, which is 
exactly the shape of a metadata-free file, so even the refusal path was 
silently bypassed. Rebase-enabled scans now stamp a schema marker that keeps 
the adapter engaged, with a test pinning the ancient-value refusal on that 
exact shape.
   
   Rebase expression benchmark as requested (M5, release, real module code): 
per row the wrapper costs 0.7 to 2.2ns depending on type and policy, 0.26 to 
0.29ns for the check-only path, against effectively zero unwrapped. End to end 
on 20M rows the full-scan cost is noise (0.225 to 0.235s across corrected, 
legacy modern, legacy ancient; Spark fallback 0.36 to 0.47s). The honest cost 
is selective predicates: the opaque wrapper disables pruning on wrapped 
columns, and on a sorted table a point predicate decoded 2,627,592 rows against 
40,000 with pruning live, a 65x read amplification that warm cache hides but 
cold or remote storage will not. That is the operational tradeoff of the legacy 
path; correct results either way, counts identical across all modes.
   
   Batteries at this head: 231/231 on both spark-3.5 and spark-4.0, 327 native 
lib tests, clippy and fmt clean.
   


-- 
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]

Reply via email to