Skip to content

fix(java): rename DatasetDelta JNI bindings to match Java declarations#6963

Merged
yanghua merged 2 commits into
lance-format:mainfrom
LuciferYang:fix/datasetdelta-jni-naming
May 28, 2026
Merged

fix(java): rename DatasetDelta JNI bindings to match Java declarations#6963
yanghua merged 2 commits into
lance-format:mainfrom
LuciferYang:fix/datasetdelta-jni-naming

Conversation

@LuciferYang
Copy link
Copy Markdown
Contributor

@LuciferYang LuciferYang commented May 27, 2026

Summary

Fixes #6962. The Rust exports in java/lance-jni/src/delta.rs were named without the native prefix that the Java side declares, so every call into the three DatasetDelta native methods failed with UnsatisfiedLinkError. This renames the exports and fixes two unrelated DeltaTest fixture problems that the JNI bug had been silently masking.

Changes

  • Rename Java_org_lance_delta_DatasetDelta_listTransactions / getInsertedRows / getUpdatedRows in java/lance-jni/src/delta.rs to add the native prefix. releaseNativeDelta was already correct.
  • Switch both DeltaTest cases from memory:// URIs (which do not persist commits across separate Dataset.write() calls) to @TempDir-backed file URIs, matching every other test in the module.
  • Add .enableStableRowIds(true) to the v1 write in testInsertedRowsComparedAgainst so the expected _row_created_at_version / _row_last_updated_at_version row-lineage columns appear in the delta output.
  • Drop the now-dead catch (UnsatisfiedLinkError) -> Assumptions.assumeTrue(false, ...) skip pattern and the Assumptions import.
  • Extract a writeBatch() helper to deduplicate the Arrow batch serialization between the two tests.

Testing

  • ./mvnw test -Dtest=DeltaTest -DskipNativeBuild=true — both tests now pass instead of silently skipping.
  • nm -gU java/lance-jni/target/debug/liblance_jni.dylib | grep DatasetDelta — confirms the four expected exports (nativeListTransactions, nativeGetInsertedRows, nativeGetUpdatedRows, releaseNativeDelta).

The Java side declares three native methods using the nativeXxx
convention:

  private native List<Transaction> nativeListTransactions();
  private native void nativeGetInsertedRows(long streamAddress);
  private native void nativeGetUpdatedRows(long streamAddress);

But the Rust exports in java/lance-jni/src/delta.rs were named without
the `native` prefix:

  Java_org_lance_delta_DatasetDelta_listTransactions
  Java_org_lance_delta_DatasetDelta_getInsertedRows
  Java_org_lance_delta_DatasetDelta_getUpdatedRows

The JVM looks up `Java_<package>_<class>_<javaMethodName>`, so these
bindings never matched and every call into DatasetDelta.listTransactions,
getInsertedRows, or getUpdatedRows throws UnsatisfiedLinkError from the
published lance-core jar (verified on 7.0.0-rc.1). releaseNativeDelta
was already correctly named.

This rename was masked by DeltaTest catching UnsatisfiedLinkError and
calling Assumptions.assumeTrue(false, ...), so CI silently skipped the
broken tests instead of failing. Test cleanup is left for a separate
change to keep this fix focused.
The previous "catch UnsatisfiedLinkError -> Assumptions.assumeTrue(false)"
pattern silently skipped both tests for as long as the JNI binding mismatch
in delta.rs was in place. Once the preceding rename commit lets the native
methods resolve, two real test-setup defects surface:

* memory:// URI does not persist commits across separate Dataset.write()
  calls. Each .execute() returned a Dataset reference, but the v2 commit
  was not visible to a sibling Dataset reference reopening the URI. Switch
  both tests to a @TempDir-backed file URI, matching every other test in
  this module.

* testInsertedRowsComparedAgainst expects _row_created_at_version /
  _row_last_updated_at_version columns in the delta output. These row
  lineage columns are only emitted when the dataset is created with
  stable row IDs enabled. Add .enableStableRowIds(true) to the v1 write.

* Drop the now-dead UnsatisfiedLinkError catches and the Assumptions
  import; extract a writeBatch() helper to deduplicate the Arrow batch
  serialization between the two tests.

Both tests now genuinely cover the DatasetDelta JNI bindings end-to-end.
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions github-actions Bot added bug Something isn't working java labels May 27, 2026
@LuciferYang
Copy link
Copy Markdown
Contributor Author

LuciferYang commented May 27, 2026

cc @Xuanwo @hamersaw @jackye1995 FYI

Copy link
Copy Markdown
Collaborator

@yanghua yanghua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yanghua yanghua merged commit 3018497 into lance-format:main May 28, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working java

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DatasetDelta JNI bindings throw UnsatisfiedLinkError

2 participants