dwsmith1983 opened a new pull request, #6753:
URL: https://github.com/apache/datafusion-comet/pull/6753

   ## Which issue does this PR close?
   
   Closes #6746.
   Closes #6747.
   
   ## Rationale for this change
   
   The native scan reads every file of a Spark partition through one object 
store, built from that partition's first file, and keeps only each file's path. 
Spark packs small files from every root path into the same partitions, so a 
Parquet scan over two buckets fetched the second bucket's files from the first: 
wrong rows when the same key exists in both, `FILE_NOT_EXIST` otherwise. The 
native CSV scan went further and built one store from the first partition's 
first file for every partition, so it read the wrong bucket even when no 
partition mixed buckets.
   
   ## What changes are included in this PR?
   
   - `NativeConfig.objectStoreKey` gives the JVM the same store identity native 
uses (`normalize_object_store_url` plus `object_store_url_key`, and whether the 
scheme is routed through libhdfs). A shared fixture table is asserted on both 
sides.
   - `CometScanExec` packs each store's files separately before Spark's bin 
packing, so no partition mixes stores. A scan over one store packs exactly as 
before. A scan over several stores stays native and can end up with a different 
number of partitions than Spark would use.
   - The CSV native scan splits any partition that mixes stores, builds each 
partition's store from that partition's own files, and takes its partition 
count from the serialized partitions.
   - Native planning now refuses a partition that still mixes stores, with an 
error naming both, instead of reading through the wrong one.
   - Scans that cannot be served by one set of forwarded options fall back to 
Spark with a reason: files in more than one scheme family (for example `s3a` 
and `gs`, or an S3-compatible alias and `s3a`), and bucketed scans whose files 
span stores. The rule checks root paths early, and the serde checks the listed 
files, so partitions stored outside the table location are covered too.
   - A declined CSV conversion now falls back to Spark's own scan. Before, the 
`CometBatchScanExec` wrapper stayed in the plan and failed to execute row-based 
input.
   - `datasources.md` describes how scans over more than one object store are 
planned.
   
   One behaviour change against 1.1 that is not a bug fix: a scan whose files 
span scheme families, such as `s3a` and `hdfs`, ran natively before when Spark 
happened not to mix them in a partition. It now falls back, because the scan 
forwards the object store settings of one scheme only.
   
   ## How are these changes tested?
   
   - `CometMultiStoreScanSuite` (new, runs in CI): Parquet and CSV over two 
HDFS name nodes through a local test filesystem. It checks that Spark's layout 
mixes them and Comet's does not, that one name node keeps Spark's layout 
exactly, and that the partition count matches what the leaf exec reports. It 
covers the fallbacks with their reasons: mixed scheme families for Parquet and 
CSV, a catalog partition outside the table root, and a bucketed table with and 
without a partition filter, plus a one-store control. It also has a dynamic 
partition pruning case.
   - `CometScanSchemeFallbackSuite`: a seeded property test for the per-store 
packing (no mixed partition, every file once, each store packed as Spark packs 
it, one store unchanged), the CSV split, and the fallback reasons.
   - `NativeConfigSuite` and `parquet_support.rs`: the same store-key fixture 
on both sides, including libhdfs routing, aliases, hostless forms, case and 
ports.
   - Rust planner tests: a partition mixing buckets or schemes is rejected, 
each CSV partition gets its own store, an empty CSV partition reads no rows, 
and an out-of-range partition index is an error.
   - `ParquetReadFromS3Suite` and the new manual `CsvReadFromS3Suite` (MinIO): 
two buckets with the same and with distinct keys, mixed and separate 
partitions, with AQE on and off, dynamic partition pruning, and the bucketed 
fallback. These reproduce both issues on `main` and match Spark with this 
change.
   


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