[LuQQiu]

Why

A compaction planner already chooses the final input fragments in each TaskData. It also knows what properties the replacement fragments should carry.

Previously, a task had no way to request those output properties. For example, an application compacting fragments from the same logical group could not explicitly label the replacements with that group's clustering ID and metadata.

This PR adds that output interface. It does not add grouping or decide which fragments belong together.

What the planner controls

Each task independently specifies:

output_clustering_id: Option<u32>
output_metadata: HashMap<String, String>

Both fields apply to every output fragment of that task. ID only, metadata only, both, or neither are supported. Metadata needs no clustering definition.

For example:

Task inputs:  [F1, F2]
Task outputs: clustering_id = Some(12), metadata = {"partition": "east"}

Result: every replacement fragment carries those exact properties.

Lance treats the metadata as opaque. It does not understand application buckets or enterprise clustering specs. The caller is responsible for choosing compatible inputs and meaningful output properties.

The requested values replace the output fields; they are not inferred or copied from the inputs. Defaults remain no clustering ID and empty metadata, preserving ordinary compaction behavior.

ID checks and distributed execution

A stale ID that was already allocated but is absent from the read snapshot is refused before rewriting. An ID above that snapshot's allocation watermark may have been created later, so it is left to commit-time validation.

The landing manifest must contain any requested definition when the replacement fragments commit. A commit-time refusal does not publish those replacements, although files or fragment-ID reservations may already have been staged.

Serialized tasks default missing fields to absent/empty. Workers must understand the new fields: older workers can ignore them, and this PR adds no capability negotiation. Java JNI refuses to export tasks carrying either property because its task representation cannot preserve them.

Reading order and related PR

Start with TaskData, refuse_stale_output_clustering, the output stamping in rewrite_files, then compaction_outputs_carry_the_tasks_output_properties.

#10445 builds on this interface: it adds caller-supplied candidate groups and reuses ordinary compaction eligibility inside each group. This PR only controls the properties of already-selected tasks' outputs.

[LuQQiu]

@claude review

[claude]

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🔴 java/lance-jni/src/optimize.rs — After this merge the Java JNI crate (lance-jni) no longer compiles, so the Java SDK cannot be built. TaskData gained a required field, output_clustering. The TaskData { fragments: task_data } literal at java/lance-jni/src/optimize.rs:537 was not updated. Fix: update every TaskData construction to set output_clustering. Ideally also carry it through the Java TaskData and the IntoJava impl at optimize.rs:363. Otherwise a plan that goes through Java loses the planner's label. [also at: rust/lance/src/dataset/optimize.rs:2379 - The Java bindings stop compiling after this merge. The JNI crate builds TaskData with a struct literal at java/lance-jni/src/optimize.rs:537, and that literal lists only fragments.]

  Why this was flagged

  TaskData in rust/lance/src/dataset/optimize.rs:2369 now has a second public field, output_clustering: Option<OutputClustering>. The struct has no Default impl and no .. constructor. The FromJObjectWithEnv<TaskData> impl at java/lance-jni/src/optimize.rs:532-541 still builds TaskData { fragments: task_data }, so rustc fails with E0063 (missing field output_clustering). java/lance-jni/Cargo.toml:26 depends on the in-tree rust/lance by path, so any Java build or CI job fails on this branch; the base branch builds fine. The diff updated every other construction site (the Rust tests, index.rs, fts.rs, utils/test.rs) but not this one. IntoJava for &TaskData (optimize.rs:363-371) also drops the field. Even once the build compiles, a task exported to Java and imported back comes out with output_clustering: None, so a distributed Java compaction removes the label.

  Verification: The diff adds pub output_clustering: Option<OutputClustering> to TaskData at rust/lance/src/dataset/optimize.rs:2379. It has no Default impl. java/lance-jni/src/optimize.rs:537-539 still build Ok(TaskData { fragments: task_data, }), so rustc rejects it with E0063. lance-jni is excluded from the top-level workspace, so a workspace cargo build will not catch this.

[LuQQiu]

@claude review

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

[LuQQiu]

Triage of the linux-build failure on this head (dataset::tests::dataset_scanner::test_vector_filter_fts_search, ids [299, 300] where [300, 299] is expected):

• The two rows carry the same full-text score (text 299 / text 300, one occurrence of the query term each, equal length), so the assertion depends on how an equal-score tie is ordered after the vector prefilter.
• This PR's diff touches optimize.rs (compaction task output label), the JNI task import/export and docs; it does not touch the scanner, the full-text scorer or the vector prefilter.
• Ran the test 3 times on this head and 3 times on the base 134a58d4c with the two commits reverted: 6 of 6 passed.

Treating it as non-reproducing here rather than root-caused; re-running the failed job for a fresh signal. Nothing in the test is being changed.

One wording correction for the description (not editing it myself): "nothing is rewritten" should read "no rewritten fragment becomes visible". When the label is refused at the landing snapshot, the task's data files and the fragment id reservation have already been written; only the publishing commit is refused.

[LuQQiu]

@claude review

[claude]

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

[LuQQiu]

@claude review

[LuQQiu]

Restacked directly onto `main` at bd55af3 as a standalone change; it no longer depends on #10399. The two commits apply on main with no conflicts and the delta is unchanged: 5 files, +256/-1.

Scope, for review:

• `TaskData.output_clustering: Option<OutputClustering { clustering_id, metadata }>` — the planner's instruction for the label every output fragment of a task carries. Unset means the outputs carry no label, whatever the inputs carried; Lance does not infer a label from inputs because a compaction may span fragments of different clusterings.
• `rewrite_files` stamps `Fragment::clustering_id` and `Fragment::metadata` on the new fragments when the label is set; the existing commit path refuses a label whose definition the manifest does not hold, so a bad label rewrites nothing.
• Serialization is `#[serde(default, skip_serializing_if = "Option::is_none")]`: tasks stay readable both ways, but a worker built before this field ignores the label and rewrites unlabelled. A planner handing labelled tasks to other processes must know they run this version or newer.
• The Java binding has no field for the label, so `IntoJava for &TaskData` refuses a labelled task instead of silently exporting it unlabelled; tasks imported from Java are unlabelled.
• No proto or format change; `clustering_id`/`metadata` on `Fragment` already exist from feat(table): implement provider-managed clustering metadata at runtime #10383.

Validation on this head: rustfmt clean on all five files. Compile and tests are left to CI on this head; no local gate was run.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

[LuQQiu]

@claude review

[LuQQiu]

c5a5ff8 replaces the coupled `OutputClustering` with two independent output properties on `TaskData`: `output_clustering_id: Option` and `output_metadata: HashMap<String, String>`, typed as the `Fragment` fields they set. Every output fragment receives exactly these values — nothing inferred or copied from inputs — and the default (none, empty) is unchanged behaviour. Metadata alone needs no clustering definition; an explicit id is still validated against the landing manifest at commit. The test covers all four combinations, serialization round-trip (fields omitted when unset), and the refused-id case, which states that a refused attempt may leave staged files and an advanced id-reservation version while publishing no replacement. The Java binding refuses a task carrying either property.

Evidence: rustfmt clean locally; compile and tests are running in this PR's CI on this head. No separate build gate was run before pushing.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

[LuQQiu]

@claude review

[LuQQiu]

@claude review

[claude]

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

[claude]

Findings marked 🟡 are optional suggestions and need no follow-up push.

Still open from earlier reviews (1):

• 🔴 rust/lance/src/dataset/optimize.rs:5068 — The new test compaction_outputs_carry_the_tasks_output_properties likely panics, so CI fails and the JSON round-trip an…

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

[LuQQiu]

@claude review

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[LuQQiu]

@claude review

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[LuQQiu]

@claude review

[claude]

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

[claude]

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.

[lance-gatekeeper]

⚠️ Gate recommendation: approve with a non-blocking risk.

The explicit output properties fit the existing contract that replacement fragments receive writer-supplied labels rather than inheriting their inputs' labels. The documented worker-version constraint remains: older workers can silently discard the requested clustering ID and metadata, so distributed callers must ensure every worker understands the new fields.

Please mark this PR with the breaking-change label.