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
