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

From: Ildar Musin <i(dot)musin(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Sergei Kornilov <sk(at)zsrv(dot)org>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Stephen Frost <sfrost(at)snowman(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: using index or check in ALTER TABLE SET NOT NULL
Date: 2018-03-07 11:03:53
Message-ID: 082cf493-a989-dcd6-7baf-a94a40d07ea0@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(dot)musin(at)postgrespro(dot)ru

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2018-03-07 11:09:45 Re: [HACKERS] Partition-wise aggregation/grouping
Previous Message Alvaro Herrera 2018-03-07 11:01:38 Re: Protect syscache from bloating with negative cache entries