database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-08-18 12:22:09 +03:00
Code Activity

Revert structural changes to not-null constraints

Browse Source 
There are some problems with the new way to handle these constraints
that were detected at the last minute, and require fixes that appear too
invasive to be doing this late in the cycle.  Revert this (again) for
now, we'll try again with these problems fixed.

The following commits are reverted:

    b0e96f3119  Catalog not-null constraints
    9b581c5341  Disallow changing NO INHERIT status of a not-null constraint
    d0ec2ddbe0  Fix not-null constraint test
    ac22a9545c  Move privilege check to the right place
    b0f7dd915b  Check stack depth in new recursive functions
    3af7217942  Update information_schema definition for not-null constraints
    c3709100be  Fix propagating attnotnull in multiple inheritance
    d9f686a72e  Fix restore of not-null constraints with inheritance
    d72d32f52d  Don't try to assign smart names to constraints
    0cd711271d  Better handle indirect constraint drops
    13daa33fa5  Disallow NO INHERIT not-null constraints on partitioned tables
    d45597f72f  Disallow direct change of NO INHERIT of not-null constraints
    21ac38f498  Fix inconsistencies in error messages

Discussion: https://postgr.es/m/202405110940.joxlqcx4dogd@alvherre.pgsql
This commit is contained in:
Alvaro Herrera
2024-05-13 11:31:09 +02:00
parent e89f4c6618
commit 6f8bb7c1e9
49 changed files with 715 additions and 4606 deletions
Download Patch File Download Diff File Expand all files Collapse all files

288
src/backend/parser/parse_utilcmd.c
View File

@@ -84,7 +84,6 @@ typedef struct
bool isalter; /* true if altering existing table */
List *columns; /* ColumnDef items */
List *ckconstraints; /* CHECK constraints */
List *nnconstraints; /* NOT NULL constraints */
List *fkconstraints; /* FOREIGN KEY constraints */
List *ixconstraints; /* index-creating constraints */
List *likeclauses; /* LIKE clauses that need post-processing */
@@ -244,7 +243,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
cxt.isalter = false;
cxt.columns = NIL;
cxt.ckconstraints = NIL;
cxt.nnconstraints = NIL;
cxt.fkconstraints = NIL;
cxt.ixconstraints = NIL;
cxt.likeclauses = NIL;
@@ -351,7 +349,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
*/
stmt->tableElts = cxt.columns;
stmt->constraints = cxt.ckconstraints;
stmt->nnconstraints = cxt.nnconstraints;
result = lappend(cxt.blist, stmt);
result = list_concat(result, cxt.alist);
@@ -550,7 +547,6 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
bool saw_default;
bool saw_identity;
bool saw_generated;
bool need_notnull = false;
ListCell *clist;
cxt->columns = lappend(cxt->columns, column);
@@ -648,8 +644,10 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
constraint->cooked_expr = NULL;
column->constraints = lappend(column->constraints, constraint);
/* have a not-null constraint added later */
need_notnull = true;
constraint = makeNode(Constraint);
constraint->contype = CONSTR_NOTNULL;
constraint->location = -1;
column->constraints = lappend(column->constraints, constraint);
}
/* Process column constraints, if any... */
@@ -667,7 +665,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
switch (constraint->contype)
{
case CONSTR_NULL:
if ((saw_nullable && column->is_not_null) || need_notnull)
if (saw_nullable && column->is_not_null)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"",
@@ -679,14 +677,6 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
break;
case CONSTR_NOTNULL:
if (cxt->ispartitioned && constraint->is_no_inherit)
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("not-null constraints on partitioned tables cannot be NO INHERIT"));
/*
* Disallow conflicting [NOT] NULL markings
*/
if (saw_nullable && !column->is_not_null)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -694,25 +684,8 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
column->colname, cxt->relation->relname),
parser_errposition(cxt->pstate,
constraint->location)));
/* Ignore redundant NOT NULL markings */
/*
* If this is the first time we see this column being marked
* not null, add the constraint entry; and get rid of any
* previous markings to mark the column NOT NULL.
*/
if (!column->is_not_null)
{
column->is_not_null = true;
saw_nullable = true;
constraint->keys = list_make1(makeString(column->colname));
cxt->nnconstraints = lappend(cxt->nnconstraints, constraint);
/* Don't need this anymore, if we had it */
need_notnull = false;
}
column->is_not_null = true;
saw_nullable = true;
break;
case CONSTR_DEFAULT:
@@ -762,19 +735,16 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
column->identity = constraint->generated_when;
saw_identity = true;
/*
* Identity columns are always NOT NULL, but we may have a
* constraint already.
*/
if (!saw_nullable)
need_notnull = true;
else if (!column->is_not_null)
/* An identity column is implicitly NOT NULL */
if (saw_nullable && !column->is_not_null)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("conflicting NULL/NOT NULL declarations for column \"%s\" of table \"%s\"",
column->colname, cxt->relation->relname),
parser_errposition(cxt->pstate,
constraint->location)));
column->is_not_null = true;
saw_nullable = true;
break;
}
@@ -880,29 +850,6 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
constraint->location)));
}
/*
* If we need a not-null constraint for SERIAL or IDENTITY, and one was
* not explicitly specified, add one now.
*/
if (need_notnull && !(saw_nullable && column->is_not_null))
{
Constraint *notnull;
column->is_not_null = true;
notnull = makeNode(Constraint);
notnull->contype = CONSTR_NOTNULL;
notnull->conname = NULL;
notnull->deferrable = false;
notnull->initdeferred = false;
notnull->location = -1;
notnull->keys = list_make1(makeString(column->colname));
notnull->skip_validation = false;
notnull->initially_valid = true;
cxt->nnconstraints = lappend(cxt->nnconstraints, notnull);
}
/*
* If needed, generate ALTER FOREIGN TABLE ALTER COLUMN statement to add
* per-column foreign data wrapper options to this column after creation.
@@ -972,16 +919,6 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
cxt->ckconstraints = lappend(cxt->ckconstraints, constraint);
break;
case CONSTR_NOTNULL:
if (cxt->ispartitioned && constraint->is_no_inherit)
ereport(ERROR,
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("not-null constraints on partitioned tables cannot be NO INHERIT"));
cxt->nnconstraints = lappend(cxt->nnconstraints, constraint);
break;
case CONSTR_FOREIGN:
if (cxt->isforeign)
ereport(ERROR,
@@ -993,6 +930,7 @@ transformTableConstraint(CreateStmtContext *cxt, Constraint *constraint)
break;
case CONSTR_NULL:
case CONSTR_NOTNULL:
case CONSTR_DEFAULT:
case CONSTR_ATTR_DEFERRABLE:
case CONSTR_ATTR_NOT_DEFERRABLE:
@@ -1028,7 +966,6 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
AclResult aclresult;
char *comment;
ParseCallbackState pcbstate;
bool process_notnull_constraints = false;
setup_parser_errposition_callback(&pcbstate, cxt->pstate,
table_like_clause->relation->location);
@@ -1097,18 +1034,14 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
continue;
/*
* Create a new column definition
* Create a new column, which is marked as NOT inherited.
*
* For constraints, ONLY the not-null constraint is inherited by the
* new column definition per SQL99.
*/
def = makeColumnDef(NameStr(attribute->attname), attribute->atttypid,
attribute->atttypmod, attribute->attcollation);
/*
* For constraints, ONLY the not-null constraint is inherited by the
* new column definition per SQL99; however we cannot do that
* correctly here, so we leave it for expandTableLikeClause to handle.
*/
if (attribute->attnotnull)
process_notnull_constraints = true;
def->is_not_null = attribute->attnotnull;
/*
* Add to column list
@@ -1182,77 +1115,19 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
* we don't yet know what column numbers the copied columns will have in
* the finished table. If any of those options are specified, add the
* LIKE clause to cxt->likeclauses so that expandTableLikeClause will be
* called after we do know that; in addition, do that if there are any NOT
* NULL constraints, because those must be propagated even if not
* explicitly requested.
*
* In order for this to work, we remember the relation OID so that
* called after we do know that. Also, remember the relation OID so that
* expandTableLikeClause is certain to open the same table.
*/
if ((table_like_clause->options &
(CREATE_TABLE_LIKE_DEFAULTS |
CREATE_TABLE_LIKE_GENERATED |
CREATE_TABLE_LIKE_CONSTRAINTS |
CREATE_TABLE_LIKE_INDEXES)) ||
process_notnull_constraints)
if (table_like_clause->options &
(CREATE_TABLE_LIKE_DEFAULTS |
CREATE_TABLE_LIKE_GENERATED |
CREATE_TABLE_LIKE_CONSTRAINTS |
CREATE_TABLE_LIKE_INDEXES))
{
table_like_clause->relationOid = RelationGetRelid(relation);
cxt->likeclauses = lappend(cxt->likeclauses, table_like_clause);
}
/*
* If INCLUDING INDEXES is not given and a primary key exists, we need to
* add not-null constraints to the columns covered by the PK (except those
* that already have one.) This is required for backwards compatibility.
*/
if ((table_like_clause->options & CREATE_TABLE_LIKE_INDEXES) == 0)
{
Bitmapset *pkcols;
int x = -1;
Bitmapset *donecols = NULL;
ListCell *lc;
/*
* Obtain a bitmapset of columns on which we'll add not-null
* constraints in expandTableLikeClause, so that we skip this for
* those.
*/
foreach(lc, RelationGetNotNullConstraints(RelationGetRelid(relation), true))
{
CookedConstraint *cooked = (CookedConstraint *) lfirst(lc);
donecols = bms_add_member(donecols, cooked->attnum);
}
pkcols = RelationGetIndexAttrBitmap(relation,
INDEX_ATTR_BITMAP_PRIMARY_KEY);
while ((x = bms_next_member(pkcols, x)) >= 0)
{
Constraint *notnull;
AttrNumber attnum = x + FirstLowInvalidHeapAttributeNumber;
Form_pg_attribute attForm;
/* ignore if we already have one for this column */
if (bms_is_member(attnum, donecols))
continue;
attForm = TupleDescAttr(tupleDesc, attnum - 1);
notnull = makeNode(Constraint);
notnull->contype = CONSTR_NOTNULL;
notnull->conname = NULL;
notnull->is_no_inherit = false;
notnull->deferrable = false;
notnull->initdeferred = false;
notnull->location = -1;
notnull->keys = list_make1(makeString(pstrdup(NameStr(attForm->attname))));
notnull->skip_validation = false;
notnull->initially_valid = true;
cxt->nnconstraints = lappend(cxt->nnconstraints, notnull);
}
}
/*
* We may copy extended statistics if requested, since the representation
* of CreateStatsStmt doesn't depend on column numbers.
@@ -1319,8 +1194,6 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
TupleConstr *constr;
AttrMap *attmap;
char *comment;
bool at_pushed = false;
ListCell *lc;
/*
* Open the relation referenced by the LIKE clause. We should still have
@@ -1491,20 +1364,6 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
}
}
/*
* Copy not-null constraints, too (these do not require any option to have
* been given).
*/
foreach(lc, RelationGetNotNullConstraints(RelationGetRelid(relation), false))
{
AlterTableCmd *atsubcmd;
atsubcmd = makeNode(AlterTableCmd);
atsubcmd->subtype = AT_AddConstraint;
atsubcmd->def = (Node *) lfirst_node(Constraint, lc);
atsubcmds = lappend(atsubcmds, atsubcmd);
}
/*
* If we generated any ALTER TABLE actions above, wrap them into a single
* ALTER TABLE command. Stick it at the front of the result, so it runs
@@ -1519,8 +1378,6 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
atcmd->objtype = OBJECT_TABLE;
atcmd->missing_ok = false;
result = lcons(atcmd, result);
at_pushed = true;
}
/*
@@ -1548,39 +1405,6 @@ expandTableLikeClause(RangeVar *heapRel, TableLikeClause *table_like_clause)
attmap,
NULL);
/*
* The PK columns might not yet non-nullable, so make sure they
* become so.
*/
if (index_stmt->primary)
{
foreach(lc, index_stmt->indexParams)
{
IndexElem *col = lfirst_node(IndexElem, lc);
AlterTableCmd *notnullcmd = makeNode(AlterTableCmd);
notnullcmd->subtype = AT_SetAttNotNull;
notnullcmd->name = pstrdup(col->name);
/* Luckily we can still add more AT-subcmds here */
atsubcmds = lappend(atsubcmds, notnullcmd);
}
/*
* If we had already put the AlterTableStmt into the output
* list, we don't need to do so again; otherwise do it.
*/
if (!at_pushed)
{
AlterTableStmt *atcmd = makeNode(AlterTableStmt);
atcmd->relation = copyObject(heapRel);
atcmd->cmds = atsubcmds;
atcmd->objtype = OBJECT_TABLE;
atcmd->missing_ok = false;
result = lcons(atcmd, result);
}
}
/* Copy comment on index, if requested */
if (table_like_clause->options & CREATE_TABLE_LIKE_COMMENTS)
{
@@ -1661,8 +1485,8 @@ transformOfType(CreateStmtContext *cxt, TypeName *ofTypename)
* with the index there.
*
* Unlike transformIndexConstraint, we don't make any effort to force primary
* key columns to be not-null. The larger cloning process this is part of
* should have cloned their not-null status separately (and DefineIndex will
* key columns to be NOT NULL. The larger cloning process this is part of
* should have cloned their NOT NULL status separately (and DefineIndex will
* complain if that fails to happen).
*/
IndexStmt *
@@ -2210,12 +2034,10 @@ transformIndexConstraints(CreateStmtContext *cxt)
ListCell *lc;
/*
* Run through the constraints that need to generate an index, and do so.
*
* For PRIMARY KEY, in addition we set each column's attnotnull flag true.
* We do not create a separate not-null constraint, as that would be
* redundant: the PRIMARY KEY constraint itself fulfills that role. Other
* constraint types don't need any not-null markings.
* Run through the constraints that need to generate an index. For PRIMARY
* KEY, mark each column as NOT NULL and create an index. For UNIQUE or
* EXCLUDE, create an index as for PRIMARY KEY, but do not insist on NOT
* NULL.
*/
foreach(lc, cxt->ixconstraints)
{
@@ -2289,7 +2111,9 @@ transformIndexConstraints(CreateStmtContext *cxt)
}
/*
* Now append all the IndexStmts to cxt->alist.
* Now append all the IndexStmts to cxt->alist. If we generated an ALTER
* TABLE SET NOT NULL statement to support a primary key, it's already in
* cxt->alist.
*/
cxt->alist = list_concat(cxt->alist, finalindexlist);
}
@@ -2297,10 +2121,12 @@ transformIndexConstraints(CreateStmtContext *cxt)
/*
* transformIndexConstraint
* Transform one UNIQUE, PRIMARY KEY, or EXCLUDE constraint for
* transformIndexConstraints. An IndexStmt is returned.
* transformIndexConstraints.
*
* For a PRIMARY KEY constraint, we additionally force the columns to be
* marked as not-null, without producing a not-null constraint.
* We return an IndexStmt. For a PRIMARY KEY constraint, we additionally
* produce not-null constraints, either by marking ColumnDefs in cxt->columns
* as is_not_null or by adding an ALTER TABLE SET NOT NULL command to
* cxt->alist.
*/
static IndexStmt *
transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
@@ -2564,7 +2390,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
* For UNIQUE and PRIMARY KEY, we just have a list of column names.
*
* Make sure referenced keys exist. If we are making a PRIMARY KEY index,
* also make sure they are not-null.
* also make sure they are NOT NULL.
*/
else
{
@@ -2572,6 +2398,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
{
char *key = strVal(lfirst(lc));
bool found = false;
bool forced_not_null = false;
ColumnDef *column = NULL;
ListCell *columns;
IndexElem *iparam;
@@ -2592,14 +2419,13 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
* column is defined in the new table. For PRIMARY KEY, we
* can apply the not-null constraint cheaply here ... unless
* the column is marked is_from_type, in which case marking it
* here would be ineffective (see MergeAttributes). Note that
* this isn't effective in ALTER TABLE either, unless the
* column is being added in the same command.
* here would be ineffective (see MergeAttributes).
*/
if (constraint->contype == CONSTR_PRIMARY &&
!column->is_from_type)
{
column->is_not_null = true;
forced_not_null = true;
}
}
else if (SystemAttributeByName(key) != NULL)
@@ -2607,7 +2433,7 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
/*
* column will be a system column in the new table, so accept
* it. System columns can't ever be null, so no need to worry
* about PRIMARY/NOT NULL constraint.
* about PRIMARY/not-null constraint.
*/
found = true;
}
@@ -2642,6 +2468,14 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
if (strcmp(key, inhname) == 0)
{
found = true;
/*
* It's tempting to set forced_not_null if the
* parent column is already NOT NULL, but that
* seems unsafe because the column's NOT NULL
* marking might disappear between now and
* execution. Do the runtime check to be safe.
*/
break;
}
}
@@ -2695,11 +2529,15 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
iparam->nulls_ordering = SORTBY_NULLS_DEFAULT;
index->indexParams = lappend(index->indexParams, iparam);
if (constraint->contype == CONSTR_PRIMARY)
/*
* For a primary-key column, also create an item for ALTER TABLE
* SET NOT NULL if we couldn't ensure it via is_not_null above.
*/
if (constraint->contype == CONSTR_PRIMARY && !forced_not_null)
{
AlterTableCmd *notnullcmd = makeNode(AlterTableCmd);
notnullcmd->subtype = AT_SetAttNotNull;
notnullcmd->subtype = AT_SetNotNull;
notnullcmd->name = pstrdup(key);
notnullcmds = lappend(notnullcmds, notnullcmd);
}
@@ -3642,7 +3480,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
cxt.isalter = true;
cxt.columns = NIL;
cxt.ckconstraints = NIL;
cxt.nnconstraints = NIL;
cxt.fkconstraints = NIL;
cxt.ixconstraints = NIL;
cxt.likeclauses = NIL;
@@ -3912,8 +3749,8 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
/*
* We assume here that cxt.alist contains only IndexStmts and possibly
* AT_SetAttNotNull statements generated from primary key constraints.
* We absorb the subcommands of the latter directly.
* ALTER TABLE SET NOT NULL statements generated from primary key
* constraints. We absorb the subcommands of the latter directly.
*/
if (IsA(istmt, IndexStmt))
{
@@ -3936,7 +3773,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
}
cxt.alist = NIL;
/* Append any CHECK, NOT NULL or FK constraints to the commands list */
/* Append any CHECK or FK constraints to the commands list */
foreach(l, cxt.ckconstraints)
{
newcmd = makeNode(AlterTableCmd);
@@ -3944,13 +3781,6 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
newcmd->def = (Node *) lfirst_node(Constraint, l);
newcmds = lappend(newcmds, newcmd);
}
foreach(l, cxt.nnconstraints)
{
newcmd = makeNode(AlterTableCmd);
newcmd->subtype = AT_AddConstraint;
newcmd->def = (Node *) lfirst_node(Constraint, l);
newcmds = lappend(newcmds, newcmd);
}
foreach(l, cxt.fkconstraints)
{
newcmd = makeNode(AlterTableCmd);