Skip to content

fix(sidebar): stop a period in a quoted name merging two objects' identities - #3068

Merged
datlechin merged 2 commits into
mainfrom
fix/tableinfo-id-dotted-names
Sep 23, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/tableinfo-id-dotted-names

Conversation

@datlechin

Copy link
Copy Markdown
Member

Defect

TableInfo.id joined a table's schema and name with a bare period, so two different tables could get one id:

Table Old id
schema a, table b.c a.b.c_TABLE
schema a.b, table c a.b.c_TABLE
no schema, table a.b a.b_TABLE
schema a, table b a.b_TABLE

A period is legal inside a quoted identifier on PostgreSQL, SQL Server, Oracle, SQLite and the rest, so both pairs are real tables a server will hold at once. RoutineInfo.id, UserDefinedTypeInfo.id and TriggerInfo.id had the same shape, and RoutineInfo and UserDefinedTypeInfo define == and hash through their id, so two such routines or types were not just keyed alike, they compared equal.

Every consumer that keys, deduplicates or diffs by these ids dropped one object of each pair:

  • SchemaService.allLoadedTables deduplicates by id, and it feeds Open Quickly, SchemaRefreshService and CatalogEditAdoption: with schemas a and a.b both loaded, one of the two tables vanished from Open Quickly.
  • DatabaseTreeFilter deduplicates tables, routines, triggers and types by id, so the sidebar filter showed one row where there were two.
  • SQLSchemaProvider.mergeTables deduplicates autocomplete's table list by id, so completing after a. could lose b.c once "a.b".c was cached.
  • DatabaseTreeMetadataService.partitionCountsChanged keys counts by id, so it compared one table's partition count against the other's and reported a change that never happened.
  • RecentTableRow, DatabaseTreeTableRef and the routine, trigger and type row refs build outline item ids from these, so the Recent section and the tree could merge two rows.
  • RowImportSheet (ForEach(availableTables, id: \.id)) and TableTransferSheet's selection binding key rows by the table id.
  • Open Quickly records the ids of routine, trigger, type and cross-connection table items in its frecency store.

The tree's routine, trigger and type rows had a second copy of the defect one level up: DatabaseTreeRoutineRef, DatabaseTreeTriggerRef and DatabaseTreeUserTypeRef joined database, schema and the object id with a bare |, while DatabaseTreeTableRef and PartitionInfo beside them already escaped it, each with its own private copy of the escaping.

Root cause

No shared way to build an identity out of names a user chose. Each type joined its parts by hand, some escaped and most did not.

Fix

IdentityPath (TablePro/Models/Database/IdentityPath.swift) joins components with a separator and escapes a backslash or that separator inside a component. Every id above now goes through it, and the two private escape helpers in DatabaseTreeTableRef and PartitionInfo are gone.

A component that holds neither character is emitted unchanged, so every id for a name without a period or backslash is byte-identical to before. That matters because Open Quickly persists these ids in its frecency store. Only objects whose names hold a period or backslash get a new id, and those were the ones colliding.

The display spellings (qualifiedName) are untouched.

What stays as it was, deliberately:

  • TriggerInfo.id still omits a missing schema or table, so a trigger with a schema and no table and one with a table and no schema can still render alike. Keeping positions would change the id of every trigger on an engine without schemas, which the frecency store holds, and one listing stamps one schema on every trigger, so the two shapes do not meet in one list.
  • RoutineInfo.id still joins its discriminator with _, which a name can contain. Escaping _ would change the id of every routine with an underscore in its name. An engine reports a discriminator for all of its routines or for none, so the two shapes do not meet in one list either.

Not in this PR

Open Quickly's own row id for a table in the current connection is QuickSwitcherItem.tableItemId, which still joins schema and name with a bare period on main. #3060 escapes it the same way, so this PR leaves those lines alone rather than conflict with it. With this PR alone, both tables now reach Open Quickly; with #3060 as well, each row is selectable on its own. tableItemId can move onto IdentityPath once both are in.

The compare feature has the same shape in its own ids (TableDiffResult, DataComparePlan, SchemaSyncOperation, CompareObjectIdentity) and also keys its snapshot maps by the dotted qualified name, and it persists plan ids in sync profiles. Fixing the ids alone there would split one key into two spellings, so it needs its own change. The Backup sheet's ObjectRow.id doubles as the row's visible label, so it needs identity and display separated first.

Tests

New tests: IdentityPathTests (new suite, 5 tests, one parameterized over . and |), plus cases in TableInfoTests, RoutineInfoTests, TriggerInfoTests, UserDefinedTypeInfoTests, SchemaServiceTests, DatabaseTreeFilterTests, DatabaseTreeNodeTests and PartitionCountRefreshTests. Each collision case asserts two objects keep two ids, and plainNamesKeepTheirId plus the existing literal-id tests pin the unchanged bytes. The old formulas give one id to every pair these tests use (a.b.c_TABLE twice, a.b_TABLE twice, FUNCTION_a.b.c twice, type_a.b.c twice, a.b.c.audit twice, a|b||FUNCTION_f twice), so each collision case fails on main. That comes from the formulas rather than a run: two attempts to run these suites with the source change reverted each waited out verify.sh's 30 minutes behind other builds on this machine and ran nothing.

Found while investigating #3048 (see #3060).

@datlechin
datlechin merged commit b03f2c3 into main Sep 23, 2026
4 checks passed
@datlechin
datlechin deleted the fix/tableinfo-id-dotted-names branch September 23, 2026 10:16
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.

1 participant