On Tue, Sep 20, 2022 at 10:39 AM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote:
> That said, the patch I posted for this ~10 years ago used a separate
> contype and was simpler than what I ended up with now, but amusingly
> enough it was returned at the time with the argument that it would be
> better to treat them as normal CHECK constraints; so I want to be very
> sure that we're not just going around in circles.

I don't have an intrinsic view on whether we ought to have one contype
or two, but I think this comment of yours from a few messages ago is
right on point: ".. though I'm now wondering if there's additional
overhead from checking
the constraint twice on each row: first the attnotnull bit, then the
CHECK itself.  Hmm.  That's probably quite bad." For that exact
reason, it seems absolutely necessary to be able to somehow identify
the "redundant" check constraints, so that we don't pay the expense of
revalidating them. Another contype would be one way of identifying
such constraints, but it could probably be done in other ways, too.
Perhaps it could even be discovered dynamically, like when we build a
relcache entry. I actually have no idea what design is best.

I am a little confused as to why we want to do this, though. While
we're on the topic of what is more complicated and simpler, what
functionality do we get out of adding all of these additional catalog
entries that then have to be linked back to the corresponding
attnotnull markings? And couldn't we get that functionality in some
much simpler way? Like, if you want to track whether the NOT NULL
constraint has been validated, we could add an attnotnullvalidated
column, or probably better, change the existing attnotnull column to a
character used as an enum, or maybe an integer bit-field of some kind.
I'm not trying to blow up your patch with dynamite or anything, but
I'm a little suspicious that this may be one of those cases where
pgsql-hackers discussed turns a complicated project into an even more
complicated project to no real benefit.

One thing that I don't particularly like about this whole design is
that it feels like it creates a bunch of catalog bloat. Now all of the
attnotnull flags also generate additional pg_constraint rows. The
catalogs in the default install will be bigger than before, and the
catalogs after user tables are created will be more bigger. If we get
some nifty benefit out of all that, cool! But if we're just
anti-optimizing the catalog storage out of some feeling that the
result will be intellectually purer than some competing design, maybe
we should reconsider. It's not stupid to optimize for common special
cases, and making a column as NOT NULL is probably at least one and
maybe several orders of magnitude more common than putting some other
CHECK constraint on it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com


Reply via email to