Commit e1ccaff6 authored by Tom Lane's avatar Tom Lane

Rework parsing of ConstraintAttributeSpec to improve NOT VALID handling.

The initial commit of the ALTER TABLE ADD FOREIGN KEY NOT VALID feature
failed to support labeling such constraints as deferrable.  The best fix
for this seems to be to fold NOT VALID into ConstraintAttributeSpec.
That's a bit more general than the documented syntax, but it allows
better-targeted syntax error messages.

In addition, do some mostly-but-not-entirely-cosmetic code review for
the whole NOT VALID patch.
parent e3df3572
......@@ -1898,7 +1898,8 @@
<entry><structfield>convalidated</structfield></entry>
<entry><type>bool</type></entry>
<entry></entry>
<entry>Has the constraint been validated? Can only be false for foreign keys</entry>
<entry>Has the constraint been validated?
Currently, can only be false for foreign keys</entry>
</row>
<row>
......
......@@ -42,9 +42,8 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET ( <replaceable class="PARAMETER">attribute_option</replaceable> = <replaceable class="PARAMETER">value</replaceable> [, ... ] )
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> RESET ( <replaceable class="PARAMETER">attribute_option</replaceable> [, ... ] )
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
ADD <replaceable class="PARAMETER">table_constraint</replaceable>
ADD <replaceable class="PARAMETER">table_constraint_using_index</replaceable>
ADD <replaceable class="PARAMETER">table_constraint</replaceable> [ NOT VALID ]
ADD <replaceable class="PARAMETER">table_constraint_using_index</replaceable>
VALIDATE CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable>
DROP CONSTRAINT [ IF EXISTS ] <replaceable class="PARAMETER">constraint_name</replaceable> [ RESTRICT | CASCADE ]
DISABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
......@@ -235,27 +234,21 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
</varlistentry>
<varlistentry>
<term><literal>ADD <replaceable class="PARAMETER">table_constraint</replaceable>
[ NOT VALID ]</literal></term>
<term><literal>ADD <replaceable class="PARAMETER">table_constraint</replaceable> [ NOT VALID ]</literal></term>
<listitem>
<para>
This form adds a new constraint to a table using the same syntax as
<xref linkend="SQL-CREATETABLE">. Newly added foreign key constraints can
also be defined as <literal>NOT VALID</literal> to avoid the
potentially lengthy initial check that must otherwise be performed.
Constraint checks are skipped at create table time, so
<xref linkend="SQL-CREATETABLE"> does not contain this option.
</para>
</listitem>
</varlistentry>
<varlistentry>
<term><literal>VALIDATE CONSTRAINT</literal></term>
<listitem>
<para>
This form validates a foreign key constraint that was previously created
as <literal>NOT VALID</literal>. Constraints already marked valid do not
cause an error response.
<xref linkend="SQL-CREATETABLE">, plus the option <literal>NOT
VALID</literal>, which is currently only allowed for foreign key
constraints.
If the constraint is marked <literal>NOT VALID</literal>, the
potentially-lengthy initial check to verify that all rows in the table
satisfy the constraint is skipped. The constraint will still be
enforced against subsequent inserts or updates (that is, they'll fail
unless there is a matching row in the referenced table). But the
database will not assume that the constraint holds for all rows in
the table, until it is validated by using the <literal>VALIDATE
CONSTRAINT</literal> option.
</para>
</listitem>
</varlistentry>
......@@ -311,6 +304,21 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
</listitem>
</varlistentry>
<varlistentry>
<term><literal>VALIDATE CONSTRAINT</literal></term>
<listitem>
<para>
This form validates a foreign key constraint that was previously created
as <literal>NOT VALID</literal>, by scanning the table to ensure there
are no unmatched rows. Nothing happens if the constraint is
already marked valid.
The value of separating validation from initial creation of the
constraint is that validation requires a lesser lock on the table
than constraint creation does.
</para>
</listitem>
</varlistentry>
<varlistentry>
<term><literal>DROP CONSTRAINT [ IF EXISTS ]</literal></term>
<listitem>
......
......@@ -258,7 +258,7 @@ static void AlterIndexNamespaces(Relation classRel, Relation rel,
static void AlterSeqNamespaces(Relation classRel, Relation rel,
Oid oldNspOid, Oid newNspOid,
const char *newNspName, LOCKMODE lockmode);
static void ATExecValidateConstraint(Relation rel, const char *constrName);
static void ATExecValidateConstraint(Relation rel, char *constrName);
static int transformColumnNameList(Oid relId, List *colList,
int16 *attnums, Oid *atttypids);
static int transformFkeyGetPrimaryKey(Relation pkrel, Oid *indexOid,
......@@ -5726,9 +5726,9 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
createForeignKeyTriggers(rel, fkconstraint, constrOid, indexOid);
/*
* Tell Phase 3 to check that the constraint is satisfied by existing rows
* We can skip this during table creation or if requested explicitly by
* specifying NOT VALID on an alter table statement.
* Tell Phase 3 to check that the constraint is satisfied by existing rows.
* We can skip this during table creation, or if requested explicitly by
* specifying NOT VALID in an ADD FOREIGN KEY command.
*/
if (!fkconstraint->skip_validation)
{
......@@ -5755,7 +5755,7 @@ ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
* ALTER TABLE VALIDATE CONSTRAINT
*/
static void
ATExecValidateConstraint(Relation rel, const char *constrName)
ATExecValidateConstraint(Relation rel, char *constrName)
{
Relation conrel;
SysScanDesc scan;
......@@ -5810,7 +5810,7 @@ ATExecValidateConstraint(Relation rel, const char *constrName)
*/
refrel = heap_open(con->confrelid, RowShareLock);
validateForeignKeyConstraint((char *) constrName, rel, refrel,
validateForeignKeyConstraint(constrName, rel, refrel,
con->conindid,
conid);
......@@ -5830,6 +5830,7 @@ ATExecValidateConstraint(Relation rel, const char *constrName)
heap_close(conrel, RowExclusiveLock);
}
/*
* transformColumnNameList - transform list of column names
*
......
......@@ -1006,6 +1006,8 @@ ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid)
}
fkcon->deferrable = stmt->deferrable;
fkcon->initdeferred = stmt->initdeferred;
fkcon->skip_validation = false;
fkcon->initially_valid = true;
/* ... and execute it */
ProcessUtility((Node *) atstmt,
......
This diff is collapsed.
......@@ -1751,7 +1751,8 @@ transformFKConstraints(CreateStmtContext *cxt,
/*
* If CREATE TABLE or adding a column with NULL default, we can safely
* skip validation of the constraint.
* skip validation of FK constraints, and nonetheless mark them valid.
* (This will override any user-supplied NOT VALID flag.)
*/
if (skipValidation)
{
......
......@@ -1372,7 +1372,6 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
appendStringInfo(&buf, " DEFERRABLE");
if (conForm->condeferred)
appendStringInfo(&buf, " INITIALLY DEFERRED");
if (!conForm->convalidated)
appendStringInfoString(&buf, " NOT VALID");
......
......@@ -1221,7 +1221,7 @@ typedef enum AlterTableType
AT_DropInherit, /* NO INHERIT parent */
AT_AddOf, /* OF <type_name> */
AT_DropOf, /* NOT OF */
AT_GenericOptions, /* OPTIONS (...) */
AT_GenericOptions /* OPTIONS (...) */
} AlterTableType;
typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */
......@@ -1234,7 +1234,6 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */
* constraint, or parent table */
DropBehavior behavior; /* RESTRICT or CASCADE for DROP cases */
bool missing_ok; /* skip error if missing? */
bool validated;
} AlterTableCmd;
......@@ -1469,8 +1468,9 @@ typedef struct CreateStmt
*
* If skip_validation is true then we skip checking that the existing rows
* in the table satisfy the constraint, and just install the catalog entries
* for the constraint. This is currently used only during CREATE TABLE
* (when we know the table must be empty).
* for the constraint. A new FK constraint is marked as valid iff
* initially_valid is true. (Usually skip_validation and initially_valid
* are inverses, but we can set both true if the table is known empty.)
*
* Constraint attributes (DEFERRABLE etc) are initially represented as
* separate Constraint nodes for simplicity of parsing. parse_utilcmd.c makes
......@@ -1544,7 +1544,7 @@ typedef struct Constraint
char fk_upd_action; /* ON UPDATE action */
char fk_del_action; /* ON DELETE action */
bool skip_validation; /* skip validation of existing rows? */
bool initially_valid; /* start the new constraint as valid */
bool initially_valid; /* mark the new constraint as valid? */
} Constraint;
/* ----------------------
......@@ -2417,7 +2417,7 @@ typedef enum VacuumOption
VACOPT_VERBOSE = 1 << 2, /* print progress info */
VACOPT_FREEZE = 1 << 3, /* FREEZE option */
VACOPT_FULL = 1 << 4, /* FULL (non-concurrent) vacuum */
VACOPT_NOWAIT = 1 << 5
VACOPT_NOWAIT = 1 << 5 /* don't wait to get lock (autovacuum only) */
} VacuumOption;
typedef struct VacuumStmt
......
......@@ -207,12 +207,6 @@ ECPG: CopyStmtCOPYopt_binaryqualified_nameopt_column_listopt_oidscopy_fromcopy_f
ECPG: CopyStmtCOPYselect_with_parensTOcopy_file_nameopt_withcopy_options addon
if (strcmp($4, "stdin") == 0)
mmerror(PARSE_ERROR, ET_ERROR, "COPY TO STDIN is not possible");
ECPG: ConstraintAttributeSpecConstraintDeferrabilitySpecConstraintTimeSpec addon
if (strcmp($1, "deferrable") != 0 && strcmp($2, "initially deferrable") == 0 )
mmerror(PARSE_ERROR, ET_ERROR, "constraint declared INITIALLY DEFERRED must be DEFERRABLE");
ECPG: ConstraintAttributeSpecConstraintTimeSpecConstraintDeferrabilitySpec addon
if (strcmp($2, "deferrable") != 0 && strcmp($1, "initially deferrable") == 0 )
mmerror(PARSE_ERROR, ET_ERROR, "constraint declared INITIALLY DEFERRED must be DEFERRABLE");
ECPG: var_valueNumericOnly addon
if ($1[0] == '$')
{
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment