From: Quan Zongliang <quanzongliang@yeah.net>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: Wed, 28 May 2025 10:26:57 +0800
Message-ID: <483ea517-4812-47b8-856b-3f7d0d15ed4e@yeah.net> (raw)
In-Reply-To: <aDU3fwLsDCRp3q0h@paquier.xyz>
References: <41ff310b-16bd-44b9-a3ef-97e20f14b709@yeah.net>
<aDU3fwLsDCRp3q0h@paquier.xyz>
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
Attachments:alterdomain_subtype.patch (text/plain, 5.1K) downloaddiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 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.yindex 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.cindex 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.hindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: quanzongliang@yeah.net, michael@paquier.xyz
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
In-Reply-To: <483ea517-4812-47b8-856b-3f7d0d15ed4e@yeah.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox