You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "rishi-rana (via GitHub)" <gi...@apache.org> on 2026/09/18 00:55:53 UTC
[PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
rishi-rana opened a new pull request, #18159:
URL: https://github.com/apache/iceberg/pull/18159
Closes #18131
## Problem
A copy-on-write `DELETE` on a v3 table fails Spark's per-rule plan validation:
```
[PLAN_VALIDATION_FAILED_RULE_IN_BATCH] Rule GroupBasedRowLevelOperationScanPlanning
in batch Early Filter and Projection Push-Down generated an invalid plan:
The plan becomes unresolved: 'ReplaceData RelationV2[id, data] ...
+- Filter NOT ((id = 101) <=> true)
+- RelationV2[id, data, _file, _pos, _row_id, _last_updated_sequence_number] ...
```
`SparkCopyOnWriteScan.rowLineageAsDataCols` removes the `__metadata_col` marker from
`_row_id` and `_last_updated_sequence_number` so the 3.5 row-level rules do not drop them.
`ReplaceData.dataInput` filters out metadata attributes, so both lineage columns stay in it,
giving four columns against a target relation that supplies two, and `outputResolved` fails.
Only the intermediate plan is affected, which is why this surfaces just under plan validation.
Spark 4.x does not override `readSchema()` at all and relies on the native metadata column
semantics from SPARK-50820, matching the version split in the issue.
## Fix
`UPDATE` and `MERGE` already append the lineage attributes to the target relation output in
`RewriteUpdateTableForRowLineage` and `RewriteMergeIntoTableForRowLineage`. `DELETE` had no
equivalent, so this adds `RewriteDeleteFromTableForRowLineage` alongside them. It needs no
assignments because copy-on-write `DELETE` carries surviving rows over unchanged.
`shouldUpdatePlan` now also returns false for non-Iceberg tables. Its inner match had no
default branch, and `DELETE` newly reaches it, so a `DELETE` against another v2 catalog in a
session with these extensions loaded would hit a `MatchError`.
I also considered dropping the `rowLineageAsDataCols` workaround instead. That is not viable:
with it removed the lineage columns are no longer written, and `TestCopyOnWriteWithLineage`
fails with `Structs do not match` and `ArrayIndexOutOfBoundsException` rather than a plan
validation error. The workaround is still required.
## Testing
No new test was needed. The existing `testDelete` reproduces this once validation is on, so
this enables `spark.sql.planChangeValidation` in `ExtensionsTestBase`. Spark gates per-rule
validation on the `spark.testing` **system property**, but the harness sets `spark.testing`
as a Spark conf, so validation was never actually running.
With validation enabled and without the fix, 30 tests fail across `TestCopyOnWriteDelete`
and `TestCopyOnWriteWithLineage`. With the fix the full `spark-extensions` suite passes:
2501 passed, 180 skipped, 0 failures (`TestRewritePositionDeleteFilesProcedure` hit an
unrelated flaky REST catalog 404 during initialization in one run and passes on its own).
## Notes for reviewers
- The rule is not restricted to copy-on-write. `RewriteUpdateTableForRowLineage` and
`RewriteMergeIntoTableForRowLineage` do not distinguish either, and I verified merge-on-read
is unaffected (`TestMergeOnReadWithLineage` 40/40). Say the word if you would rather it were
narrowed to the group-based path.
- Unlike the sibling rules, this one falls through to the unchanged plan instead of risking a
`MatchError`, and guards on `resolved` the way Spark's own `RewriteDeleteFromTable` and
`RewriteMergeIntoTableForRowLineage` do.
- Enabling plan validation for the whole extensions suite is deliberate, to stop this class of
bug recurring, but it is the part of this change most likely to affect unrelated tests. Happy
to scope it to the row lineage tests instead.
---
**AI Disclosure**
- Model: Claude Opus 5
- Platform/Tool: Claude Code
- Human Oversight: partially reviewed
- Prompt Summary: Investigate #18131, identify the root cause of the copy-on-write DELETE plan
validation failure, and implement a fix with regression coverage. The agent reproduced the
failure, traced it to the missing DELETE counterpart of the existing row lineage rules,
empirically ruled out the alternative fix, and ran the full spark-extensions suite.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "rishi-rana (via GitHub)" <gi...@apache.org>.
rishi-rana commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4077378312
##########
spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/RewriteOperationForRowLineage.scala:
##########
@@ -42,6 +42,7 @@ trait RewriteOperationForRowLineage extends RewriteRowLevelCommand {
r.table match {
case sparkTable: SparkTable =>
TableUtil.supportsRowLineage(sparkTable.table())
+ case _ => false
Review Comment:
Added in b1981589a: `TestRowLineageWithForeignCatalog`, with a minimal v2 catalog whose tables are
not Iceberg tables, and a `DELETE` against one of them.
The test is coupled to this line — dropping the branch makes it fail with:
```
scala.MatchError: ...TestRowLineageWithForeignCatalog$ForeignTable
(of class ...TestRowLineageWithForeignCatalog$ForeignTable)
```
One thing worth flagging while writing it: a plain non-Iceberg table under `spark_catalog`
(`CREATE TABLE ... USING parquet`) does *not* reach this branch. It resolves to a v1 relation, so
the outer `case _ => false` already handles it and you get a normal `UNSUPPORTED_FEATURE.TABLE_OPERATION`
error. It takes a genuine third-party v2 catalog registered alongside Iceberg to get here, which is
why the test needs its own catalog rather than reusing an existing fixture.
Also note this branch fixes a latent issue rather than only one I introduced: `shouldUpdatePlan` is
shared with `RewriteUpdateTableForRowLineage` and `RewriteMergeIntoTableForRowLineage`, so `UPDATE`
and `MERGE` against a foreign v2 catalog could already hit the same `MatchError` before this PR. The
new `DELETE` rule just made it easier to reach. Happy to split it into its own PR if you would rather
keep this one to the plan validation fix.
Full `spark-extensions` 3.5 suite passes with the new test (0 failures), and `spotlessCheck` /
`scalastyleMain` are clean.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "rishi-rana (via GitHub)" <gi...@apache.org>.
rishi-rana commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4078462913
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRowLineageWithForeignCatalog.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.iceberg.spark.extensions;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.SparkCatalogConfig;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.connector.catalog.Identifier;
+import org.apache.spark.sql.connector.catalog.SupportsDeleteV2;
+import org.apache.spark.sql.connector.catalog.SupportsRead;
+import org.apache.spark.sql.connector.catalog.Table;
+import org.apache.spark.sql.connector.catalog.TableCapability;
+import org.apache.spark.sql.connector.catalog.TableCatalog;
+import org.apache.spark.sql.connector.catalog.TableChange;
+import org.apache.spark.sql.connector.expressions.Transform;
+import org.apache.spark.sql.connector.expressions.filter.Predicate;
+import org.apache.spark.sql.connector.read.Batch;
+import org.apache.spark.sql.connector.read.InputPartition;
+import org.apache.spark.sql.connector.read.PartitionReaderFactory;
+import org.apache.spark.sql.connector.read.Scan;
+import org.apache.spark.sql.connector.read.ScanBuilder;
+import org.apache.spark.sql.types.StructType;
+import org.apache.spark.sql.util.CaseInsensitiveStringMap;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Verifies that the row lineage rules leave tables from other v2 catalogs alone. The rules run on
+ * every resolved row-level command, not only on Iceberg tables.
+ */
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestRowLineageWithForeignCatalog extends ExtensionsTestBase {
Review Comment:
It's a minimal `TableCatalog` defined at the bottom of this same file, standing in for any
third-party v2 catalog a user might register alongside Iceberg (Delta, JDBC v2, a vendor connector).
Its tables are plain `Table` implementations rather than `SparkTable`, which is the case
`shouldUpdatePlan` has to tolerate.
I would have preferred Spark's own `InMemoryTableCatalog` fixture, but it ships only in the
spark-sql / spark-catalyst *test* jars, and `iceberg-spark-extensions` doesn't depend on those, so
there is nothing on the test classpath to reuse. Hence the ~90 lines of stub.
Happy to rename it if `ForeignCatalog` isn't clear — `NonIcebergCatalog` is probably more direct
about what it is testing. Let me know which you prefer.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#issuecomment-5723713957
@rishi-rana Is this fix Spark 3.5 specific? If not, open a PR against Spark 4.2 first.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4078428366
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/ExtensionsTestBase.java:
##########
@@ -51,6 +51,7 @@ public static void startMetastoreAndSpark() {
.master("local[2]")
.config("spark.driver.host", InetAddress.getLoopbackAddress().getHostAddress())
.config("spark.testing", "true")
+ .config("spark.sql.planChangeValidation", "true")
Review Comment:
@huaxingao @szehon-ho Is this worth enabling for all Spark versions?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4078570872
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRowLineageWithForeignCatalog.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.iceberg.spark.extensions;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.SparkCatalogConfig;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.connector.catalog.Identifier;
+import org.apache.spark.sql.connector.catalog.SupportsDeleteV2;
+import org.apache.spark.sql.connector.catalog.SupportsRead;
+import org.apache.spark.sql.connector.catalog.Table;
+import org.apache.spark.sql.connector.catalog.TableCapability;
+import org.apache.spark.sql.connector.catalog.TableCatalog;
+import org.apache.spark.sql.connector.catalog.TableChange;
+import org.apache.spark.sql.connector.expressions.Transform;
+import org.apache.spark.sql.connector.expressions.filter.Predicate;
+import org.apache.spark.sql.connector.read.Batch;
+import org.apache.spark.sql.connector.read.InputPartition;
+import org.apache.spark.sql.connector.read.PartitionReaderFactory;
+import org.apache.spark.sql.connector.read.Scan;
+import org.apache.spark.sql.connector.read.ScanBuilder;
+import org.apache.spark.sql.types.StructType;
+import org.apache.spark.sql.util.CaseInsensitiveStringMap;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Verifies that the row lineage rules leave tables from other v2 catalogs alone. The rules run on
+ * every resolved row-level command, not only on Iceberg tables.
+ */
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestRowLineageWithForeignCatalog extends ExtensionsTestBase {
Review Comment:
Do you mean DSv2 catalog in Spark? Maybe just use SparkDSv2Catalog
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "rishi-rana (via GitHub)" <gi...@apache.org>.
rishi-rana commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4117929238
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/ExtensionsTestBase.java:
##########
@@ -51,6 +51,7 @@ public static void startMetastoreAndSpark() {
.master("local[2]")
.config("spark.driver.host", InetAddress.getLoopbackAddress().getHostAddress())
.config("spark.testing", "true")
+ .config("spark.sql.planChangeValidation", "true")
Review Comment:
I ran the full `spark-extensions` suite with `spark.sql.planChangeValidation` enabled on each of the
other supported Spark versions:
| Spark | tests | failures | plan-validation failures |
|---|---|---|---|
| 4.0 | 2718 | 21 | 0 |
| 4.1 | 2893 | 0 | 0 |
| 4.2 | 2952 | 0 | 0 |
The 21 failures on 4.0 are all `TestRewriteTablePathProcedure` hitting a local Hive metastore
(`TTransportException` / `MetaException`); that class passes when run on its own. No version produced
a `PLAN_VALIDATION_FAILED_RULE_IN_BATCH`.
One thing worth knowing for the decision: `ExtensionsTestBase` sets `spark.testing` as a Spark conf,
but Spark gates per-rule validation on `Utils.isTesting`, which reads the `spark.testing` **system
property**. So these suites have never actually had validation on, which is how the 3.5 issue here
went unnoticed.
So enabling it on 4.x is clean today and purely preventative — there is no bug to fix there. Happy to
do it as a follow-up PR so this one stays on the 3.5 fix, or to fold it in here if you'd rather have
it in one 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "rishi-rana (via GitHub)" <gi...@apache.org>.
rishi-rana commented on PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#issuecomment-5725398118
@manuzhang Yes, this one is Spark 3.5 specific — there is no 4.2 counterpart to port it to.
The row lineage rules for row-level operations only exist under `spark/v3.5`:
```
v3.5 RewriteOperationForRowLineage.scala RewriteUpdateTableForRowLineage.scala RewriteMergeIntoTableForRowLineage.scala
v4.0 (none)
v4.1 (none)
v4.2 (none)
```
Same for the trigger. `SparkCopyOnWriteScan` overrides `readSchema()` to strip the metadata column
marker off the lineage columns only in 3.5 (added in #12736); 4.0/4.1/4.2 have no override and rely
on the native metadata column semantics from SPARK-50820. Without that marker being stripped there is
no mismatch between `ReplaceData.dataInput` and the target relation output, so the plan stays resolved.
I verified this rather than assuming it: I enabled `spark.sql.planChangeValidation` in the 4.2
`ExtensionsTestBase` and ran the same three classes that fail on 3.5:
| | Spark 4.2 | Spark 3.5 before this PR |
|---|---|---|
| TestCopyOnWriteDelete | 112 passed, 20 skipped | 26 failed |
| TestCopyOnWriteWithLineage | 40 passed | 4 failed |
| TestMergeOnReadWithLineage | 40 passed | 40 passed |
192 passed / 0 failures on 4.2. I reverted that local 4.2 change since it is not part of this PR.
Happy to open a separate PR enabling `spark.sql.planChangeValidation` in the 4.x test harnesses if
you think it is worth having as a guard there too — it passes today, so it would be purely
preventative and seemed like a different concern from this fix.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4078433132
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRowLineageWithForeignCatalog.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.iceberg.spark.extensions;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.SparkCatalogConfig;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.connector.catalog.Identifier;
+import org.apache.spark.sql.connector.catalog.SupportsDeleteV2;
+import org.apache.spark.sql.connector.catalog.SupportsRead;
+import org.apache.spark.sql.connector.catalog.Table;
+import org.apache.spark.sql.connector.catalog.TableCapability;
+import org.apache.spark.sql.connector.catalog.TableCatalog;
+import org.apache.spark.sql.connector.catalog.TableChange;
+import org.apache.spark.sql.connector.expressions.Transform;
+import org.apache.spark.sql.connector.expressions.filter.Predicate;
+import org.apache.spark.sql.connector.read.Batch;
+import org.apache.spark.sql.connector.read.InputPartition;
+import org.apache.spark.sql.connector.read.PartitionReaderFactory;
+import org.apache.spark.sql.connector.read.Scan;
+import org.apache.spark.sql.connector.read.ScanBuilder;
+import org.apache.spark.sql.types.StructType;
+import org.apache.spark.sql.util.CaseInsensitiveStringMap;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Verifies that the row lineage rules leave tables from other v2 catalogs alone. The rules run on
+ * every resolved row-level command, not only on Iceberg tables.
+ */
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestRowLineageWithForeignCatalog extends ExtensionsTestBase {
Review Comment:
what is `ForeignCatalog`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4073515516
##########
spark/v3.5/spark-extensions/src/main/scala/org/apache/spark/sql/catalyst/analysis/RewriteOperationForRowLineage.scala:
##########
@@ -42,6 +42,7 @@ trait RewriteOperationForRowLineage extends RewriteRowLevelCommand {
r.table match {
case sparkTable: SparkTable =>
TableUtil.supportsRowLineage(sparkTable.table())
+ case _ => false
Review Comment:
Please add tests to cover 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "rishi-rana (via GitHub)" <gi...@apache.org>.
rishi-rana commented on PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#issuecomment-5763385048
@manuzhang let me know if I am missing something in my above findings
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 3.5: Add row lineage rule for DELETE to keep plans resolved [iceberg]
Posted by "rishi-rana (via GitHub)" <gi...@apache.org>.
rishi-rana commented on code in PR #18159:
URL: https://github.com/apache/iceberg/pull/18159#discussion_r4117928539
##########
spark/v3.5/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestRowLineageWithSparkDSv2Catalog.java:
##########
@@ -0,0 +1,194 @@
+/*
+ * 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.iceberg.spark.extensions;
+
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+import java.util.Map;
+import java.util.Set;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.spark.SparkCatalogConfig;
+import org.apache.spark.sql.catalyst.analysis.NoSuchTableException;
+import org.apache.spark.sql.connector.catalog.Identifier;
+import org.apache.spark.sql.connector.catalog.SupportsDeleteV2;
+import org.apache.spark.sql.connector.catalog.SupportsRead;
+import org.apache.spark.sql.connector.catalog.Table;
+import org.apache.spark.sql.connector.catalog.TableCapability;
+import org.apache.spark.sql.connector.catalog.TableCatalog;
+import org.apache.spark.sql.connector.catalog.TableChange;
+import org.apache.spark.sql.connector.expressions.Transform;
+import org.apache.spark.sql.connector.expressions.filter.Predicate;
+import org.apache.spark.sql.connector.read.Batch;
+import org.apache.spark.sql.connector.read.InputPartition;
+import org.apache.spark.sql.connector.read.PartitionReaderFactory;
+import org.apache.spark.sql.connector.read.Scan;
+import org.apache.spark.sql.connector.read.ScanBuilder;
+import org.apache.spark.sql.types.StructType;
+import org.apache.spark.sql.util.CaseInsensitiveStringMap;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Verifies that the row lineage rules leave tables from other v2 catalogs alone. The rules run on
+ * every resolved row-level command, not only on Iceberg tables.
+ */
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestRowLineageWithForeignCatalog extends ExtensionsTestBase {
Review Comment:
Renamed in 7ad872f30 — the catalog is now `SparkDSv2Catalog` and its table `SparkDSv2Table`, and the test class follows as `TestRowLineageWithSparkDSv2Catalog`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org