On 2025/5/27 11:54, Michael Paquier wrote:
On Tue, May 27, 2025 at 11:06:46AM +0800, Quan Zongliang wrote:I noticed that the subtype of AlterDomainStmt is directly using constants in the code. It is not conducive to the maintenance and reading of the code. Based on the definition of AlterTableType, use "AD_" as the prefix. Define several macros to replace the original characters. The subtype of AlterTableCmd is defined using an enumeration. The subtypes of AlterDomainStmt are relatively few in number, and the original definition uses characters. These definitions still use characters and maintain the values unchanged. If some plugins or tools are also processing AlterDomainStmt, there will be no errors.Sounds like a good idea. As far as I can see after a closer lookup at the tree, you have updated all the code paths that matter for this change, and you have added a CF entry: https://commitfest.postgresql.org/patch/5780/ +#define AD_VaidateConstraint 'V' /* VALIDATE CONSTRAINT */
Updated Thank you.
s/Vaidate/Validate -- Michael
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 54ad38247aa..3f09f85a480 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15696,7 +15696,7 @@ ATPostAlterTypeParse(Oid oldId, Oid oldRelId, Oid
refRelId, char *cmd,
{
AlterDomainStmt *stmt = (AlterDomainStmt *) stm;
- if (stmt->subtype == 'C') /* ADD CONSTRAINT */
+ if (stmt->subtype == AD_AddConstraint)
{
Constraint *con = castNode(Constraint,
stmt->def);
AlterTableCmd *cmd = makeNode(AlterTableCmd);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0b5652071d1..103021c0947 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11629,7 +11629,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'T';
+ n->subtype = AD_AlterDefault;
n->typeName = $3;
n->def = $4;
$$ = (Node *) n;
@@ -11639,7 +11639,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'N';
+ n->subtype = AD_DropNotNull;
n->typeName = $3;
$$ = (Node *) n;
}
@@ -11648,7 +11648,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'O';
+ n->subtype = AD_SetNotNull;
n->typeName = $3;
$$ = (Node *) n;
}
@@ -11657,7 +11657,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'C';
+ n->subtype = AD_AddConstraint;
n->typeName = $3;
n->def = $5;
$$ = (Node *) n;
@@ -11667,7 +11667,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'X';
+ n->subtype = AD_DropConstraint;
n->typeName = $3;
n->name = $6;
n->behavior = $7;
@@ -11679,7 +11679,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'X';
+ n->subtype = AD_DropConstraint;
n->typeName = $3;
n->name = $8;
n->behavior = $9;
@@ -11691,7 +11691,7 @@ AlterDomainStmt:
{
AlterDomainStmt *n =
makeNode(AlterDomainStmt);
- n->subtype = 'V';
+ n->subtype = AD_ValidateConstraint;
n->typeName = $3;
n->name = $6;
$$ = (Node *) n;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 25fe3d58016..aff8510755f 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1343,7 +1343,7 @@ ProcessUtilitySlow(ParseState *pstate,
*/
switch (stmt->subtype)
{
- case 'T': /* ALTER DOMAIN
DEFAULT */
+ case AD_AlterDefault:
/*
* Recursively alter
column default for table and,
@@ -1353,30 +1353,30 @@ ProcessUtilitySlow(ParseState *pstate,
AlterDomainDefault(stmt->typeName,
stmt->def);
break;
- case 'N': /* ALTER DOMAIN
DROP NOT NULL */
+ case AD_DropNotNull:
address =
AlterDomainNotNull(stmt->typeName,
false);
break;
- case 'O': /* ALTER DOMAIN
SET NOT NULL */
+ case AD_SetNotNull:
address =
AlterDomainNotNull(stmt->typeName,
true);
break;
- case 'C': /* ADD
CONSTRAINT */
+ case AD_AddConstraint:
address =
AlterDomainAddConstraint(stmt->typeName,
stmt->def,
&secondaryObject);
break;
- case 'X': /* DROP
CONSTRAINT */
+ case AD_DropConstraint:
address =
AlterDomainDropConstraint(stmt->typeName,
stmt->name,
stmt->behavior,
stmt->missing_ok);
break;
- case 'V': /* VALIDATE
CONSTRAINT */
+ case AD_ValidateConstraint:
address =
AlterDomainValidateConstraint(stmt->typeName,
stmt->name);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 4610fc61293..6eb19eeb2de 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2534,14 +2534,7 @@ typedef struct AlterCollationStmt
typedef struct AlterDomainStmt
{
NodeTag type;
- char subtype; /*------------
- * T =
alter column default
- * N =
alter column drop not null
- * O =
alter column set not null
- * C = add
constraint
- * X =
drop constraint
- *------------
- */
+ char subtype; /* see AD_xxx constants below */
List *typeName; /* domain to work on */
char *name; /* column or constraint name to
act on */
Node *def; /* definition of default or
constraint */
@@ -2549,6 +2542,13 @@ typedef struct AlterDomainStmt
bool missing_ok; /* skip error if missing? */
} AlterDomainStmt;
+/* AlterDomainStmt action */
+#define AD_AlterDefault 'T'
/* SET|DROP DEFAULT */
+#define AD_DropNotNull 'N' /* DROP
NOT NULL */
+#define AD_SetNotNull 'O' /* SET
NOT NULL */
+#define AD_AddConstraint 'C' /* ADD
CONSTRAINT */
+#define AD_DropConstraint 'X' /* DROP
CONSTRAINT */
+#define AD_ValidateConstraint 'V' /* VALIDATE
CONSTRAINT */
/* ----------------------
* Grant|Revoke Statement
