diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index d979ce2..424f608 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -60,6 +60,7 @@ #include "commands/typecmds.h" #include "commands/user.h" #include "executor/executor.h" +#include "executor/spi.h" #include "foreign/foreign.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -370,6 +371,8 @@ 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 isSetNotNullNeedTableScan(Relation rel, Form_pg_attribute attr); +static bool ConstraintImpliedByRelConstraint(Relation scanrel, List *partConstraint); static ObjectAddress ATExecColumnDefault(Relation rel, const char *colName, Node *newDefault, LOCKMODE lockmode); static ObjectAddress ATExecAddIdentity(Relation rel, const char *colName, @@ -5863,8 +5866,10 @@ 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; + if (isSetNotNullNeedTableScan(rel, (Form_pg_attribute) GETSTRUCT(tuple))) { + /* Tell Phase 3 it needs to test the constraint */ + tab->new_notnull = true; + } ObjectAddressSubSet(address, RelationRelationId, RelationGetRelid(rel), attnum); @@ -5880,6 +5885,148 @@ ATExecSetNotNull(AlteredTableInfo *tab, Relation rel, return address; } +static bool +isSetNotNullNeedTableScan(Relation rel, Form_pg_attribute attr) +{ + List *indexoidlist; + ListCell *indexoidscan; + bool isMatchedIndexFound = false; + StringInfoData querybuf; + int indexAttr; + int checkIndexAttrNum; + + // same part in PartConstraintImpliedByRelConstraint + List *notNullConstraint = NIL; + NullTest *nnulltest = makeNode(NullTest); + List *nullConstraint = NIL; + NullTest *ntest = 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; + notNullConstraint = lappend(notNullConstraint, nnulltest); + + // first check existed constraints + if (ConstraintImpliedByRelConstraint(rel, notNullConstraint)) { + ereport(DEBUG1, + (errmsg("verifying table \"%s\" NOT NULL constraint " + "on %s attribute by existed constraints", + RelationGetRelationName(rel), NameStr(attr->attname)))); + return false; + } + + ntest->arg = (Expr *) makeVar(1, + attr->attnum, + attr->atttypid, + attr->atttypmod, + attr->attcollation, + 0); + ntest->nulltesttype = IS_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. + */ + ntest->argisrow = false; + ntest->location = -1; + nullConstraint = lappend(nullConstraint, ntest); + + indexoidlist = RelationGetIndexList(rel); + foreach(indexoidscan, indexoidlist) + { + Oid indexoid = lfirst_oid(indexoidscan); + Relation indexRel; + Form_pg_index indexStruct; + + indexRel = index_open(indexoid, AccessShareLock); + indexStruct = indexRel->rd_index; + + if (IndexIsValid(indexStruct)) + { + if (RelationGetIndexPredicate(indexRel) != NIL) { + if (predicate_implied_by( + RelationGetIndexPredicate(indexRel), + nullConstraint, + false + )) { + // regardless indexed attributes we can use this index by predicate + isMatchedIndexFound = true; + } + } else if (indexRel->rd_amroutine->amsearchnulls && indexStruct->indnatts > 0) { + checkIndexAttrNum = (indexRel->rd_amroutine->amoptionalkey ? indexStruct->indnatts : 1); + + for (indexAttr = 0; indexAttr < checkIndexAttrNum; ++indexAttr) { + if (attr->attnum == indexStruct->indkey.values[ indexAttr ]) { + // index has no predicate and cover target attribute + isMatchedIndexFound = true; + break; + } + } + } + } + + if (isMatchedIndexFound) { + ereport(DEBUG1, + (errmsg("verifying table \"%s\" NOT NULL constraint " + "on %s attribute by index", + RelationGetRelationName(rel), + NameStr(attr->attname)))); + + initStringInfo(&querybuf); + appendStringInfo( + &querybuf, + "SELECT 1 FROM %s WHERE %s IS NULL LIMIT 1", + quote_qualified_identifier( + get_namespace_name(RelationGetNamespace(rel)), + RelationGetRelationName(rel) + ), + quote_identifier(NameStr(attr->attname)) + ); + + /* Open SPI context. */ + if (SPI_connect() != SPI_OK_CONNECT) + elog(ERROR, "SPI_connect failed"); + if (SPI_execute(querybuf.data, false, 1) != SPI_OK_SELECT) + elog(ERROR, "SPI_exec failed: %s", querybuf.data); + if (SPI_processed > 0) + { + ereport(ERROR, + (errcode(ERRCODE_NOT_NULL_VIOLATION), + errmsg("column \"%s\" contains null values", + NameStr(attr->attname)), + errtablecol(rel, attr->attnum))); + } + + /* Close SPI context. */ + if (SPI_finish() != SPI_OK_FINISH) + elog(ERROR, "SPI_finish failed"); + } + + index_close(indexRel, AccessShareLock); + + if (isMatchedIndexFound) { + break; + } + } + + list_free(indexoidlist); + + return ! isMatchedIndexFound; +} + /* * ALTER TABLE ALTER COLUMN SET/DROP DEFAULT * @@ -13628,6 +13775,13 @@ bool PartConstraintImpliedByRelConstraint(Relation scanrel, List *partConstraint) { + return ConstraintImpliedByRelConstraint(scanrel, partConstraint); +} + +static bool +ConstraintImpliedByRelConstraint(Relation scanrel, + List *partConstraint) +{ List *existConstraint = NIL; TupleConstr *constr = RelationGetDescr(scanrel)->constr; int num_check, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 11f0baa..38c007e 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1005,6 +1005,14 @@ alter table atacc1 alter test set not null; ERROR: column "test" contains null values delete from atacc1; alter table atacc1 alter test set not null; +-- set not null can use index +create index on atacc1 (test); +alter table atacc1 alter column test drop not null; +insert into atacc1 values (null); +alter table atacc1 alter test set not null; +ERROR: column "test" contains null values +delete from atacc1; +alter table atacc1 alter test set not null; -- 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 diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 02a33ca..8a1e007 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -763,6 +763,14 @@ alter table atacc1 alter test set not null; delete from atacc1; alter table atacc1 alter test set not null; +-- set not null can use index +create index on atacc1 (test); +alter table atacc1 alter column test drop not null; +insert into atacc1 values (null); +alter table atacc1 alter test set not null; +delete from atacc1; +alter table atacc1 alter test set not null; + -- try altering a non-existent column, should fail alter table atacc1 alter bar set not null; alter table atacc1 alter bar drop not null;