Re: [PATCHES] YADP - Yet another Dependency Patch

[ copied to hackers ]

> 1. I don't like the code that installs and removes ad-hoc
dependencies
> from relations to type Oid. On its own terms it's wrong (if it were
...
> explicit representation of pinning in the pg_depends table, perhaps
it
> would work to create a row claiming that "table 0 / Oid 0 / subid 0"
> depends on a pinned object.

Yes, a pinned dependency makes much more sense.

int4, bool, varchar, and name are in the same boat.

I'll make it so dependCreate() will ignore adding any additional
dependencies on pinned types (class 0, Oid 0, SubID 0) and
dependDelete() will never allow deletion when that dependency exists.

> 2. Is it really necessary to treat pg_depends as a bootstrapped
> relation? That adds a lot of complexity, as you've evidently
already
> found, and it does not seem necessary if you're going to load the
system
> dependencies in a later step of the initdb process. You can just
make
> the dependency-adding routines be no-ops in bootstrap mode; then
create
> pg_depends as an ordinary system catalog; and finally load the
entries
> post-bootstrap.

Ack.. <sound of hand hitting head>. All that work to avoid a simple
if statement.

Ahh well.. learning at it's finest :)

> 3. Isn't there a better way to find the initial dependencies? That
> SELECT is truly ugly, and more to the point is highly likely to
break
> anytime someone rearranges the catalogs. I'd like to see it
generated
> automatically (maybe using a tool like findoidjoins); or perhaps we
> could do the discovery of the columns to look at on-the-fly.

I'm not entirely sure how to approach this, but it does appear that
findoidjoins would find all the relations.

So... I could create a pg_ function which will find all oid joins,
and call dependCreate() for each entry it finds. That way
dependCreate will ignore anything that was pinned (see above)
automagically. It would also make initdb quite slow, and would add a
pg_ function that one should normally avoid during normal production.
Then again, I suppose it could be used to recreate missing
dependencies if a user was manually fiddling with that table.

initdb would call SELECT pg_findSystemDepends(); or something.

> 4. Do not use the parser's RESTRICT/CASCADE tokens as enumerated
type
> values. They change value every time someone tweaks the grammar.
> (Yes, I know you copied from extant code; that code is on my
hitlist.)
> Define your own enum type instead of creating a lot of bogus
> dependencies on parser/parser.h.

All but one of those will go away once the functions are modified to
accept the actual RESTRICT or CASCADE bit. That was going to be step
2 of the process but I suppose I could do it now, along with a rather
large regression test. The only place that RESTRICT will be used is
dependDelete(); Nowhere else will care. It'll simply pass on what
was given to it by the calling function from utility.c or a cascading
dependDelete. Of course, gram.y will be littered with the
'opt_restrictcascade' tag.

The RESTRICT usage is more of a current placeholder. I've marked the
includes as /* FOR RESTRICT */ for that reason, make them easy to
remove later.

> 6. The tests on relation names in dependDelete, getObjectName are
(a)
> slow and (b) not schema-aware. Can you make these into OID
comparisons
> instead?

Ahh yes. Good point.