Skip to content

feat(sqlserver): composite FK aggregation via STRING_AGG / FOR XML PATH (#146)#214

Merged
debba merged 3 commits into
TabularisDB:feat/sql-serverfrom
saurabh500:feat/146-fk-composite-aggregation
May 19, 2026
Merged

feat(sqlserver): composite FK aggregation via STRING_AGG / FOR XML PATH (#146)#214
debba merged 3 commits into
TabularisDB:feat/sql-serverfrom
saurabh500:feat/146-fk-composite-aggregation

Conversation

@saurabh500
Copy link
Copy Markdown
Contributor

@saurabh500 saurabh500 commented May 18, 2026

Closes #146.

What

Aggregate composite foreign-key columns into a single result row per constraint, instead of one row per column. Adds STRING_AGG (WITHIN GROUP ...) for SQL Server 2017+ and a STUFF(... FOR XML PATH(''), ...) fallback for 2012-2016, dispatched at runtime via a new detect_server_version probe against SERVERPROPERTY('ProductVersion') (with @@VERSION and a DEFAULT_MAJOR = 14 fallback).

Changes

  • models.rsForeignKey gains 3 #[serde(default)] fields (columns: Vec<String>, ref_columns: Vec<String>, ref_schema: Option<String>), plus local_columns() / referenced_columns() helpers that fall back to the legacy single-column fields. Old persisted JSON deserializes unchanged.
  • drivers/sqlserver/introspection.rs
    • Replaces Q_GET_FOREIGN_KEYS / Q_GET_ALL_FOREIGN_KEYS_BATCH with _STRING_AGG / _XML_PATH variants. Both branches emit the same row shape: name, ref_schema, ref_table, columns, ref_columns, on_update, on_delete (+ table_name for the batch variants). Cascade actions are normalised in SQL via CASE over sys.foreign_keys.update_referential_action so the output matches the INFORMATION_SCHEMA form the rest of the codebase already expects (NO ACTION, not NO_ACTION).
    • build_foreign_key signature now takes column arrays; legacy column_name / ref_column are derived from the first array element so downstream consumers (FK list view, ER diagram) keep working before they migrate.
    • get_foreign_keys / get_all_foreign_keys_batch detect the server version once per call, pick the right query, and parse the comma-aggregated columns via a new split_agg_columns helper.
    • New detect_server_version async helper reuses the existing parse_major_version / parse_version_banner / DEFAULT_MAJOR from version.rs.
  • drivers/{sqlite,mysql,postgres}/mod.rs — 6 FK construction sites get ..Default::default() so new fields stay at their default.

Tests

  • Updated unit tests for query shape on both _STRING_AGG and _XML_PATH variants and both single-table and batch flavours (asserts STRING_AGG WITHIN GROUP, STUFF(... FOR XML PATH('')), deterministic ORDER BY fkc.constraint_column_id, normalised action strings).
  • New build_foreign_key tests covering: composite columns, empty arrays, missing actions.
  • split_agg_columns parser tests.
  • ForeignKey::local_columns() / referenced_columns() fallback test for drivers that haven't migrated.
  • Regression test deserialising the legacy JSON shape (no columns / ref_columns / ref_schema) — verifies serde defaults still kick in.
  • cargo test --lib drivers::sqlserver103 passed, 0 failed.
  • cargo test --lib — 608 passed, 2 pre-existing flaky updater::tests failures (Windows flatpak/snap detection) unrelated to this PR.

…TH (TabularisDB#146)

Aggregate composite foreign-key columns into a single row per constraint.
Dispatch STRING_AGG on SQL Server 2017+ and STUFF/FOR XML PATH on 2012-2016
via a runtime SERVERPROPERTY('ProductVersion') probe with @@Version fallback.

ForeignKey gains columns / ref_columns / ref_schema (serde-default for
backward-compatible IPC) plus local_columns / referenced_columns helpers
that fall back to the legacy single-column fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@saurabh500 saurabh500 marked this pull request as ready for review May 18, 2026 18:22
@saurabh500
Copy link
Copy Markdown
Contributor Author

@debba this is ready for review

@debba
Copy link
Copy Markdown
Collaborator

debba commented May 19, 2026

Hi @saurabh500 ,
I can see the following error, trying build:


  VITE v7.3.1  ready in 76 ms

  ➜  Local:   http://localhost:5173/
  ➜  Network: use --host to expose
     Running DevCommand (`cargo  run --no-default-features --color always --`)
error: failed to parse lock file at: /Users/debba/Progetti/tabularis/src-tauri/Cargo.lock

Caused by:
  TOML parse error at line 1378, column 6
       |
  1378 | name = "der_derive"
       |      ^
  unexpected `=` in array, expected value, `]`
        Info Watching /Users/debba/Progetti/tabularis/src-tauri for changes...
 ELIFECYCLE  Command failed with exit code 101.

No problem if I remove manually Cargo.lock and re-try, can you check?

@saurabh500
Copy link
Copy Markdown
Contributor Author

@debba Yes please generate a fresh cargo lock file.

@debba
Copy link
Copy Markdown
Collaborator

debba commented May 19, 2026

@saurabh500 Ok Cargo.lock is tracked, I will push it on this PR

@debba debba merged commit 7d34ae8 into TabularisDB:feat/sql-server May 19, 2026
1 check passed
saurabh500 added a commit to saurabh500/tabularis that referenced this pull request May 20, 2026
Merge base branch feat/sql-server (which now contains composite PK/FK
work from PRs TabularisDB#213/TabularisDB#214) into feat/147-identity-insert. Conflicts in
src-tauri/src/drivers/sqlserver/helpers.rs were purely additive on both
sides — kept both the IDENTITY_INSERT helpers/tests (value_to_sql_param,
build_insert_sql identity tests) and the composite PK SQL builders
(build_pk_where_clause, build_delete_composite_sql,
build_update_composite_sql) with their tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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