dwsmith1983 commented on code in PR #6753:
URL: https://github.com/apache/datafusion-comet/pull/6753#discussion_r4236401653
##########
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:
> Please use `NativeConfig.resolveLibhdfsSchemes(hadoopConf)` here,
consistently with serialization and packing, and cover Hadoop-only
configuration.
Done. The early check now reads both scheme lists from the Hadoop conf, as
`convert` and native do. A new `CometMultiStoreScanSuite` test sets
`fs.comet.s3Compliant.schemes=blob` and `fs.comet.libhdfs.schemes=hdfs,blob`
only as Hadoop keys and checks that a scan over `s3a://bucket` and
`blob://bucket` stays native, with each store's files in their own partitions.
The scheme gate above it still reads the libhdfs list from SQLConf, which
predates this PR; I filed #6837 for it.
--
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]
