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

Sergei Kornilov <sk@zsrv.org>

From: Sergei Kornilov <sk@zsrv.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Ildar Musin <i.musin@postgrespro.ru>, Stephen Frost <sfrost@snowman.net>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2018-03-06T19:27:40Z
Lists: pgsql-hackers
Hello

> Do you actually need test output proving that this code path was taken
> rather than the default one? Seems like looking at the code coverage
> report might be enough.
I not known. In v4 i use DEBUG1 message and do not check code path in tests at all: by full table scan or by constraint, i tested only command result to not break behavior.
Today Ildar Musin proposed to test code path through NotNullImpliedByRelConstraints function. This is my first patch and I do not have the confidence to write a test. So I looked more closely at the alter table tests, found several info in attach partition and updated my patch.

> I did not see any INFO messages in a quick test of ALTER TABLE ATTACH
> PARTITION, but if there are any lurking in there, they probably need
> to be downgraded.
In src/test/regress/expected/alter_table.out i found 7 test with
> INFO: partition constraint for table "..." is implied by existing constraints
and 5 with
> INFO:  updated partition constraint for default partition "..." is implied by existing constraints
ereport's are in ValidatePartitionConstraints function src/backend/commands/tablecmds.c

regards, Sergei


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.