fix: serialize version metadata through JNI and correct row-ID lookup by ivscheianu · Pull Request #6465 · lance-format/lance

ivscheianu · 2026-04-09T10:23:50Z

Summary

Two bugs in the stable row ID + CDF version tracking pipeline caused _row_created_at_version to report incorrect values after updates.

Bug 1: JNI serialization gap for version metadata

FragmentMetadata round-trips through JNI (Rust -> Java -> Rust) lost created_at_version_meta and last_updated_at_version_meta. The JNI layer serialized RowIdMeta but hard-coded None for both version metadata fields on the Java->Rust path, so any version information attached by lance-core was silently dropped when the connector returned fragments.

Fix: Add VersionMeta.java (mirrors RowIdMeta), extend FragmentMetadata with two new VersionMeta fields, and update lance-jni/src/fragment.rs to serialize/deserialize RowDatasetVersionMeta via JSON through the new Java type. Also extracted a generic extract_nullable_field<T>() helper to reduce duplicated nullable-getter boilerplate.

Bug 2: Incorrect row-ID lookup in `Operation::Update`

When preserving created_at versions for updated rows, the code assumed row_id >> 32 yields the original fragment ID. This is only true for unstable row addresses. Stable row IDs are sequential integers unrelated to
fragment layout. Any dataset with stable row IDs would look up the wrong fragment and fall back to a default version of 1.

Fix: Build a reverse index (stable row ID -> (fragment ref, offset)) from each existing fragment's RowIdMeta sequence, then look up each new fragment's row IDs in this index to find the correct original fragment and row offset.

Additional improvements

RowIdMeta.fromRowIds(long[]) — static factory that encodes stable row IDs into the inline protobuf/JSON format expected by lance-core, centralizing logic that was previously hand-rolled in the connector.

Companion PR

This PR is paired with lance-format/lance-spark#407 which implements the native UPDATE path in the Spark connector: capturing _rowid during update(), attaching RowIdMeta to new fragments, and consolidating deletion handling on the driver.

… in Operation::Update

codecov · 2026-04-09T11:12:26Z

Codecov Report

❌ Patch coverage is 96.73367% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
rust/lance/src/dataset/transaction.rs	96.70%	12 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

…ow crash

ivscheianu · 2026-04-14T14:55:17Z

        let total_slots = stats.max - stats.min + 1;

-        let range_with_holes = 24 + 4 * n_holes as usize;
+        let range_with_holes = 24usize.saturating_add(4usize.saturating_mul(n_holes as usize));


created #6515 for a long-term fix, will try to take care of the impl in a separate PR

andrei-ionescu · 2026-04-21T15:39:33Z

 }

+impl IntoJava for &RowDatasetVersionMeta {
+    fn into_java<'a>(self, env: &mut JNIEnv<'a>) -> Result<JObject<'a>> {


Nit pick... Let's use 'local for the lifetime, similar to the other Fragment.into_java().

'a is actually the dominant convention across all the other IntoJava impls in this file (DataFile, DeletionFile, RowIdMeta, FragmentMergeResult, etc.). Fragment is actually the outlier. My main concern was about consistency, happy to change it if you think it's the appropriate move

andrei-ionescu · 2026-04-21T15:52:42Z


+/// Fallback version for rows whose original creation version cannot be determined.
+/// Version 1 is the initial dataset version in the Lance format.
+const UNKNOWN_CREATED_AT_VERSION: u64 = 1;


I would suggest to rename it to DEFAULT_VERSION.

Suggested change

const UNKNOWN_CREATED_AT_VERSION: u64 = 1;

const DEFAULT_VERSION: u64 = 1;

I'd prefer to keep UNKNOWN_CREATED_AT_VERSION because the name captures why this value is used (fallback when the original creation version can't be determined), not just what it is. DEFAULT_VERSION would be a little ambiguous at the call sites since a reader would have to trace back to the constant definition to understand the intent

andrei-ionescu · 2026-04-21T15:59:32Z

+    frag_indices.sort_by_key(|&i| existing_fragments[i].id);
+    for &i in &frag_indices {


I see that we do to iterations over the existing_fragments. Can this be improved?

updated, thanks!

andrei-ionescu · 2026-04-21T16:03:33Z

+            None => None,
+        };
+
+        if let Some(row_ids) = row_ids {


I think this should be moved in the match above. This will also make code more readable and move the else branch in the None case.

Folding the if let Some(row_ids) into the Inline arm would require duplicating the fallback build_version_meta logic across three places: the Inline/Err sub-arm, the External arm, and the None arm. The current structure keeps that fallback in one else branch, which is the only reason the intermediate row_ids: Option<_> variable exists. Agreed it's not the most elegant shape, but I think it's less duplication than the alternative

andrei-ionescu · 2026-04-21T16:03:45Z

+
+            fragment.last_updated_at_version_meta = build_version_meta(fragment, new_version);
+        } else {
+            let version_meta = build_version_meta(fragment, new_version);


This can be moved in the None case.

related to the above #6465 (comment)

andrei-ionescu · 2026-04-21T16:09:47Z

+            let inherited = current_manifest
+                .map(|m| m.uses_stable_row_ids())
+                .unwrap_or(false);
+            let stable_row_ids = config.use_stable_row_ids || inherited;


Nitpick... the name of the variable should let us understand that it is a boolean, so use_stable_row_ids would be better.

fixed 👍🏻

andrei-ionescu · 2026-04-21T16:12:31Z

+        // build_manifest may have already set FLAG_STABLE_ROW_IDS on the manifest.
+        // Preserve it here so this second apply_feature_flags call does not clear it
+        // when config.use_stable_row_ids is false (the ManifestWriteConfig default).
+        let stable_row_ids = config.use_stable_row_ids || manifest.uses_stable_row_ids();


Ditto... use_stable_row_ids to show that is a boolean.

fixed 👍🏻

ivscheianu added 2 commits April 9, 2026 13:09

fix: serialize version metadata through JNI and correct row-ID lookup…

9220329

… in Operation::Update

Merge remote-tracking branch 'upstream/main'

16e3320

github-actions Bot added bug Something isn't working java labels Apr 9, 2026

ivscheianu mentioned this pull request Apr 9, 2026

feat: implement native update path with stable row-id preservation lance-format/lance-spark#407

Draft

ivscheianu added 2 commits April 9, 2026 13:31

chore: format

7929b1e

chore: fix clippy errors

49166c4

ivscheianu marked this pull request as ready for review April 9, 2026 10:45

ivscheianu and others added 2 commits April 9, 2026 14:37

chore: updated tests

831c480

Merge branch 'main' into stable-row-id

8432baf

This was referenced Apr 10, 2026

UPDATE COLUMNS FROM: bump _row_last_updated_at_version for matched rows lance-format/lance-spark#418

Open

ADD COLUMNS FROM should advance _row_last_updated_at_version for materialized rows lance-format/lance-spark#419

Open

ivscheianu and others added 7 commits April 13, 2026 07:41

Merge branch 'main' into stable-row-id

91e1467

Merge branch 'main' into stable-row-id

aede2f5

chore: partially refactor the code

09350b8

Merge branch 'main' into stable-row-id

beaa897

fix: saturating arithmetic in sorted_sequence_sizes to prevent overfl…

f2ec314

…ow crash

fix: updated implementation, fixed corner cases

23ab508

Merge branch 'main' into stable-row-id

418d74a

ivscheianu commented Apr 14, 2026

View reviewed changes

Merge branch 'main' into stable-row-id

42ab390

ivscheianu mentioned this pull request Apr 17, 2026

fix: prevent arithmetic overflow in U64Segment encoding selection for sparse/extreme row id ranges #6516

Open

ivscheianu added 3 commits April 18, 2026 07:58

Merge branch 'main' into stable-row-id

335d6f5

Merge branch 'main' into stable-row-id

051bf12

Merge branch 'main' into stable-row-id

112c21c

andrei-ionescu reviewed Apr 21, 2026

View reviewed changes

ivscheianu added 2 commits April 22, 2026 08:53

chore: addressed comments

2a49ff9

chore: format

04ad74f

	const UNKNOWN_CREATED_AT_VERSION: u64 = 1;
	const DEFAULT_VERSION: u64 = 1;

		frag_indices.sort_by_key(\|&i\| existing_fragments[i].id);
		for &i in &frag_indices {

Conversation

ivscheianu commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug 1: JNI serialization gap for version metadata

Bug 2: Incorrect row-ID lookup in Operation::Update

Additional improvements

Companion PR

Uh oh!

codecov Bot commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

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!

ivscheianu Apr 22, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ivscheianu commented Apr 9, 2026 •

edited

Loading

Bug 2: Incorrect row-ID lookup in `Operation::Update`

codecov Bot commented Apr 9, 2026 •

edited

Loading

ivscheianu Apr 22, 2026 •

edited

Loading