Allow ALTER TABLE .. SET NOT NULL to skip provably unnecessary scans.

If existing CHECK or NOT NULL constraints preclude the presence
of nulls, we need not look to see whether any are present.

Sergei Kornilov, reviewed by Stephen Frost, Ildar Musin, David Rowley,
and by me.

Discussion: http://postgr.es/m/81911511895540@web58j.yandex.ru
This commit is contained in:
Robert Haas 2019-03-13 08:55:00 -04:00
parent 95fa9f1a13
commit bbb96c3704
4 changed files with 195 additions and 23 deletions

View File

@ -214,8 +214,17 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
<listitem>
<para>
These forms change whether a column is marked to allow null
values or to reject null values. You can only use <literal>SET
NOT NULL</literal> when the column contains no null values.
values or to reject null values.
</para>
<para>
<literal>SET NOT NULL</literal> may only be applied to a column
providing none of the records in the table contain a
<literal>NULL</literal> value for the column. Ordinarily this is
checked during the <literal>ALTER TABLE</literal> by scanning the
entire table; however, if a valid <literal>CHECK</literal> constraint is
found which proves no <literal>NULL</literal> can exist, then the
table scan is skipped.
</para>
<para>

View File

@ -160,7 +160,7 @@ typedef struct AlteredTableInfo
/* Information saved by Phases 1/2 for Phase 3: */
List *constraints; /* List of NewConstraint */
List *newvals; /* List of NewColumnValue */
bool new_notnull; /* T if we added new NOT NULL constraints */
bool verify_new_notnull; /* T if we should recheck NOT NULL */
int rewrite; /* Reason for forced rewrite, if any */
Oid newTableSpace; /* new tablespace; 0 means no change */
bool chgPersistence; /* T if SET LOGGED/UNLOGGED is used */
@ -372,6 +372,9 @@ static ObjectAddress ATExecDropNotNull(Relation rel, const char *colName, LOCKMO
static void ATPrepSetNotNull(Relation rel, bool recurse, bool recursing);
static ObjectAddress ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
const char *colName, LOCKMODE lockmode);
static bool NotNullImpliedByRelConstraints(Relation rel, Form_pg_attribute attr);
static bool ConstraintImpliedByRelConstraint(Relation scanrel,
List *partConstraint, List *existedConstraints);
static ObjectAddress ATExecColumnDefault(Relation rel, const char *colName,
Node *newDefault, LOCKMODE lockmode);
static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName,
@ -4550,10 +4553,11 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
else
{
/*
* Test the current data within the table against new constraints
* generated by ALTER TABLE commands, but don't rebuild data.
* If required, test the current data within the table against new
* constraints generated by ALTER TABLE commands, but don't rebuild
* data.
*/
if (tab->constraints != NIL || tab->new_notnull ||
if (tab->constraints != NIL || tab->verify_new_notnull ||
tab->partition_constraint != NULL)
ATRewriteTable(tab, InvalidOid, lockmode);
@ -4714,13 +4718,13 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode)
}
notnull_attrs = NIL;
if (newrel || tab->new_notnull)
if (newrel || tab->verify_new_notnull)
{
/*
* If we are rebuilding the tuples OR if we added any new NOT NULL
* constraints, check all not-null constraints. This is a bit of
* overkill but it minimizes risk of bugs, and heap_attisnull is a
* pretty cheap test anyway.
* If we are rebuilding the tuples OR if we added any new but not
* verified NOT NULL constraints, check all not-null constraints.
* This is a bit of overkill but it minimizes risk of bugs, and
* heap_attisnull is a pretty cheap test anyway.
*/
for (i = 0; i < newTupDesc->natts; i++)
{
@ -5749,11 +5753,9 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
{
/*
* If the new column is NOT NULL, and there is no missing value,
* tell Phase 3 it needs to test that. (Note we don't do this for
* an OID column. OID will be marked not null, but since it's
* filled specially, there's no need to test anything.)
* tell Phase 3 it needs to check for NULLs.
*/
tab->new_notnull |= colDef->is_not_null;
tab->verify_new_notnull |= colDef->is_not_null;
}
}
@ -6121,8 +6123,19 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
CatalogTupleUpdate(attr_rel, &tuple->t_self, tuple);
/* Tell Phase 3 it needs to test the constraint */
tab->new_notnull = true;
/*
* Ordinarily phase 3 must ensure that no NULLs exist in columns that
* are set NOT NULL; however, if we can find a constraint which proves
* this then we can skip that. We needn't bother looking if
* we've already found that we must verify some other NOT NULL
* constraint.
*/
if (!tab->verify_new_notnull &&
!NotNullImpliedByRelConstraints(rel, (Form_pg_attribute) GETSTRUCT(tuple)))
{
/* Tell Phase 3 it needs to test the constraint */
tab->verify_new_notnull = true;
}
ObjectAddressSubSet(address, RelationRelationId,
RelationGetRelid(rel), attnum);
@ -6138,6 +6151,42 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
return address;
}
/*
* NotNullImpliedByRelConstraints
* Does rel's existing constraints imply NOT NULL for the given attribute?
*/
static bool
NotNullImpliedByRelConstraints(Relation rel, Form_pg_attribute attr)
{
NullTest *nnulltest = makeNode(NullTest);
nnulltest->arg = (Expr *) makeVar(1,
attr->attnum,
attr->atttypid,
attr->atttypmod,
attr->attcollation,
0);
nnulltest->nulltesttype = IS_NOT_NULL;
/*
* argisrow = false is correct even for a composite column, because
* attnotnull does not represent a SQL-spec IS NOT NULL test in such a
* case, just IS DISTINCT FROM NULL.
*/
nnulltest->argisrow = false;
nnulltest->location = -1;
if (ConstraintImpliedByRelConstraint(rel, list_make1(nnulltest), NIL))
{
ereport(DEBUG1,
(errmsg("existing constraints on column \"%s\".\"%s\" are sufficient to prove that it does not contain nulls",
RelationGetRelationName(rel), NameStr(attr->attname))));
return true;
}
return false;
}
/*
* ALTER TABLE ALTER COLUMN SET/DROP DEFAULT
*
@ -14416,8 +14465,7 @@ PartConstraintImpliedByRelConstraint(Relation scanrel,
{
List *existConstraint = NIL;
TupleConstr *constr = RelationGetDescr(scanrel)->constr;
int num_check,
i;
int i;
if (constr && constr->has_not_null)
{
@ -14451,6 +14499,27 @@ PartConstraintImpliedByRelConstraint(Relation scanrel,
}
}
return ConstraintImpliedByRelConstraint(scanrel, partConstraint, existConstraint);
}
/*
* ConstraintImpliedByRelConstraint
* Do scanrel's existing constraints imply the given constraint?
*
* testConstraint is the constraint to validate. provenConstraint is a
* caller-provided list of conditions which this function may assume
* to be true. Both provenConstraint and testConstraint must be in
* implicit-AND form, must only contain immutable clauses, and must
* contain only Vars with varno = 1.
*/
bool
ConstraintImpliedByRelConstraint(Relation scanrel, List *testConstraint, List *provenConstraint)
{
List *existConstraint = list_copy(provenConstraint);
TupleConstr *constr = RelationGetDescr(scanrel)->constr;
int num_check,
i;
num_check = (constr != NULL) ? constr->num_check : 0;
for (i = 0; i < num_check; i++)
{
@ -14481,13 +14550,13 @@ PartConstraintImpliedByRelConstraint(Relation scanrel,
/*
* Try to make the proof. Since we are comparing CHECK constraints, we
* need to use weak implication, i.e., we assume existConstraint is
* not-false and try to prove the same for partConstraint.
* not-false and try to prove the same for testConstraint.
*
* Note that predicate_implied_by assumes its first argument is known
* immutable. That should always be true for partition constraints, so we
* don't test it here.
* immutable. That should always be true for both NOT NULL and
* partition constraints, so we don't test it here.
*/
return predicate_implied_by(partConstraint, existConstraint, true);
return predicate_implied_by(testConstraint, existConstraint, true);
}
/*

View File

@ -1028,6 +1028,8 @@ insert into atacc1 (test2, test) values (1, NULL);
ERROR: null value in column "test" violates not-null constraint
DETAIL: Failing row contains (null, 1).
drop table atacc1;
-- we want check if not null was implied by constraint
set client_min_messages to 'debug1';
-- alter table / alter column [set/drop] not null tests
-- try altering system catalogs, should fail
alter table pg_class alter column relname drop not null;
@ -1043,15 +1045,19 @@ ERROR: relation "non_existent" does not exist
-- test checking for null values and primary key
create table atacc1 (test int not null);
alter table atacc1 add constraint "atacc1_pkey" primary key (test);
DEBUG: ALTER TABLE / ADD PRIMARY KEY will create implicit index "atacc1_pkey" for table "atacc1"
DEBUG: building index "atacc1_pkey" on table "atacc1" serially
alter table atacc1 alter column test drop not null;
ERROR: column "test" is in a primary key
alter table atacc1 drop constraint "atacc1_pkey";
alter table atacc1 alter column test drop not null;
insert into atacc1 values (null);
alter table atacc1 alter test set not null;
DEBUG: verifying table "atacc1"
ERROR: column "test" contains null values
delete from atacc1;
alter table atacc1 alter test set not null;
DEBUG: verifying table "atacc1"
-- try altering a non-existent column, should fail
alter table atacc1 alter bar set not null;
ERROR: column "bar" of relation "atacc1" does not exist
@ -1065,6 +1071,54 @@ alter table myview alter column test set not null;
ERROR: "myview" is not a table or foreign table
drop view myview;
drop table atacc1;
-- set not null verified by constraints
create table atacc1 (test_a int, test_b int);
insert into atacc1 values (null, 1);
-- constraint not cover all values, should fail
alter table atacc1 add constraint atacc1_constr_or check(test_a is not null or test_b < 10);
DEBUG: verifying table "atacc1"
alter table atacc1 alter test_a set not null;
DEBUG: verifying table "atacc1"
ERROR: column "test_a" contains null values
alter table atacc1 drop constraint atacc1_constr_or;
-- not valid constraint, should fail
alter table atacc1 add constraint atacc1_constr_invalid check(test_a is not null) not valid;
alter table atacc1 alter test_a set not null;
DEBUG: verifying table "atacc1"
ERROR: column "test_a" contains null values
alter table atacc1 drop constraint atacc1_constr_invalid;
-- with valid constraint
update atacc1 set test_a = 1;
alter table atacc1 add constraint atacc1_constr_a_valid check(test_a is not null);
DEBUG: verifying table "atacc1"
alter table atacc1 alter test_a set not null;
DEBUG: existing constraints on column "atacc1"."test_a" are sufficient to prove that it does not contain nulls
delete from atacc1;
insert into atacc1 values (2, null);
alter table atacc1 alter test_a drop not null;
-- test multiple set not null at same time
-- test_a checked by atacc1_constr_a_valid, test_b should fail by table scan
alter table atacc1 alter test_a set not null, alter test_b set not null;
DEBUG: existing constraints on column "atacc1"."test_a" are sufficient to prove that it does not contain nulls
DEBUG: verifying table "atacc1"
ERROR: column "test_b" contains null values
-- commands order has no importance
alter table atacc1 alter test_b set not null, alter test_a set not null;
DEBUG: verifying table "atacc1"
ERROR: column "test_b" contains null values
-- valid one by table scan, one by check constraints
update atacc1 set test_b = 1;
alter table atacc1 alter test_b set not null, alter test_a set not null;
DEBUG: verifying table "atacc1"
alter table atacc1 alter test_a drop not null, alter test_b drop not null;
-- both column has check constraints
alter table atacc1 add constraint atacc1_constr_b_valid check(test_b is not null);
DEBUG: verifying table "atacc1"
alter table atacc1 alter test_b set not null, alter test_a set not null;
DEBUG: existing constraints on column "atacc1"."test_b" are sufficient to prove that it does not contain nulls
DEBUG: existing constraints on column "atacc1"."test_a" are sufficient to prove that it does not contain nulls
drop table atacc1;
reset client_min_messages;
-- test inheritance
create table parent (a int);
create table child (b varchar(255)) inherits (parent);

View File

@ -776,6 +776,9 @@ insert into atacc1 (test2, test) values (2, 3);
insert into atacc1 (test2, test) values (1, NULL);
drop table atacc1;
-- we want check if not null was implied by constraint
set client_min_messages to 'debug1';
-- alter table / alter column [set/drop] not null tests
-- try altering system catalogs, should fail
alter table pg_class alter column relname drop not null;
@ -809,6 +812,43 @@ drop view myview;
drop table atacc1;
-- set not null verified by constraints
create table atacc1 (test_a int, test_b int);
insert into atacc1 values (null, 1);
-- constraint not cover all values, should fail
alter table atacc1 add constraint atacc1_constr_or check(test_a is not null or test_b < 10);
alter table atacc1 alter test_a set not null;
alter table atacc1 drop constraint atacc1_constr_or;
-- not valid constraint, should fail
alter table atacc1 add constraint atacc1_constr_invalid check(test_a is not null) not valid;
alter table atacc1 alter test_a set not null;
alter table atacc1 drop constraint atacc1_constr_invalid;
-- with valid constraint
update atacc1 set test_a = 1;
alter table atacc1 add constraint atacc1_constr_a_valid check(test_a is not null);
alter table atacc1 alter test_a set not null;
delete from atacc1;
insert into atacc1 values (2, null);
alter table atacc1 alter test_a drop not null;
-- test multiple set not null at same time
-- test_a checked by atacc1_constr_a_valid, test_b should fail by table scan
alter table atacc1 alter test_a set not null, alter test_b set not null;
-- commands order has no importance
alter table atacc1 alter test_b set not null, alter test_a set not null;
-- valid one by table scan, one by check constraints
update atacc1 set test_b = 1;
alter table atacc1 alter test_b set not null, alter test_a set not null;
alter table atacc1 alter test_a drop not null, alter test_b drop not null;
-- both column has check constraints
alter table atacc1 add constraint atacc1_constr_b_valid check(test_b is not null);
alter table atacc1 alter test_b set not null, alter test_a set not null;
drop table atacc1;
reset client_min_messages;
-- test inheritance
create table parent (a int);
create table child (b varchar(255)) inherits (parent);