Many thanks for excellent work! I've tested the patch successfully.
I ran this query (on a patched database) to see if there are still any catalog tables without primary keys: SELECT table_name FROM information_schema.tables WHERE table_schema = 'pg_catalog' AND table_type = 'BASE TABLE' EXCEPT SELECT table_constraints.table_name FROM information_schema.table_constraints JOIN information_schema.key_column_usage ON key_column_usage.constraint_name = table_constraints.constraint_name WHERE table_constraints.table_schema = 'pg_catalog' AND table_constraints.constraint_type = 'PRIMARY KEY' table_name ------------- pg_depend pg_shdepend (2 rows) Wouldn't it be possible to add primary keys to these two as well? It would need to be multi-column primary keys, but should be ok since we have that for other catalogs? /Joel On Thu, Jan 21, 2021, at 18:15, Tom Lane wrote: > Peter Eisentraut <peter.eisentr...@2ndquadrant.com> writes: > > On 2021-01-17 23:07, Tom Lane wrote: > >> I've reviewed this patch. It looks pretty solid to me, with a couple > >> trivial nits as mentioned below, and one bigger thing that's perhaps > >> in the category of bikeshedding. Namely, do we really want to prefer > >> using the OID indexes as the primary keys? > > > I chose this because the notional foreign keys point to the OID. > > If you design some basic business database with customer IDs, product > > IDs, etc., you'd also usually make the ID the primary key, even if you > > have, say, a unique constraint on the product name. But this is of > > course a matter of taste to some degree. > > Fair enough. As I said upthread, I just wanted to be sure we'd considered > the alternative. I'm content to use the OIDs as pkeys, although I think > that decision should be explicitly recorded somewhere (cf attachment). > > >> The contents of system_constraints.sql seem pretty randomly ordered, > >> and I bet the order isn't stable across machines. > > > They follow the order in which the catalogs are processed byt genbki.pl. > > Looking closer, I see the data structure is an array not a hash, so > I withdraw the concern about instability. > > After reading the patch again, I have a couple more nits about comments, > which I'll just present as a proposed delta patch. Otherwise it's good. > I'll mark it RFC. > > regards, tom lane
