Allow writes to CDF tables for add-only, remove-only, and non-data-ch… by giovanni-sorice · Pull Request #5567 · delta-io/delta

giovanni-sorice · 2025-11-24T16:20:41Z

Co-authored-by: Coauthor One andreAmorimF
Co-authored-by: Coauthor Two aryadneguardieiro
Co-authored-by: Coauthor Three yurimantchev-sudo

Which Delta project/connector is this regarding?

Description

Allow writes to CDF tables for add-only, remove-only, and non-data-change transactions.
More context: https://delta-users.slack.com/archives/C04TRPG3LHZ/p1763402127485709?thread_ts=1763400653.848629&cid=C04TRPG3LHZ

How was this patch tested?

Does this PR introduce any user-facing changes?

…ange transactions Co-authored-by: Coauthor One andreAmorimF Co-authored-by: Coauthor Two aryadneguardieiro Co-authored-by: Coauthor Three yurimantchev-sudo

OussamaSaoudi · 2025-11-25T18:33:02Z

+      assert(TableConfig.CHANGE_DATA_FEED_ENABLED.fromMetadata(snapshot.getMetadata))
+    }
+  }
+


Could we get test cases with removes?

Remove only succeeds

Add + remove with data change = false succeeds

Add + remove fails

scottsand-db · 2025-11-25T18:39:30Z

+      // Track CDF validation state: whether we've seen adds and removes
+      // with enableChangeDataFeed=true
+      // This is wrapped in an array to allow modification from within the lambda
+      final boolean[] hasAddWithDataChange = {false};


Nice! I think an AtomicBoolean would be a bit cleaner?

scottsand-db · 2025-11-25T18:41:07Z

+                      // Track removes with enableChangeDataFeed=true for table creation validation
+                      if (isCdfEnabled && removeFile.getDataChange()) {
+                        hasRemoveWithDataChange[0] = true;
+                        // Check if we've already seen adds with enableChangeDataFeed=true


I think instead of with enableChangeDataFeed=true you mean with dataChange = true?

scottsand-db · 2025-11-25T18:41:25Z

                      if (isAppendOnlyTable && removeFile.getDataChange()) {
                        throw DeltaErrors.cannotModifyAppendOnlyTable(dataPath.toString());
                      }
+                      // Track removes with enableChangeDataFeed=true for table creation validation


Can you help me understand: What here is specific to table creation?

Next, what logic here is doing an check for is-this-table-creation?

Actually, we based ourselves in this kernel-rs modification and there it doesn't seem we have a check specific to table creation 🤔 . Did we miss something maybe?

Sorry, I'm not clear what table creation has to do with here? Your comment says // Track removes with dataChange=true for table creation validation

True, the comment is misleading. Removing references to table creation.

andreAmorimF · 2025-12-11T10:25:00Z

+      // Kernel now supports writes to CDF-enabled tables with restrictions.
+      // Validation is performed at commit time to check for unsupported mixed operations
+      // (add+remove with dataChange=true) which would require CDC file generation.
+      return !TableConfig.ICEBERG_WRITER_COMPAT_V1_ENABLED.fromMetadata(metadata)


kernel iceberg doesn't supprot CDF

andreAmorimF · 2025-12-11T10:25:28Z

      boolean isTableFeatureOverrideKey =
          key.startsWith(TableFeatures.SET_TABLE_FEATURE_SUPPORTED_PREFIX);
      boolean isTableConfigKey = key.startsWith("delta.");
+      boolean isTableConstraintKey = key.startsWith("delta.constraints.");


delta.constraints.* should be a known feature. This was needed since all tests that was previously checking for changeDataFeed as unsupported feature, were changed to check against constraints.

I haven't thought about this deeply yet but I don't think this is the right way to handle this. I think ideally we don't need to add this code at all, but if we do it should definitely be in TableConfig not adhoc here. I can look at this closer when I get back from PTO.

This is a larger API change and we should think about this carefully. This may be the correct decision but we should figure out why. Because the tests are failing isn't a good enough reason to make this change.

cc @scottsand-db this is along the lines of allowing delta properties discussion we were having

Agreed with @allisonport-db, please revert this. This PR is meant for CDF, not constraints.

OussamaSaoudi · 2026-01-30T00:02:36Z

Hey @andreAmorimF, we're getting close! It seems like the failing test is coming from

  testIncompatibleUnsupportedTableFeature(
    "changeDataFeed",
    tablePropertiesToEnable = Map(TableConfig.CHANGE_DATA_FEED_ENABLED.getKey -> "true"))

I think this should become

  testIncompatibleUnsupportedTableFeature(
    "changeDataFeed",
    tablePropertiesToEnable = Map(TableConfig.CHANGE_DATA_FEED_ENABLED.getKey -> "true"))
    expectedErrorMessage = "Table features [changeDataFeed] are incompatible with icebergWriterCompatV1"))

this is in the IcebergWriterCompatV1Suite.scala ^

…ormat into relax_cdf_write_condition

…/DeltaTableWritesSuite.scala Co-authored-by: OussamaSaoudi <45303303+OussamaSaoudi@users.noreply.github.com>

scottsand-db · 2026-02-25T22:18:07Z

                      if (isAppendOnlyTable && removeFile.getDataChange()) {
                        throw DeltaErrors.cannotModifyAppendOnlyTable(dataPath.toString());
                      }
+                      // Track removes with dataChange=true for table creation validation


can you help me understand how this if statement applies to table creation? sorry, I don't see the connection

from what I understood, these checks run in all commit actions (including table creation). The issue here is that the comment is too specific to table creation, do I understand well?

If this is the problem and you are okay with it, I can remove these confusing comments since we already have some explanation of the tracking mechanism on these lines

scottsand-db · 2026-02-25T22:18:41Z

+                    }
+                    if (!action.isNullAt(ADD_FILE_ORDINAL)) {
+                      AddFile addFile = new AddFile(action.getStruct(ADD_FILE_ORDINAL));
+                      // Track adds with dataChange=true for table creation validation


Ditto here -- what does this have to do with table creation?

For example, I don't see a check elsewhere in the code of: if hasAddWithDataChange and isCreateTable then do X

scottsand-db

Looks great! Just a minor comment on your comment that I don't quite follow. Once you respond/fix, will merge! Thanks!

scottsand-db

Nice!

scottsand-db · 2026-03-20T20:25:14Z

Can you please rebase against master?

delta-io#5567) Co-authored-by: Coauthor One andreAmorimF Co-authored-by: Coauthor Two aryadneguardieiro Co-authored-by: Coauthor Three yurimantchev-sudo  #### Which Delta project/connector is this regarding?  - [ ] Spark - [ ] Standalone - [ ] Flink - [x] Kernel - [ ] Other (fill in here) ## Description Allow writes to CDF tables for add-only, remove-only, and non-data-change transactions. More context: https://delta-users.slack.com/archives/C04TRPG3LHZ/p1763402127485709?thread_ts=1763400653.848629&cid=C04TRPG3LHZ  ## How was this patch tested?  ## Does this PR introduce _any_ user-facing changes?  --------- Co-authored-by: André Fonseca <1077309+andreAmorimF@users.noreply.github.com> Co-authored-by: OussamaSaoudi <45303303+OussamaSaoudi@users.noreply.github.com>

giovanni-sorice added 2 commits November 24, 2025 17:20

Allow writes to CDF tables for add-only, remove-only, and non-data-ch…

5225103

…ange transactions Co-authored-by: Coauthor One andreAmorimF Co-authored-by: Coauthor Two aryadneguardieiro Co-authored-by: Coauthor Three yurimantchev-sudo

Split comment in TransactionImpl.java

7064393

giovanni-sorice marked this pull request as draft November 25, 2025 16:26

Remove trailing whitespace in TransactionImpl.java

12da593

OussamaSaoudi reviewed Nov 25, 2025

View reviewed changes

scottsand-db reviewed Nov 25, 2025

View reviewed changes

Comment thread kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/DeltaTableWritesSuite.scala

andreAmorimF force-pushed the relax_cdf_write_condition branch from 17e41f4 to 12da593 Compare December 2, 2025 12:23

andreAmorimF added 18 commits December 2, 2025 13:26

tracking with AtomicBoolean

6114bf8

Merge branch 'master' into relax_cdf_write_condition

83ec4d1

IcebergWriterCompact doesn't allow CDF

d8da967

Reformulating tests

1ece235

Fixing tests

a4f9895

Adding comments

eb27cb6

Organizing helper functions

727f5f2

Lint

37c41a1

Fixing integration test

3340917

Fixing integration test

0fe77c2

Removing comments

1d06217

Fixing test

c2ee9c5

Fixing test

1d9f8a2

Testing condition

1da3a33

Lint

f285b58

Debug

a84e33a

Debug

d2709f6

Fixing test

9493004

andreAmorimF reviewed Dec 11, 2025

View reviewed changes

andreAmorimF added 11 commits January 30, 2026 17:18

Iceberg compatibility test fix

65a46f0

Lint

c867de2

Iceberg compatibility test fix

9bdf983

Fixing asJava usage

4120901

Fixing asJava usage

80078c7

Fixing imcompatibility with map type

4d653da

Merge branch 'relax_cdf_write_condition' of github.com:nubank/delta-f…

c5192be

…ormat into relax_cdf_write_condition

lint

d5c9cbc

Merge branch 'master' into relax_cdf_write_condition

605f718

Fixing imcompatibility with map type

a533e25

Merge branch 'relax_cdf_write_condition' of github.com:nubank/delta-f…

b1d9f82

…ormat into relax_cdf_write_condition

OussamaSaoudi reviewed Feb 4, 2026

View reviewed changes

Comment thread kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults/DeltaTableWritesSuite.scala

OussamaSaoudi approved these changes Feb 4, 2026

View reviewed changes

OussamaSaoudi marked this pull request as ready for review February 4, 2026 00:39

andreAmorimF and others added 4 commits February 4, 2026 15:10

Update kernel/kernel-defaults/src/test/scala/io/delta/kernel/defaults…

3f9c402

…/DeltaTableWritesSuite.scala Co-authored-by: OussamaSaoudi <45303303+OussamaSaoudi@users.noreply.github.com>

lint

6d36231

fixing test

1d8acdd

Merge branch 'master' into relax_cdf_write_condition

5361748

OussamaSaoudi requested a review from scottsand-db February 9, 2026 18:35

scottsand-db reviewed Feb 25, 2026

View reviewed changes

scottsand-db requested changes Feb 25, 2026

View reviewed changes

andreAmorimF added 2 commits March 3, 2026 15:58

Removing comments

1270a8c

Merge branch 'master' into relax_cdf_write_condition

794569c

scottsand-db approved these changes Mar 20, 2026

View reviewed changes

Merge branch 'master' into relax_cdf_write_condition

2d6198f

scottsand-db merged commit cca7c70 into delta-io:master Mar 24, 2026
33 checks passed

Conversation

giovanni-sorice commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which Delta project/connector is this regarding?

Description

How was this patch tested?

Does this PR introduce any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottsand-db Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreAmorimF Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OussamaSaoudi commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scottsand-db left a comment

Choose a reason for hiding this comment

Uh oh!

scottsand-db left a comment

Choose a reason for hiding this comment

Uh oh!

scottsand-db commented Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

giovanni-sorice commented Nov 24, 2025 •

edited

Loading

scottsand-db Nov 25, 2025 •

edited

Loading

andreAmorimF Dec 11, 2025 •

edited

Loading

OussamaSaoudi commented Jan 30, 2026 •

edited

Loading