feat: add caller-managed data file parts - #8923
Conversation
…-concat # Conflicts: # rust/lance-file/src/concat.rs # rust/lance/src/dataset/fragment.rs # rust/lance/src/dataset/tests/fragment_write_columns.rs
…-concat # Conflicts: # rust/lance/src/dataset/blob.rs
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
4 fixed / 2 new. The caller-managed encoded-part direction remains viable, but the final target identity cannot be recovered across coordinator processes and is not bound to the dataset storage namespace. That prevents the stated retry/recovery model and allows a same-schema dataset to commit Blob descriptors whose sidecars live elsewhere.
A viable revision should make the generated logical target round-trippable and bind it to the concrete dataset/base namespace, validating that identity before writing or assembling any part.
| /// Callers persist this value alongside their part order and reuse it for every | ||
| /// part write and for final concatenation. It contains no attempt, lease owner, | ||
| /// retry, or commit state. | ||
| #[derive(Debug, Clone)] |
There was a problem hiding this comment.
The documented caller-persistence contract cannot be implemented with this type. new is the only construction path and always generates a fresh filename, while all identity fields are private and the type exposes only Clone; after a coordinator restart or transport to another worker process, callers cannot reconstruct the same final filename and Blob namespace. That means retries and recovery for the distributed workflow this change follows cannot write or reopen parts against the original target.
Please provide a durable round trip, or a validated restore constructor for the logical identity, that preserves the filename, dataset/base binding, schema, and concrete file version without persisting credentials or runtime store handles.
There was a problem hiding this comment.
We narrowed the contract and updated the PR description accordingly. DataFileTarget is intentionally a runtime-only value reused within one live assembly operation. Lance does not serialize or restore it and does not define coordinator restart or worker-transport semantics. Caller-owned storage applies to the ordinary, complete part files; it does not imply a Lance-owned durable target or part-descriptor format. Workflows that require recovery keep that state in the caller layer.
There was a problem hiding this comment.
Understood. I will treat the runtime-only, no-recovery scope as an accepted architecture risk and will not pursue this finding further.
| let base = base_id | ||
| .map(|id| format!("base:{id}")) | ||
| .unwrap_or_else(|| "primary".to_string()); | ||
| BlobNamespace::new(format!("{base}/{file_name}")) |
There was a problem hiding this comment.
This token identifies a target name, not the dataset namespace that owns its Blob objects. The same cloned target passes validate_data_file_target for another same-schema V2.2 dataset; its parts carry the identical primary/<file> token, so the second dataset accepts and commits them while the descriptors resolve sidecars under the second root and the payloads remain under the first. The result is a successfully committed unreadable file. The per-target token fixed the prior target-to-target mismatch, but the identity also needs a dataset/resolved-base binding that every dataset entry point validates.
Reproducer
I created same-Blob-schema V2.2 datasets at two distinct TempDir roots, built the target and part on the first, and ran:
let replacement = only_fragment(&second)
.write_columns_from_parts(&target, &[part])
.await
.unwrap();
let second = commit(&second, replacement).await.unwrap();
let mut scanner = second.scan();
scanner.blob_handling(BlobHandling::AllBinary);
let error = scanner.try_into_batch().await.unwrap_err();
assert!(error.to_string().contains("Not found"));CARGO_TARGET_DIR=/home/agent/tmp/pr8923-cross-dataset cargo test -p lance --lib dataset::tests::data_file_part::gatekeeper_cross_dataset_blob_target --locked -- --nocapture
The test passed: assembly and commit succeeded, then the scan reported Not found: .../second/data/<target>/10000000000000000000000000000000.blob.
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
2 remain / 0 fixed. The revision documents the target as runtime-only and makes dataset/base association a caller precondition, but those limitations still prevent the stated distributed recovery workflow and still allow Blob descriptors to be committed against a dataset that does not own their payloads.
A viable revision should make the logical target reconstructable and bind Blob-bearing targets and parts to the resolved dataset/base storage identity before writing or assembly.
| let base = base_id | ||
| .map(|id| format!("base:{id}")) | ||
| .unwrap_or_else(|| "primary".to_string()); | ||
| BlobTargetId::new(format!("{base}/{file_name}")) |
There was a problem hiding this comment.
The token built here still identifies only primary|base:<id>/<file_name>, not the resolved dataset/storage namespace. A cloned target made through dataset A therefore passes the schema, version, and base checks when used with a distinct same-schema dataset B; B accepts and commits the part, but its descriptors resolve beneath B while the Blob payload remains beneath A, so the committed file cannot be read. Documenting the association as a caller precondition does not close the prior dataset-binding finding.
Bind the logical target to the resolved dataset/base storage identity and validate it in every dataset entry point before writing or assembling Blob data.
Reproducer
I added gatekeeper_cross_dataset_blob_target beside the existing part tests. It creates two V2.2 Blob datasets at distinct TempDir roots, creates the target and part through the first, then assembles and commits that target through the second before scanning with BlobHandling::AllBinary.
CARGO_TARGET_DIR=/home/agent/tmp/pr8923-cross-dataset cargo test -p lance --lib dataset::tests::data_file_part::gatekeeper_cross_dataset_blob_target --locked -- --nocapture
Expected: the second dataset rejects the foreign target before assembly or commit. Observed: assembly and commit succeeded, and the test passed only because the subsequent scan returned Not found for the Blob payload under the second dataset root (1 passed; 0 failed).
There was a problem hiding this comment.
We acknowledge the reproduced behavior. The contract requires the caller to use the same Dataset and resolved base for every part write and final assembly. BlobTargetId only prevents mixing parts assigned to different final targets within one live operation; it is not a dataset, base, or object-store identity. Lance intentionally does not persist or derive storage ownership for this runtime token or validate its association across Dataset instances. The PR description now states this precondition and ownership boundary explicitly.
There was a problem hiding this comment.
This remains because the acknowledged behavior crosses the durable-data integrity boundary: the API successfully returns and commits a DataFile whose Blob descriptors cannot be read from that dataset. A caller precondition does not prevent that invalid durable state, so the storage-ownership association must still be validated before writing or assembly.
Problem
Callers need to encode independent row ranges once and assemble them in caller-supplied order without decoding and re-encoding. Lance should provide encoded-part validation and final ordinary data-file construction while leaving part storage and orchestration to the caller.
Behavior
DataFileTarget is a runtime-only value for one live assembly operation. It creates the same canonical random file name used by ordinary Lance writes without creating, reserving, or registering an object. Lance does not serialize or restore this value and does not define coordinator restart or cross-process recovery semantics.
Each DataFilePart is a runtime view of an ordinary, complete Lance file. Callers choose part paths and order, retain the live target for the operation, and own part storage, cleanup, and commit fencing.
Blob v2 writers receive disjoint ID leases and write managed payloads directly beneath the sidecar directory selected by the final target. BlobTargetId only rejects mixing parts assigned to different final targets within one assembly operation. It is not a dataset, base, or object-store identity. The caller must use the same dataset and resolved base for every part write and final assembly.
Format and ownership
No new Lance file, manifest, transaction, target, or part format is introduced. The completed output is an ordinary DataFile committed through the existing transaction path; readers cannot distinguish it from a normally written file. The implementation uses the current file format and existing encoded page-relocation machinery.
Callers own target lifetime, part storage, dataset/base association, cleanup, and commit fencing. Lance owns target name generation, part encoding and intrinsic validation, runtime target-identity checks, and final data-file construction.
Follow-up to #8660 and Discussion #8615.