sunchao commented on code in PR #6753:
URL: https://github.com/apache/datafusion-comet/pull/6753#discussion_r4235507838
##########
spark/src/main/scala/org/apache/comet/rules/CometScanRule.scala:
##########
@@ -290,18 +290,16 @@ case class CometScanRule(session: SparkSession)
s"Unsupported filesystem schemes: ${unsupportedFsSchemes.mkString(",
")}")
return None
}
- // More than one bucket cannot be served by the single object store native
planning registers
- // per FilePartition; see aliasScanBuckets. Scoped to alias scans: plain
multi-bucket `s3://`
- // has the same flaw today and silently declining it would newly fall back
scans that work by
- // luck, so that widening is left as a separate decision. Note the sibling
Iceberg guard
- // (dataFileBuckets, below) is NOT so scoped -- it declines multi-bucket
`s3a://` too.
- val scanBuckets = CometScanRule.aliasScanBuckets(roots)
- if (scanBuckets.size > 1) {
- withFallbackReason(
- scanExec,
- "Native Parquet scan reads S3-compliant alias paths across multiple
buckets " +
- s"(${scanBuckets.toSeq.sorted.mkString(", ")}); Comet registers one
object store " +
- "per file partition and would read every file from the first file's
bucket")
+ // An early answer from the root paths. CometNativeScan.convert decides,
over the listed
+ // files and with the scheme lists from the Hadoop conf that native uses.
+ val multiStoreReason = CometScanUtils.multiStoreFallbackReason(
+ "Native Parquet scan",
+ roots.map(_.uri),
+ s3CompliantSchemes,
+ libhdfsSchemes,
Review Comment:
[P2] Use the effective Hadoop configuration for this early check. With
`fs.comet.s3Compliant.schemes=blob` and `fs.comet.libhdfs.schemes=hdfs,blob`
supplied through `core-site.xml`, a non-bucketed scan over
`s3a://bucket/a.parquet` and `blob://bucket/b.parquet` should remain native:
the stores are native S3 and libhdfs, and alias settings are not translated.
However, this `libhdfsSchemes` value comes from SQLConf and defaults to only
`hdfs`. The new check therefore treats both paths as native S3, reports an
alias collision, and immediately falls back to Spark. The base admitted these
roots, including previously safe scans with separate file partitions. Please
use `NativeConfig.resolveLibhdfsSchemes(hadoopConf)` here, consistently with
serialization and packing, and cover Hadoop-only configuration.
Evidence: Compiled the exact-head `NativeConfig` and changed
`CometScanUtils` helpers against Spark 4.1.3. A probe loaded both scheme
properties from Hadoop XML while leaving SQLConf unset and exercised the rule’s
configuration selection. Output: `base gate declines=false`; `early
routing=hdfs`; `effective routing=blob,hdfs`; the head early gate returned the
alias-collision reason, while the same check with effective Hadoop routing
returned `None`. Effective keys were `s3://bucket` and `blob://bucket
(libhdfs)`. Source inspection confirms the returned reason causes `nativeScan`
to return `None`. Reproduction:
`/tmp/comet6753-5b856-review-62crfd9k/src/EarlyGateProbe.scala`, output in
`EarlyGateProbe.log`. No object-store requests were made.
--
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]
