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

Reply via email to