Re: Recording foreign key relationships for the system catalogs

Very nice. Thanks to this patch, I can get rid of my own parse-catalogs.pl hack and use pg_get_catalog_foreign_keys() instead.

+1

I can with high confidence assert the correctness of pg_get_catalog_foreign_keys()'s output,
as it matches the lookup tables for the tool I'm hacking on precisely:

--
-- verify single column foreign keys
--
WITH
a AS (
SELECT
fktable::text,
fkcols[1]::text,
pktable::text,
pkcols[1]::text
FROM pg_get_catalog_foreign_keys()
WHERE cardinality(fkcols) = 1
),
b AS (
SELECT
table_name,
column_name,
ref_table_name,
ref_column_name
FROM pit.oid_joins
)
SELECT
(SELECT COUNT(*) FROM (SELECT * FROM a EXCEPT SELECT * FROM b) AS x) AS except_b,
(SELECT COUNT(*) FROM (SELECT * FROM b EXCEPT SELECT * FROM a) AS x) AS except_a,
(SELECT COUNT(*) FROM (SELECT * FROM b INTERSECT SELECT * FROM a) AS x) AS a_intersect_b
;

except_b | except_a | a_intersect_b
----------+----------+---------------
0 | 0 | 209
(1 row)

--
-- verify multi-column foreign keys
--
WITH
a AS (
SELECT
fktable::text,
fkcols,
pktable::text,
pkcols
FROM pg_get_catalog_foreign_keys()
WHERE cardinality(fkcols) > 1
),
b AS (
SELECT
table_name,
ARRAY[rel_column_name,attnum_column_name],
'pg_attribute',
'{attrelid,attnum}'::text[]
FROM pit.attnum_joins
)
SELECT
(SELECT COUNT(*) FROM (SELECT * FROM a EXCEPT SELECT * FROM b) AS x) AS except_b,
(SELECT COUNT(*) FROM (SELECT * FROM b EXCEPT SELECT * FROM a) AS x) AS except_a,
(SELECT COUNT(*) FROM (SELECT * FROM b INTERSECT SELECT * FROM a) AS x) AS a_intersect_b
;

except_b | except_a | a_intersect_b
----------+----------+---------------
0 | 0 | 8
(1 row)

/Joel

On Sun, Jan 31, 2021, at 22:39, Tom Lane wrote:
> Now that dfb75e478 is in, I looked into whether we can have some
> machine-readable representation of the catalogs' foreign key
> relationships. As per the previous discussion [1], it's not
> practical to have "real" SQL foreign key constraints, because
> the semantics we use aren't quite right (i.e., using 0 instead
> of NULL in rows with no reference). Nonetheless it would be
> nice to have the knowledge available in some form, and we agreed
> that a set-returning function returning the data would be useful.
>
> The attached patch creates that function, and rewrites the oidjoins.sql
> regression test to use it, in place of the very incomplete info that's
> reverse-engineered by findoidjoins. It's mostly straightforward.
>
> My original thought had been to add DECLARE_FOREIGN_KEY() macros
> for all references, but I soon realized that in a large majority of
> the cases, that's redundant with the BKI_LOOKUP() annotations we
> already have. So I taught genbki.pl to extract FK data from
> BKI_LOOKUP() as well as the explicit DECLARE macros. That didn't
> remove the work entirely, because it turned out that we hadn't
> bothered to apply BKI_LOOKUP() labels to most of the catalogs that
> have no hand-made data. A big chunk of the patch consists in
> adding those as needed. Also, I had to make the BKI_LOOKUP()
> mechanism a little more complete, because it failed on pg_namespace
> and pg_authid references. (It will still fail on some other
> cases such as BKI_LOOKUP(pg_foreign_server), but I think there's
> no need to fill that in until/unless we have some built-in data
> that needs it.)
>
> There are various loose ends yet to be cleaned up:
>
> * I'm unsure whether it's better for the SRF to return the
> column names as textual names, or as column numbers. Names was
> a bit easier for all the parts of the current patch so I did
> it that way, but maybe there's a case for the other way.
> Actually the whole API for the SRF is just spur-of-the-moment,
> so maybe a different API would be better.
>
> * It would now be possible to remove the PGNSP and PGUID kluges
> entirely in favor of plain BKI_LOOKUP references to pg_namespace
> and pg_authid. The catalog header usage would get a little
> more verbose: BKI_DEFAULT(PGNSP) becomes BKI_DEFAULT(pg_catalog)
> and BKI_DEFAULT(PGUID) becomes BKI_DEFAULT(POSTGRES). I'm a bit
> inclined to do it, simply to remove one bit of mechanism that has
> to be documented; but it's material for a separate patch perhaps.
>
> * src/tools/findoidjoins should be nuked entirely, AFAICS.
> Again, that could be a follow-on patch.
>
> * I've not touched the SGML docs. Certainly
> pg_get_catalog_foreign_keys() should be documented, and some
> adjustments in bki.sgml might be appropriate.
>
> regards, tom lane
>
> [1] https://www.postgresql.org/message-id/flat/dc5f44d9-5ec1-a596-0251-dadadcdede98%402ndquadrant.com
>
>
>
> *Attachments:*
> * add-catalog-foreign-key-info-1.patch

Kind regards,

Joel