Horton

Re: using index or check in ALTER TABLE SET NOT NULL

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Sergei Kornilov <sk@zsrv.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Stephen Frost <sfrost@snowman.net>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-11-30T20:55:00Z
Lists: pgsql-hackers 
On Wed, Nov 29, 2017 at 11:53 AM, Sergei Kornilov <sk@zsrv.org> wrote:
> Here is new patch with check only existed valid constraints and without SPI at all.

Please use pgindent or anyway try to follow project style.  { needs to
go on a line by itself, for example.

isSetNotNullNeedsTableScan seems different than other similarly-named
functions we have.  NotNullImpliedByRelConstraints?

It also lacks a header comment.

Having both PartConstraintImpliedByRelConstraint and
ConstraintImpliedByRelConstraint with identical implementations
doesn't seem like a good idea to me.  I guess just renaming it is
probably fine.

The comment /* T if we added new NOT NULL constraints */ should
probably be changed to /* T if we should recheck NOT NULL constraints
*/ or similar.

I'd suggest registering this with the next CommitFest.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

  1. Avoid using INFO elevel for what are fundamentally debug messages.

  2. Revert setting client_min_messages to 'debug1' in new tests.

  3. Allow ALTER TABLE .. SET NOT NULL to skip provably unnecessary scans.

  4. Improve predtest.c's internal docs, and enhance its functionality a bit.