feat: support writing domain metadata (1/2) #1274

MannDP · 2025-09-09T19:03:54Z

What changes are proposed in this pull request?

This PR is (1/2) to support writing domain metadata.

(this PR) support adding or updating a domain metadata configuration
(follow up) support removing a domain metadata, feat: support writing domain metadata (2/2) #1275.

resolves Support domain metadata writes #1270

Writing domain metadata, as per the Delta protocol requires:

The table must be on writer version 7, and a feature named domainMetadata must exist in the table's writerFeatures. We satisfy this via an explicit check in Transaction::commit.
Two overlapping transactions conflict if they both contain a domain metadata action for the same metadata domain. We satisfy this with Kernel's coarse grained conflict detection based on the fact that the delta_log dir already has a log file with the name this txn was going to write. No new logic needed in this PR.

How was this change tested?

Added new integration tests in write.rs.

codecov · 2025-09-09T19:07:04Z

Codecov Report

❌ Patch coverage is 94.07407% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.55%. Comparing base (93f3f30) to head (ec20fed).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
kernel/src/transaction/mod.rs	94.95%	0 Missing and 6 partials ⚠️
test-utils/src/lib.rs	77.77%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1274      +/-   ##
==========================================
+ Coverage   84.48%   84.55%   +0.06%     
==========================================
  Files         113      113              
  Lines       27948    28009      +61     
  Branches    27948    28009      +61     
==========================================
+ Hits        23612    23683      +71     
+ Misses       3197     3189       -8     
+ Partials     1139     1137       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zachschuermann

quick drive-by review, dropping a few comments

kernel/src/transaction/mod.rs

kernel/src/actions/mod.rs

kernel/src/transaction/mod.rs

lbhm

Flushing some comments based on a quick skim. I did not look at the new tests.

kernel/src/transaction/mod.rs

zachschuermann

gonna definitely have conflicts with #1239, perhaps should just block on that merging. also - can we take on the stuff i mentioned here: #1239 (comment) ?

kernel/src/actions/mod.rs

kernel/src/transaction/mod.rs

… behavior change.

kernel/tests/write.rs

kernel/src/transaction/mod.rs

nicklan

looks great thanks, just a couple small things

kernel/src/transaction/mod.rs

kernel/tests/write.rs

kernel/src/transaction/mod.rs

lbhm · 2025-09-18T08:13:49Z

test-utils/src/lib.rs

    Ok(())
 }
+
+pub fn assert_result_error_with_message<T, E: ToString>(res: Result<T, E>, message: &str) {


Duplicating this method does not feel right to me. Should we rather move it here and import the method from the test_utils dependency in the main crate?
@zachschuermann, what do you think?

i looked into doing that this pr, but then the PR diff blows up. #1319 shows how the removal is to be done.

this would make a good starter task (if we want one)

duplicating a method is bad, but this is a simple test util so it's not a big deal

if we think it is a blocker, then i will open the pr linked above as a real pr and get it in.

I'm fine with the approach you suggested. I guess Zach has to make the call here.

yea we can just take in follow-up

nicklan

lgtm! thanks

kernel/src/transaction/mod.rs

zachschuermann

looks good after we close on a few final things! thanks @MannDP!!

kernel/src/transaction/mod.rs

zachschuermann · 2025-09-24T16:41:45Z

test-utils/src/lib.rs

    Ok(())
 }
+
+pub fn assert_result_error_with_message<T, E: ToString>(res: Result<T, E>, message: &str) {


yea we can just take in follow-up

kernel/src/transaction/mod.rs

MannDP self-assigned this Sep 9, 2025

zachschuermann reviewed Sep 9, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

kernel/src/actions/mod.rs Outdated Show resolved Hide resolved

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

MannDP force-pushed the manndp/write_domain_metadata branch from 305ddbf to 713a21c Compare September 9, 2025 22:21

MannDP marked this pull request as ready for review September 9, 2025 22:32

MannDP requested a review from zachschuermann September 9, 2025 22:39

MannDP changed the title ~~feat: support domain metadata writes~~ feat: support domain metadata writes (1/2) Sep 9, 2025

MannDP changed the title ~~feat: support domain metadata writes (1/2)~~ feat: support writing domain metadata (1/2) Sep 9, 2025

MannDP mentioned this pull request Sep 9, 2025

feat: support writing domain metadata (2/2) #1275

Open

MannDP requested review from nicklan, scovich and DrakeLin September 10, 2025 16:24

MannDP changed the title ~~feat: support writing domain metadata (1/2)~~ <WIP> feat: support writing domain metadata (1/2) Sep 10, 2025

MannDP marked this pull request as draft September 10, 2025 17:42

MannDP changed the title ~~<WIP> feat: support writing domain metadata (1/2)~~ feat: support writing domain metadata (1/2) Sep 10, 2025

MannDP marked this pull request as ready for review September 10, 2025 20:32

MannDP marked this pull request as draft September 10, 2025 20:49

MannDP changed the title ~~feat: support writing domain metadata (1/2)~~ <WIP> feat: support writing domain metadata (1/2) Sep 10, 2025

MannDP changed the title ~~<WIP> feat: support writing domain metadata (1/2)~~ feat: support writing domain metadata (1/2) Sep 10, 2025

MannDP marked this pull request as ready for review September 10, 2025 23:57

zachschuermann mentioned this pull request Sep 11, 2025

feat!: Add row tracking writer feature #1239

Merged

lbhm reviewed Sep 11, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

zachschuermann reviewed Sep 11, 2025

View reviewed changes

kernel/src/actions/mod.rs Outdated Show resolved Hide resolved

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

MannDP changed the title ~~feat: support writing domain metadata (1/2)~~ <wip> feat: support writing domain metadata (1/2) Sep 12, 2025

MannDP marked this pull request as draft September 12, 2025 20:05

MannDP force-pushed the manndp/write_domain_metadata branch from c7f5f6b to 7d90c15 Compare September 12, 2025 22:32

MannDP commented Sep 12, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

MannDP commented Sep 12, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

MannDP commented Sep 12, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

MannDP force-pushed the manndp/write_domain_metadata branch from 7d90c15 to f854284 Compare September 15, 2025 18:30

CODE MOVEMENT/REFACTOR ONLY! Implements the comments left on 1239. No…

c475873

… behavior change.

lbhm reviewed Sep 16, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

lbhm reviewed Sep 16, 2025

View reviewed changes

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

kernel/src/transaction/mod.rs Show resolved Hide resolved

MannDP force-pushed the manndp/write_domain_metadata branch from 7892501 to c475873 Compare September 16, 2025 16:30

impl pr feedback

9c89f0e

MannDP force-pushed the manndp/write_domain_metadata branch from 22b5b3c to 9c89f0e Compare September 16, 2025 17:54

github-actions bot added the breaking-change Change that require a major version bump label Sep 16, 2025

MannDP requested review from lbhm and zachschuermann September 16, 2025 18:06

nicklan reviewed Sep 17, 2025

View reviewed changes

kernel/src/transaction/mod.rs Show resolved Hide resolved

kernel/src/transaction/mod.rs Outdated Show resolved Hide resolved

kernel/src/transaction/mod.rs Show resolved Hide resolved

impl pr feedback

6f13346

MannDP force-pushed the manndp/write_domain_metadata branch from d99ebae to 6f13346 Compare September 17, 2025 23:35

MannDP requested a review from nicklan September 17, 2025 23:54

MannDP force-pushed the manndp/write_domain_metadata branch from 3dbc7ab to 6f13346 Compare September 18, 2025 00:03

Merge branch 'main' into manndp/write_domain_metadata

f84148e

MannDP commented Sep 18, 2025

View reviewed changes

kernel/tests/write.rs Outdated Show resolved Hide resolved

fix logical conflict

7866bb1

lbhm reviewed Sep 18, 2025

View reviewed changes

impl pr feedback

1aa7f66

MannDP force-pushed the manndp/write_domain_metadata branch from 0609301 to a0db3e5 Compare September 18, 2025 17:18

Merge branch 'main' into manndp/write_domain_metadata

7fc906f

MannDP force-pushed the manndp/write_domain_metadata branch from a0db3e5 to 7fc906f Compare September 18, 2025 17:19

nicklan approved these changes Sep 19, 2025

View reviewed changes

kernel/src/transaction/mod.rs Show resolved Hide resolved

zachschuermann approved these changes Sep 24, 2025

View reviewed changes

MannDP added 2 commits September 24, 2025 18:57

Merge branch 'main' into manndp/write_domain_metadata

60e0f34

pr cmts

ec20fed

MannDP force-pushed the manndp/write_domain_metadata branch from 2ba9817 to ec20fed Compare September 24, 2025 20:12

MannDP merged commit 05e6698 into delta-io:main Sep 24, 2025
21 checks passed

MannDP deleted the manndp/write_domain_metadata branch September 24, 2025 20:19

feat: support writing domain metadata (1/2) #1274

feat: support writing domain metadata (1/2) #1274

Uh oh!

Conversation

MannDP commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

How was this change tested?

Uh oh!

codecov bot commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zachschuermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lbhm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zachschuermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicklan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lbhm Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

MannDP Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

lbhm Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

zachschuermann Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

nicklan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zachschuermann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

zachschuermann Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MannDP commented Sep 9, 2025 •

edited

Loading

codecov bot commented Sep 9, 2025 •

edited

Loading