Use (catoid, oid) object identity for filter lookups#44
Merged
Conversation
The filter table keyed rows on oid alone, so OIDs that collide across Postgres system catalogs (e.g. a pg_type oid equal to a pg_proc oid) could match the wrong row and filter out the wrong object — dropping a CREATE TYPE whose oid coincided with an excluded extension function, which then failed the dependent CREATE TABLE and aborted the clone. Mirror pg_depend's (classid, objid) identity: every filter row carries a catoid, the unique index becomes (catoid, oid), and lookups take (catalogOid, objectOid). A catnames table maps the relevant catalog OIDs, fetched at runtime so nothing is hardcoded. TOC entries with no catalog id (e.g. REFRESH MATERIALIZED VIEW) fall back to an OID-only lookup. Ports upstream dimitri/pgcopydb dimitri#972, reconciled with this repo's fork-only filter code: copydb_objectid_is_filtered_out takes catalogOid (from the archive item) instead of upstream's item struct; the fork-only extension, extension-object, and pg_depend inserts carry catoid (classid, or the pg_extension/pg_class catnames entry); and the extension-object -> s_table query-back is guarded by the pg_class catoid.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
filtertable keyed rows onoidalone, with a UNIQUE index on(oid). PostgreSQL OIDs are unique only within a system catalog, not across them, so apg_typeOID can equal apg_procOID. When that happens, a lookup for one object matches the other's filter row and the wrong object is skipped.Observed in a migration: a
CREATE TYPEwas silently dropped because its enum OID matched an excluded extension function'spg_procOID, after which the dependentCREATE TABLEfailedtype ... does not existand the clone aborted (exit 12).Fix
Mirror
pg_depend's(classid, objid)identity model:filtergains acatoidcolumn; the unique index becomes(catoid, oid).catnamestable maps the relevant catalog OIDs (pg_class,pg_constraint,pg_attrdef,pg_extension,pg_collation), fetched from Postgres at runtime — nothing hardcoded.(catalogOid, objectOid). TOC entries that carry no catalog id (e.g.REFRESH MATERIALIZED VIEW) fall back to an OID-only lookup, which is safe because OIDs are drawn from a single per-database counter.Ports upstream
dimitri/pgcopydbdimitri#972, reconciled with this repo's fork-only filter code:copydb_objectid_is_filtered_outtakes acatalogOid(threaded from the archive item) rather than upstream'sitemstruct.extension,extension-object, andpg_dependfilter inserts carrycatoid(classid, or thepg_extension/pg_classcatnamesentry).extension-object→s_tablequery-back join is guarded by thepg_classcatoid (it was an OID-only match, the same collision class this fix addresses).Tests
Upstream ships no regression test for this, and a true cross-catalog OID collision can't be reproduced deterministically (OIDs are PG-assigned). Coverage relies on the existing filter/extension suites for no-regression.
Verified locally: full
PGVERSION=18 make testsgreen;exclude-extension,filtering, andextensionsexplicitly exercise the changed paths.