From: Ildar Musin <i.musin@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Sergei Kornilov <sk@zsrv.org>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
Date: Wed, 7 Mar 2018 14:03:53 +0300
Message-ID: <082cf493-a989-dcd6-7baf-a94a40d07ea0@postgrespro.ru> (raw)
In-Reply-To: <1142.1520362313@sss.pgh.pa.us>
References: <880511520351578@web5j.yandex.ru>
<20180306172528.xnelm3xcq2qeyd2g@alvherre.pgsql>
<1715451520361128@web26o.yandex.ru>
<1142.1520362313@sss.pgh.pa.us>
Hello Sergei, Alvaro, Tom,
On 06.03.2018 20:25, Alvaro Herrera wrote:
> You should be able to use an event trigger that raises a message when
> table_rewrite is hit, to notify the test driver that a rewrite happens.
>
> (If any DDL that causes a table rewrite fails to trigger the
> table_rewrite event correctly, that is a bug.)
>
It seems that 'table_rewrite' event trigger isn't fired in case of
simple scan (without actual rewrite).
> ISTM that depending on DEBUG messages is bad because debugging lines
> added elsewhere will make your tests fail;
I think that between test depending on DEBUG message and no test at all
the former one is preferable. Are there other options to test it?
On 06.03.2018 21:51, Tom Lane wrote:
>
> 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.
>
Code coverage cannot guarantee correctness. I think there should be at
least two tests:
* full scan is performed when there are no check constraints that
restrict NULL values;
* full scan is omitted when there are such constraints.
The last one could also include some unobvious cases like CHECK (col >
0), complex expressions with boolean operators etc.
PS: Btw, some check constraints that implies NOT NULL still won't
prevent from full table scan. For instance:
# create table test (a int, b int check ((a + b) is not null));
CREATE TABLE
# set client_min_messages to 'debug1';
SET
# insert into test values (1, 1);
INSERT 0 1
# alter table test alter column a set not null;
DEBUG: verifying table "test" <<<< full table scan!
ALTER TABLE
Since operator+ (aka int4pl) is strict it returns NULL if at least one
of its operands is NULL. And this check constraint ensures therefore
that both columns 'a' and 'b' are NOT NULL. It isn't probably the issue
of this patch, just something that someone may find interesting.
--
Ildar Musin
i.musin@postgrespro.ru
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: i.musin@postgrespro.ru, tgl@sss.pgh.pa.us, sk@zsrv.org, alvherre@alvh.no-ip.org, sfrost@snowman.net, robertmhaas@gmail.com
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
In-Reply-To: <082cf493-a989-dcd6-7baf-a94a40d07ea0@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox