Skip to content

Commit

Permalink
[6.x] Fix pg_constraint oid duplication (#8166) (#8167)
Browse files Browse the repository at this point in the history
Multiple of our invested constraints were being given the same oid
which caused problems.

One specific problem was generating bogus pgdumps, which annoyingly
mostly only showed up in inplace-upgrade tests:


https://github.com/edgedb/edgedb/actions/runs/12442350513/job/34740389082?pr=8159

The problem was that pg_get_constraintdef was returning a constraint
definition for the "wrong" object; the defining query would return 3
rows, and postgres silently returns the first.

Fix this by adding some bits to the oid separate from the uuid of the
link, and test that the fix works by putting the body of
pg_get_constraintdef into a subquery.

This hopefully will unblock #8159.
  • Loading branch information
msullivan authored Jan 3, 2025
1 parent 567de6a commit ac259fa
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
18 changes: 14 additions & 4 deletions edb/pgsql/metaschema.py
Original file line number Diff line number Diff line change
Expand Up @@ -6389,12 +6389,14 @@ def make_wrapper_view(name: str) -> trampoline.VersionedView:
name=('edgedbsql', 'uuid_to_oid'),
args=(
('id', 'uuid'),
# extra is two extra bits to throw into the oid, for now
('extra', 'int4', '0'),
),
returns=('oid',),
volatility='immutable',
text="""
SELECT (
('x' || substring(id::text, 2, 7))::bit(28)::bigint
('x' || substring(id::text, 2, 7))::bit(28)::bigint*4 + extra
+ 40000)::oid;
"""
)
Expand Down Expand Up @@ -7194,7 +7196,9 @@ def make_wrapper_view(name: str) -> trampoline.VersionedView:
-- foreign keys for object tables
SELECT
edgedbsql_VER.uuid_to_oid(sl.id) as oid,
-- uuid_to_oid needs "extra" arg to disambiguate from the link table
-- keys below
edgedbsql_VER.uuid_to_oid(sl.id, 0) as oid,
vt.table_name || '_fk_' || sl.name AS conname,
edgedbsql_VER.uuid_to_oid(vt.module_id) AS connamespace,
'f'::"char" AS contype,
Expand Down Expand Up @@ -7240,7 +7244,9 @@ def make_wrapper_view(name: str) -> trampoline.VersionedView:
-- - single link with link properties (source & target),
-- these constraints do not actually exist, so we emulate it entierly
SELECT
edgedbsql_VER.uuid_to_oid(sp.id) AS oid,
-- uuid_to_oid needs "extra" arg to disambiguate from other
-- constraints using this pointer
edgedbsql_VER.uuid_to_oid(sp.id, spec.attnum) AS oid,
vt.table_name || '_fk_' || spec.name AS conname,
edgedbsql_VER.uuid_to_oid(vt.module_id) AS connamespace,
'f'::"char" AS contype,
Expand Down Expand Up @@ -7856,6 +7862,10 @@ def construct_pg_view(
returns=('text',),
volatility='stable',
text=r"""
-- Wrap in a subquery SELECT so that we get a clear failure
-- if something is broken and this returns multiple rows.
-- (By default it would silently return the first.)
SELECT (
SELECT CASE
WHEN contype = 'p' THEN
'PRIMARY KEY(' || (
Expand All @@ -7868,7 +7878,6 @@ def construct_pg_view(
SELECT attname
FROM edgedbsql_VER.pg_attribute
WHERE attrelid = conrelid AND attnum = ANY(conkey)
LIMIT 1
) || '")' || ' REFERENCES "'
|| pn.nspname || '"."' || pc.relname || '"(id)'
ELSE ''
Expand All @@ -7878,6 +7887,7 @@ def construct_pg_view(
LEFT JOIN edgedbsql_VER.pg_namespace pn
ON pc.relnamespace = pn.oid
WHERE con.oid = conid
)
"""
),
trampoline.VersionedFunction(
Expand Down
5 changes: 5 additions & 0 deletions edb/pgsql/patches.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,10 @@ def _setup_patches(patches: list[tuple[str, str]]) -> list[tuple[str, str]]:
"""
PATCHES: list[tuple[str, str]] = _setup_patches([
# 6.0b2?
# One of the sql-introspection's adds a param with a default to
# uuid_to_oid, so we need to drop the original to avoid ambiguity.
('sql', '''
drop function if exists edgedbsql_v6_2f20b3fed0.uuid_to_oid(uuid) cascade
'''),
('sql-introspection', ''),
])

0 comments on commit ac259fa

Please sign in to comment.