Subject: Re: using index or check in ALTER TABLE SET NOT NULL
To: Sergei Kornilov <sk@zsrv.org>, Stephen Frost <sfrost@snowman.net>
References: <81911511895540@web58j.yandex.ru>
 <CA+TgmoZKR4dTR7KyoM4jfJN2J-eNoBRG2_XHdZa1f0VUbK1xXQ@mail.gmail.com>
 <20171129152135.GN4628@tamriel.snowman.net> <9878.1511970441@sss.pgh.pa.us>
 <370481511970756@web3j.yandex.ru> <491651511974381@web27j.yandex.ru>
 <CA+TgmoahVfu0dUmvM1bO3csn=DtFp1fG92995rjYD9u_kO-Y1g@mail.gmail.com>
 <369811512396584@web6o.yandex.ru> <20180123153711.GI2416@tamriel.snowman.net>
 <354171516734367@web46g.yandex.ru>
 <8fd20248-4059-4d46-b33f-c9760cb3ddb9@postgrespro.ru>
 <611431520341951@web57o.yandex.ru>
Cc: Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
From: Ildar Musin <i.musin@postgrespro.ru>
Message-ID: <7a6a8eb5-a36c-239a-1724-9fc8a2f29115@postgrespro.ru>
Date: Tue, 6 Mar 2018 17:37:06 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.3.0
MIME-Version: 1.0
In-Reply-To: <611431520341951@web57o.yandex.ru>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 06.03.2018 16:12, Sergei Kornilov wrote:
> Hello thank you for review!
>
>> Adding check constraint will also force the full table scan. So I
>> think it would be better to rephrased it as follows:
> Agree. I updated docs in new attached patch slightly different
>
>> Regarding regression tests it may be useful to set
>> client_min_messages to 'debug1' before setting "NOT NULL" attribute
>> for a column. In this case you can tell for sure that
>> NotNullImpliedByRelConstraints() returned true (i.e. your code
>> actually did the job) as the special debug message is printed to
>> the log.
> I can not find any debug messages in tests: grep client_min_messages
> -irn src/test/ Only some warning level and few error. So i verify in
> regression tests only top-level behavior. Or this paragraph was for
> other people, not for tests?
>

Sorry, probably I didn't explain it clearly enough. I meant that your
regression tests can't distinguish cases when the full table scan was
actually performed from the ones when it was skipped due to
NotNullImpliedByRelConstraints() check. For instance, consider this
piece from the test suite:


# create table atacc1 (test_a int, test_b int);
CREATE TABLE
...

# alter table atacc1 add constraint atacc1_constr_a_valid check(test_a
is not null);
ALTER TABLE

# alter table atacc1 alter test_a set not null;
ALTER TABLE


It is not obvious that full table scan was omitted. But if we set
client_min_messages to 'debug1', we'll be able to see what is really
happened by the different debug messages. For example:


# create table atacc1 (test_a int, test_b int);
CREATE TABLE
...

# set client_min_messages to 'debug1';
SET

# alter table atacc1 alter test_a set not null;
DEBUG:  verifying table "atacc1"   <<<< full table scan!
ALTER TABLE

# alter table atacc1 alter test_a drop not null;
ALTER TABLE

# alter table atacc1 add constraint atacc1_constr_a_valid check(test_a
is not null);
DEBUG:  verifying table "atacc1"
ALTER TABLE

# alter table atacc1 alter test_a set not null;
DEBUG:  verifying table "atacc1" NOT NULL constraint on test_a attribute
by existed constraints   <<<< full scan was skipped!
ALTER TABLE


-- 
Ildar Musin
i.musin@postgrespro.ru