fix(dataset): reject all system column names on write

[LuciferYang]

What & why

The write-path reserved-column-name guard only rejected three of the five system column names (_rowid, _rowaddr, _rowoffset), missing _row_created_at_version and _row_last_updated_at_version. Those two were introduced by the row-tracking feature and the guard was never extended.

System columns are virtual — they are injected into scan results at read time and never stored in the physical data. Projection::to_schema appends _rowid, _rowaddr, and the two row-version columns to a projection, and the scanner's filterable_schema sets all of those flags on every filtered scan. So a user data column literally named _row_created_at_version passed ingest, then collided with the appended system field the next time the dataset was scanned with a filter, hitting to_schema's extend(...).unwrap() and panicking. Rare (the column name has to match exactly) but reachable on well-formed, in-memory data — not a corrupt-file case.

How & why it is correct

Replace the hardcoded three-name check with is_system_column, which covers all five names. This rejects the collision at the write boundary with a clear error instead of letting it surface as a later panic, so to_schema's existing contract genuinely holds. is_system_column is a strict superset of the old three names, so the previously-rejected names still error exactly as before.

Compatibility

No behavior change for legitimate writes. The row-version columns are virtual and never part of a stored schema, so no valid write payload contains them; the only schema that reaches this guard is the user-provided data schema in InsertBuilder. Internal writers (merge-insert, update) build their schemas from dataset.schema() and bypass this guard entirely. The only newly-rejected input is a user explicitly naming a physical column after a system column — which is precisely the case that used to panic on read.

Test plan

• New parameterized test rejects_reserved_system_column_names covering all five system column names; the two version-column cases fail against the old three-name guard and pass with the fix.
• cargo test -p lance --lib, cargo clippy -p lance --tests -- -D warnings, cargo fmt --all -- --check are green.

Summary by CodeRabbit

• Bug Fixes
  • Dataset ingestion now rejects user-provided field names that conflict with reserved system columns.
  • Validation covers the complete set of system column names, including row-version fields.
  • Added coverage to ensure conflicting field names consistently fail ingestion.

[coderabbitai]

📝 Walkthrough

Walkthrough

The insert writer now rejects user-provided fields matching the complete Lance system-column set, including row-version columns. Parameterized tests cover the reserved names.

Changes

System column validation

Layer / File(s)	Summary
Insert schema validation and coverage rust/lance/src/dataset/write/insert.rs	Write-time validation uses is_system_column, and parameterized ingestion tests cover reserved row, offset, address, and version column names.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: xuanwo

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: rejecting all system column names during dataset writes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rust/lance/src/dataset/write/insert.rs`:
- Around line 537-541: Update the error assertion in the reserved system-column
test to match the returned error as Error::InvalidInput and verify its message
contains the specific reserved_name value. Replace the generic “reserved name”
substring check while preserving the existing unexpected-error context.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4de32310-6cb4-4aa6-8d35-ae1050b9f5f3

📥 Commits

Reviewing files that changed from the base of the PR and between cf3d850 and 964b6df.

📒 Files selected for processing (1)

• rust/lance/src/dataset/write/insert.rs

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the error variant and the specific column name.

This test currently validates only a generic message fragment. Match Error::InvalidInput and assert that the message contains reserved_name, so unrelated failures cannot pass the test.

As per coding guidelines: “Assert on both the error variant and the message content in tests.”

Proposed fix

-        let err = result.expect_err("writing a reserved system column name should fail");
-        assert!(
-            err.to_string().contains("reserved name"),
-            "unexpected error for {reserved_name}: {err}"
-        );
+        let err = result.expect_err("writing a reserved system column name should fail");
+        match err {
+            Error::InvalidInput { source, .. } => {
+                let message = source.to_string();
+                assert!(message.contains("reserved name"));
+                assert!(message.contains(reserved_name));
+            }
+            other => panic!("expected InvalidInput, got: {other:?}"),
+        }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	let err = result.expect_err("writing a reserved system column name should fail");
	assert!(
	err.to_string().contains("reserved name"),
	"unexpected error for {reserved_name}: {err}"
	);
	let err = result.expect_err("writing a reserved system column name should fail");
	match err {
	Error::InvalidInput { source, .. } => {
	let message = source.to_string();
	assert!(message.contains("reserved name"));
	assert!(message.contains(reserved_name));
	}
	other => panic!("expected InvalidInput, got: {other:?}"),
	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance/src/dataset/write/insert.rs` around lines 537 - 541, Update the
error assertion in the reserved system-column test to match the returned error
as Error::InvalidInput and verify its message contains the specific
reserved_name value. Replace the generic “reserved name” substring check while
preserving the existing unexpected-error context.

Source: Coding guidelines

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!