You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "jordepic (via GitHub)" <gi...@apache.org> on 2026/08/07 15:05:49 UTC
[PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
jordepic opened a new pull request, #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298
## Which issue does this PR close?
(Partially, second of three) Closes #4322.
## Rationale for this change
The first part of this work (#4658) split the Iceberg V2 write into a "writer" operator (inside AQE) and a "committer" operator, with the writer still running through iceberg-java. The end goal is to replace the writer's per-task parquet write with iceberg-rust — but only when the native write would produce the same outcome as iceberg-java: the same parquet features, statistics, and manifest metadata. If we can't reproduce a write exactly, we should fall back and say why.
This PR adds that detection layer. It contains no native writing (that's part three); it decides eligibility and reports the reason for every fall-back, so the compatibility contract is reviewable and pinned by tests before the native writer lands.
## What changes are included in this PR?
1) A `CometIcebergNativeWrite` operator serde, registered in `CometExecRule` and gated by a new `spark.comet.write.iceberg.nativeAcceleration.enabled` config (off by default). Its `getSupportLevel` inspects the write and either accepts it or reports the first failing check via Comet's extended EXPLAIN; conversion is stubbed ("not yet implemented") until part three.
2) The eligibility rules follow an allowlist philosophy: most Iceberg write settings are NOT supported. Only vetted `write.parquet.*` keys are accepted (a fixed set that the native writer will translate exactly, plus a few value-checked ones like `page-version=v1` and row-group-check counts at their defaults); any other `write.parquet.*` or `parquet.*` key — including keys added by future Iceberg versions — falls back. Checks run on the effective configuration (table properties overlaid with `SparkWrite.writeProperties`, so per-write options and `spark.sql.iceberg.*` session overrides are honoured), and also cover the resolved write format, format version, encryption, metrics modes, custom FileIO/location providers, storage schemes, and `parquet.*` keys in the session Hadoop configuration.
3) Documentation: `iceberg-writes.md` now states the supported configuration as a whitelist table, documents the unconditional divergences accepted behind the toggle (footer key-value metadata, page CRCs, encoding labels, partition-path escaping, row-group/file-rolling cadence), and records the plan to assemble manifest `DataFile` metrics on the JVM at commit time using Iceberg's own `MetricsConfig`, which will let the remaining `counts`/`none` metrics restrictions be lifted in part three.
4) Reflection helpers in `IcebergReflection` for reading `SparkWrite` internals (format, table, writeProperties, data location), with every reflection failure treated as a fall-back rather than a pass.
## How are these changes tested?
A new `CometIcebergWriteDetectionSuite` (32 tests) pins every rule: one negative test per fall-back trigger asserting the reason string, positive tests for the boundary cases (defaults, explicit metrics modes, session-conf codec overrides, s3 data locations, partitioned tables), a smoke test that every reflection accessor resolves against the Iceberg runtime on the classpath, and a test that the planner registration records a fall-back reason on the write exec. The suite passes on the spark-3.4 (Iceberg 1.5.2, Scala 2.12) and spark-3.5 (Iceberg 1.8.1) profiles.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3776346703
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteDetectionSuite.scala:
##########
@@ -0,0 +1,685 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.comet
+
+import java.io.File
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.CometTestBase
+import org.apache.spark.sql.comet.IcebergWriteExec
+
+import org.apache.comet.CometSparkSessionExtensions.isSpark35Plus
+import org.apache.comet.iceberg.IcebergReflection
+import org.apache.comet.serde.{Compatible, SupportLevel, Unsupported}
+import org.apache.comet.serde.operator.CometIcebergNativeWrite
+
+class CometIcebergWriteDetectionSuite extends CometTestBase with CometIcebergTestBase {
+
+ override protected def sparkConf: SparkConf = {
+ super.sparkConf
+ .set(CometConf.COMET_ICEBERG_WRITE_SPLIT_OPERATOR_ENABLED.key, "true")
+ .set(CometConf.COMET_ICEBERG_NATIVE_WRITE_ENABLED.key, "true")
+ }
+
+ test("clean parquet V2 table planned as AppendData yields Compatible") {
+ assume(icebergAvailable, "Iceberg not available in classpath")
+ withDetectionCatalog { dir =>
+ createTable(dir, "ok", partitionSpec = "")
+ assertSupportLevelIs[Compatible]("ok")
+ }
+ }
+
+ test("registration tags a fall-back reason on the write exec") {
+ assume(icebergAvailable, "Iceberg not available in classpath")
Review Comment:
Good call — done. The suite now overrides `test` with the `assume` (same pattern as `CometFuzzIcebergBase`), and the per-test calls are gone.
##########
spark/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -131,6 +131,16 @@ object CometConf extends ShimCometConf {
.booleanConf
.createWithDefault(false)
+ val COMET_ICEBERG_NATIVE_WRITE_ENABLED: ConfigEntry[Boolean] =
+ conf("spark.comet.write.iceberg.nativeAcceleration.enabled")
Review Comment:
Done — renamed to `spark.comet.iceberg.write.enabled` to mirror `spark.comet.parquet.write.enabled`.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5257536482
@andygrove they passed! thank you!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3738734831
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
My apologies, I knew that and forgot, all done
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5268237299
@andygrove Thank you for your quick reviews here! I cannot merge myself, but when this gets merged I'll get to work putting up patchset 3! Getting close :)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5221624772
@andygrove would you mind another CI kick?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5220294087
@andygrove would you mind kicking CI off on this one? Sorry, I fixed a formatting issue
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3737868842
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
Talking about a planned follow-up here is confusing to me. Is this PR the planned follow-up, or should this link to an issue?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5220308135
> @andygrove sounds good on the communications front, good point!
It also helps the LLMs follow along!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3738694381
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
Sorry, I meant could you include the link in the docs.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3771131615
##########
spark/src/test/scala/org/apache/comet/CometIcebergWriteDetectionSuite.scala:
##########
@@ -0,0 +1,685 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.comet
+
+import java.io.File
+
+import org.apache.spark.SparkConf
+import org.apache.spark.sql.CometTestBase
+import org.apache.spark.sql.comet.IcebergWriteExec
+
+import org.apache.comet.CometSparkSessionExtensions.isSpark35Plus
+import org.apache.comet.iceberg.IcebergReflection
+import org.apache.comet.serde.{Compatible, SupportLevel, Unsupported}
+import org.apache.comet.serde.operator.CometIcebergNativeWrite
+
+class CometIcebergWriteDetectionSuite extends CometTestBase with CometIcebergTestBase {
+
+ override protected def sparkConf: SparkConf = {
+ super.sparkConf
+ .set(CometConf.COMET_ICEBERG_WRITE_SPLIT_OPERATOR_ENABLED.key, "true")
+ .set(CometConf.COMET_ICEBERG_NATIVE_WRITE_ENABLED.key, "true")
+ }
+
+ test("clean parquet V2 table planned as AppendData yields Compatible") {
+ assume(icebergAvailable, "Iceberg not available in classpath")
+ withDetectionCatalog { dir =>
+ createTable(dir, "ok", partitionSpec = "")
+ assertSupportLevelIs[Compatible]("ok")
+ }
+ }
+
+ test("registration tags a fall-back reason on the write exec") {
+ assume(icebergAvailable, "Iceberg not available in classpath")
Review Comment:
maybe we can override `test` and include `assume(icebergAvailable, "Iceberg not available in classpath")` there to avoid calling it every test?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5281948980
@andygrove would you mind kicking CI once more? I addressed @comphead 's nits. Would love to merge after that though! Seems you, Parth and Oleks are generally okay with 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3738091454
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
Perhaps you could link to an issue describing the work in 3/3?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3738734831
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
My apologies, I knew that and forgot, all done, may need to kick CI again
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3737930944
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
3/3 is the planned follow up - how would you prefer I handle 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5241789778
@andygrove Thanks for the thorough review — every point is addressed in the current push. Point by point:
1. **PropertyKeys init / catch width**: the two strict rule vals are now `lazy`, so no `PropertyKeys` access happens during object initialization — every reflective constant lookup now occurs inside `getSupportLevel`'s try, and Scala lazy vals retry on failure so nothing gets poisoned. The catch is widened to `NonFatal`.
2. **Allowlist scope / fanout**: part 3 implements both clustered and fanout writer modes (mirroring `SparkWriteConf.useFanoutWriter`), so `write.spark.fanout.enabled` is declared supported rather than gated, and `write.target-file-size-bytes` rides the documented file-rolling divergence — both now have explicit rows in the doc table and tests. The "everything else must be absent" wording is reworded to state exactly which namespaces the allowlist enforces (`write.parquet.*`, `parquet.*`) and why other `write.*` keys are out of scope (plan-shaping, committer-side, non-parquet formats, or merge-on-read which the split plan never intercepts).
3. **Metrics modes**: inverted into a single allowlist rule — `full` and `truncate(N)` parse, anything else (including `truncat(16)` typos and future modes) falls back, with a test. The redundant `bloom-filter-max-bytes` and `stats-enabled.column.*` rules are gone; the property allowlist subsumes them.
4. **format-version**: the `None` case now falls back with "could not determine the table format-version".
5. **getOuterSparkWrite**: a missing `this$0` now returns `None`, so the reported reason is "could not unwrap SparkWrite".
6. **Compression**: moved to the accepted-divergences list (zstd default levels, LZ4 framing — byte-identical output isn't achievable even for a default table), and the "must match exactly" closing no longer claims compression.
7. **Registration**: the `CometExecRule` arm is now guarded by the config flag, so the default configuration tags nothing at all — no spurious fall-back reason on Comet's own operator. A test pins that no reason is recorded when the flag is off. I kept the wiring (rather than deferring to part 3) so the explain plumbing is exercised when users opt in.
8. **Test writes**: `captureWriteExec` is strict by default and fails the test if the INSERT throws; only the fixtures that deliberately write to unreachable locations or set broken classes (hdfs/s3/gs/oss/memory schemes, encryption, custom location provider / FileIO) tolerate failure.
9. **Coverage**: added tests for fanout, target-file-size, gs/oss/memory and explicit `file://` schemes, the disabled-flag behavior, a stubbed non-Iceberg `BatchWrite`, the metrics typo, and `write.parquet.shred-variants`. The "could not unwrap SparkWrite" branch isn't directly constructible (it needs a real `SparkWrite$` inner class without `this$0`), so it's covered only by the code path being fail-closed.
Your version-drift concern was on the money: running the suite on all four profiles locally caught two real issues the 3.4/3.5-only verification missed, and both are what CI was failing on. Iceberg 1.11 on Spark 4.x unconditionally resolves `write.parquet.shred-variants=false` into every parquet write's `writeProperties` (now vetted, with `true` gated since shredding changes the parquet schema), and Hadoop 3.4's `core-default.xml` ships `io.compression.codec.zstd.level`, which made gating codec-level keys via the Hadoop conf a false positive on every Spark 4 write — those gates are removed and folded into the accepted compressed-bytes divergence (they can only shift compressed bytes). The suite (40 tests) is green locally on spark-3.4/1.5.2, 3.5/1.8.1, 4.0/1.10, and 4.1/1.11.
Would you mind approving CI once more? The doc-link push reset the previous approval.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3771169752
##########
spark/src/main/scala/org/apache/comet/CometConf.scala:
##########
@@ -131,6 +131,16 @@ object CometConf extends ShimCometConf {
.booleanConf
.createWithDefault(false)
+ val COMET_ICEBERG_NATIVE_WRITE_ENABLED: ConfigEntry[Boolean] =
+ conf("spark.comet.write.iceberg.nativeAcceleration.enabled")
Review Comment:
reg to param name would be nice to be in sync with existing writer param name
```
conf("spark.comet.parquet.write.enabled")
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5282525513
@andygrove I believe the failing test is transient
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3737930944
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
3/3 is the planned follow up
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3738524850
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
See #5308
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "parthchandra (via GitHub)" <gi...@apache.org>.
parthchandra commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5273928475
I think we can merge this.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5218846827
Hey @andygrove , @comphead , @mbutrovich , @parthchandra ! Here's commit #2/3 in accelerating iceberg writes. I've had to deal with a lot of ambiguity in this one when decided what we do and don't want to accelerate, but I think it's a pretty comprehensive list at this point and covers the edge cases. Feel free to reply here, iceberg's slack, or the datafusion discord if you want to discuss more or have a call. Thanks!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#discussion_r3738694381
##########
docs/source/user-guide/latest/iceberg-writes.md:
##########
@@ -95,3 +98,84 @@ The rewrite is skipped — and the write runs through Spark's stock combined ope
In every fallback case the write is planned as if Comet were absent; there is no correctness
trade-off, only no plan change.
+
+## Native Parquet write eligibility
+
+A planned follow-up replaces the `IcebergWrite` operator's per-task Parquet write with
Review Comment:
Sorry, I meant could you include the link the docs.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5220294166
> Hey @andygrove , @comphead , @mbutrovich , @parthchandra ! Here's commit 2/3 in accelerating iceberg writes. I've had to deal with a lot of ambiguity in this one when deciding what we do and don't want to accelerate, but I think it's a pretty comprehensive list at this point and covers the edge cases. Feel free to reply here, iceberg's slack, or the datafusion discord if you want to discuss more or have a call. Thanks!
Thanks @jordepic. I appreciate the offer of the direct communications channels, but we prefer to keep discussions in issues/PRs so that the whole community can follow along. All issue/PR discussions get copied to a mailing list and archived as well. This is an important part of the Apache process.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "jordepic (via GitHub)" <gi...@apache.org>.
jordepic commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5220300011
@andygrove sounds good on the communications front, good point!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] feat: detect Iceberg V2 writes and emit fall-back reasons [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #5298:
URL: https://github.com/apache/datafusion-comet/pull/5298#issuecomment-5268645317
@mbutrovich @comphead @parthchandra any objection to merging this one?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org