Skip to content

feat(java): complete transaction operation mappings - #8925

Merged
majin1102 merged 6 commits into
lance-format:mainfrom
everySympathy:codex/java-transaction-mapping
Sep 4, 2026
Merged

feat(java): complete transaction operation mappings#8925
majin1102 merged 6 commits into
lance-format:mainfrom
everySympathy:codex/java-transaction-mapping

Conversation

@everySympathy

Copy link
Copy Markdown
Contributor

Summary

Complete the Java SDK and JNI transaction mapping against the Rust Transaction / Operation definitions.

  • add Java models and bidirectional JNI mappings for DataOverlay, UpdateMemWalState, Clone, and UpdateBases
  • complete existing mappings for:
    • Overwrite.initial_bases
    • Update.compacted_sstables, inserted_rows_filter, update mode, field ids, and fragment offsets
    • RewrittenIndex.new_index_files
    • index file metadata and derived size
  • cover all current Rust operation variants in both Rust-to-Java and Java-to-Rust conversion
  • preserve Optional / empty semantics where representable, canonicalize protobuf-equivalent empty metadata, and return diagnostic errors for lossy or unsupported values
  • reject unsigned-id/count overflow, invalid overlay coverage, invalid Bloom metadata, and inconsistent or overflowing index file sizes instead of silently wrapping or dropping values
  • retain compatibility accessors and the existing protected Overwrite constructor

Testing

  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings
  • cargo clippy --tests --manifest-path java/lance-jni/Cargo.toml -- -D warnings
  • cd java && ./mvnw -Dtest=TransactionTest,FragmentTest,UpdateTest test
    • 47 Java tests passed
    • 19 Rust JNI tests passed
  • cd java && ./mvnw test
    • 493 tests run, 0 failures, 13 errors, 26 skipped
    • all 13 errors are pre-existing external REST namespace tests receiving HTTP 403 (RestNamespaceTest and DynamicContextProviderTest); all other tests passed

The transaction tests cover Dataset.readTransaction(), every newly added operation, repaired fields, bidirectional round trips, null/empty Optional behavior, and diagnostic rejection of values that cannot be represented losslessly.

@github-actions github-actions Bot added A-java Java bindings + JNI enhancement New feature or request labels Sep 1, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gate recommendation: approve.

The typed Java/JNI mappings now cover every current transaction operation in both directions, while boundary validation prevents silent truncation or field loss. Keeping the typed mapping is reasonable here because the existing protobuf conversion is not itself a lossless opaque transport.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 1, 2026

@majin1102 majin1102 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@majin1102
majin1102 merged commit e728b53 into lance-format:main Sep 4, 2026
13 checks passed
Xuanwo pushed a commit that referenced this pull request Sep 4, 2026
## Summary

The Java JNI build on `main` is broken since #8925 ([failing
run](https://github.com/lance-format/lance/actions/runs/33884542905/job/101061005145)):

```
error[E0119]: conflicting implementations of trait `IntoJava` for type `&lance_table::format::BasePath`
   --> src/transaction.rs:125:1
    |
125 | impl IntoJava for &BasePath {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `&lance_table::format::BasePath`
    |
::: src/blocking_dataset.rs:89:1
 89 | impl IntoJava for &BasePath {
    | --------------------------- first implementation here
```

#8925 added an `impl IntoJava for &BasePath` in `transaction.rs` that
duplicates the pre-existing, behaviorally identical impl in
`blocking_dataset.rs`. This removes the duplicate; the
`Overwrite.initial_bases` export in `transaction.rs` resolves to the
impl in `blocking_dataset.rs`, and the now-unused `BasePath` import is
dropped.

## Test Plan

- `cargo check --manifest-path java/lance-jni/Cargo.toml` — passes
(previously E0119)
- `cargo clippy --tests --manifest-path java/lance-jni/Cargo.toml` —
clean
- `cargo test --manifest-path java/lance-jni/Cargo.toml --lib` — 22
passed (incl.
`transaction::tests::test_checked_transaction_integer_conversions`,
which covers the `basePath.id` overflow conversion)
- Reintroducing a duplicate impl is caught by the compiler itself
(E0119), so no new test is needed for the removal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-java Java bindings + JNI enhancement New feature or request K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants