Thread: Re: using index to speedup add not null constraints to a table

On Wed, Feb 5, 2025 at 4:24 PM jian he <jian.universality@gmail.com> wrote:
>
> rebased new patch attached.
> I also did some cosmetic changes. comments refined.
> make sure using index_scan mechanism to fast check column not-null can
> only be used via btree index.
> isolation tests are simplified.

I realized that my previous patch was quite wrong,
we should not do indexscan verify individual not-null constraints on phase2.

So a new patch is attached,
the main idea is Phase2 collects all to be added not-null constraints
to AlteredTableInfo->constraints.
then in Phase3  check, can we use index to fast check not-null
constraint or not.

To minimize concurrency issues, using an index scan to quickly validate
NOT NULL constraints requires strict conditions in Phase3:
* No table rewrite
* No table scan
* Each NOT NULL constraint must have a suitable supporting index for
fast checking
* The table must already hold an AccessExclusiveLock
* The DDL must not involve creating any new indexes

I don't have any good ideas to do the regress tests.
I use
                ereport(NOTICE,
                        errmsg("all not-null constraints on relation
\"%s\" are validated by index scan",
                                RelationGetRelationName(oldrel)));
to do the tests.

for example:
create temp table t2 (x int, y int, z int, primary key (x, y));
create unique index t2_z_uidx on t2(z);

alter table t2 alter column z set not null;
NOTICE:  all not-null constraints on relation "t2" are validated by index scan
ALTER TABLE

On Fri, Apr 18, 2025 at 4:07 PM jian he <jian.universality@gmail.com> wrote:
>
> I don't have any good ideas to do the regress tests.
> I use
>                 ereport(NOTICE,
>                         errmsg("all not-null constraints on relation
> \"%s\" are validated by index scan",
>                                 RelationGetRelationName(oldrel)));
> to do the tests.
>
for tests, just found out i can imitate
src/test/modules/test_misc/t/001_constraint_validation.pl,

So I created a file:
src/test/modules/test_misc/t/008_indexscan_validate_notnull.pl
for TAP tests.

On 2025-Apr-28, jian he wrote:

> for tests, just found out i can imitate
> src/test/modules/test_misc/t/001_constraint_validation.pl,
> 
> So I created a file:
> src/test/modules/test_misc/t/008_indexscan_validate_notnull.pl
> for TAP tests.

Seems reasonable, didn't look at it in detail.  I think you don't have
any tests where you try to set multiple columns as NOT NULL in a single
ALTER TABLE command; I think this is worth having.  Something like

CREATE TABLE foo (col1 int, col2 int, col3 int);
... create indexes on col1 and col2 ...
alter table foo set col1 not null,
                set col3 not null,
        add constraint bla not null b;
and stuff like that.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"If you have nothing to say, maybe you need just the right tool to help you
not say it."                   (New York Times, about Microsoft PowerPoint)

Hi,

On 2025-04-28 12:36:14 +0800, jian he wrote:
> On Fri, Apr 18, 2025 at 4:07 PM jian he <jian.universality@gmail.com> wrote:
> >
> > I don't have any good ideas to do the regress tests.
> > I use
> >                 ereport(NOTICE,
> >                         errmsg("all not-null constraints on relation
> > \"%s\" are validated by index scan",
> >                                 RelationGetRelationName(oldrel)));
> > to do the tests.
> >
> for tests, just found out i can imitate
> src/test/modules/test_misc/t/001_constraint_validation.pl,
> 
> So I created a file:
> src/test/modules/test_misc/t/008_indexscan_validate_notnull.pl
> for TAP tests.

The tests have not passed in a few weeks:
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/cf%2F5444

Greetings,

Andres Freund