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));

# set client_min_messages to 'debug1';

# insert into test values (1, 1);

# alter table test alter column a set not null;
DEBUG:  verifying table "test"   <<<< full table scan!

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

Reply via email to