Skip to content

test(engine): cover camelCase @unique on append and merge loads - #875

Merged
azimafroozeh merged 1 commit into
ModernRelay:mainfrom
pronskiy:fix/unique-camelcase-column
Oct 4, 2026
Merged

azimafroozeh merged 1 commit into
ModernRelay:mainfrom
pronskiy:fix/unique-camelcase-column

Conversation

@pronskiy

@pronskiy pronskiy commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

What & why

#765 reported that writes into a type with a camelCase @unique property failed with No field named versiontag, for load --mode append, load --mode merge and mutate … insert. #768 fixed the committed-uniqueness probe (col → ident in CommittedState::unique_holders), but its regression case, issue_765_camelcase_unique_property_insert.gqt, only covers insert. The two load modes reach the same probe through CommittedState::load, and no test covers them. A GQT case can't: GQT seeds only with Overwrite, which skips the committed probe.

This adds load_issue_765_camelcase_unique_column_loads_and_rejects_duplicates to crates/omnigraph/tests/validators.rs. For both Append and Merge, it loads Message { slug @key, messageId @unique } into an empty graph. It then checks that a second load with the same messageId is refused with @unique violation on Message.messageId and leaves one row.

Backing issue / RFC

Checklist

  • Change is focused (one logical change)
  • Tests added/updated for behavior changes
  • Public docs updated if user-facing surface changed: N/A, test-only; no changelog note
  • Reviewed against docs/dev/invariants.md: N/A, no production code changes

Local verification

  • cargo test --locked -p omnigraph-engine --test validators issue_765: 1 passed
  • cargo test --locked -p omnigraph-engine --test validators: 35 passed
  • cargo fmt --all --check: clean
  • cargo clippy -p omnigraph-engine --all-targets --locked -- -D warnings -W clippy::dbg_macro: clean
  • Reverting fix(engine): use literal identifiers for property names #768's ident back to col in CommittedState::unique_holders makes the test fail with the original No field named messageid … lance-datafusion-11.0.0/src/planner.rs:1025:20. With the mode order swapped, the Merge load fails the same way, so each mode catches the bug on its own.

Notes for reviewers

  • The test loops over the two modes instead of adding one test per mode. Both modes build the same CommittedState::load view, and the loop keeps a single schema and body.
  • A camelCase @key needs no test here: keyed loads upsert on the __id system column (MergeInsertBuilder::try_new(ds, vec![system_columns.id])), so the key column's name never reaches a filter.

@azimafroozeh
azimafroozeh added this pull request to the merge queue Oct 4, 2026
@azimafroozeh

Copy link
Copy Markdown
Collaborator

Thanks!

Merged via the queue into ModernRelay:main with commit cf59565 Oct 4, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants