-
Notifications
You must be signed in to change notification settings - Fork 306
feat: Change default value of COMET_NATIVE_SCAN_IMPL to auto
#1933
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ae23bee
38d6643
db10be7
207d39b
facd4e3
5beaad8
a0bba56
e2fda71
248816d
79f77d6
ef5c0a9
4050677
5b23ef0
483cb8c
eedcde8
b996964
06a80fb
590388e
90c8183
31964b3
75e5c87
cca388d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,7 +34,7 @@ import org.apache.spark.sql.execution.datasources.v2.parquet.ParquetScan | |
| import org.apache.spark.sql.internal.SQLConf | ||
| import org.apache.spark.sql.types._ | ||
|
|
||
| import org.apache.comet.{CometConf, DataTypeSupport} | ||
| import org.apache.comet.{CometConf, CometSparkSessionExtensions, DataTypeSupport} | ||
| import org.apache.comet.CometConf._ | ||
| import org.apache.comet.CometSparkSessionExtensions.{isCometLoaded, isCometScanEnabled, withInfo, withInfos} | ||
| import org.apache.comet.parquet.{CometParquetScan, SupportsComet} | ||
|
|
@@ -261,6 +261,10 @@ case class CometScanRule(session: SparkSession) extends Rule[SparkPlan] { | |
|
|
||
| val fallbackReasons = new ListBuffer[String]() | ||
|
|
||
| if (CometSparkSessionExtensions.isSpark40Plus) { | ||
| fallbackReasons += s"$SCAN_NATIVE_ICEBERG_COMPAT is not implemented for Spark 4.0.0" | ||
| } | ||
|
|
||
| // native_iceberg_compat only supports local filesystem and S3 | ||
| if (!scanExec.relation.inputFiles | ||
| .forall(path => path.startsWith("file://") || path.startsWith("s3a://"))) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
However, there is a
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also supports HDFS if the feature is enabled
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't bother with s3:// and s3n:// urls. Those are defunct afaik. |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can revisit this after #1830 is merged