pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedStandardize the definition of the subtype field of AlterDomainStmt
9+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2025-07-03 8ec04c8577 Refactor subtype field of AlterDomainStmt
* Standardize the definition of the subtype field of AlterDomainStmt
@ 2025-05-27 03:06 Quan Zongliang <quanzongliang@yeah.net>
2025-05-27 03:54 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Michael Paquier <michael@paquier.xyz>
2025-05-28 11:22 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Peter Eisentraut <peter@eisentraut.org>
0 siblings, 2 replies; 9+ messages in thread
From: Quan Zongliang @ 2025-05-27 03:06 UTC (permalink / raw)
To: pgsql-hackers
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.
--
Quan Zongliang
Attachments:
alterdomain_subtype.patch (text/plain, 5.1K) download
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..391e70b594c 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_VaidateConstraint;
n->typeName = $3;
n->name = $6;
$$ = (Node *) n;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 25fe3d58016..5c0a4eb53cb 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_VaidateConstraint:
address =
AlterDomainValidateConstraint(stmt->typeName,
stmt->name);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 4610fc61293..7349918e35e 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_VaidateConstraint 'V' /* VALIDATE CONSTRAINT */
/* ----------------------
* Grant|Revoke Statement
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
@ 2025-05-27 03:54 ` Michael Paquier <michael@paquier.xyz>
2025-05-27 09:44 ` Re: Standardize the definition of the subtype field of AlterDomainStmt wenhui qiu <qiuwenhuifx@gmail.com>
2025-05-28 02:26 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
1 sibling, 2 replies; 9+ messages in thread
From: Michael Paquier @ 2025-05-27 03:54 UTC (permalink / raw)
To: Quan Zongliang <quanzongliang@yeah.net>; +Cc: pgsql-hackers
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 */
s/Vaidate/Validate
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-27 03:54 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Michael Paquier <michael@paquier.xyz>
@ 2025-05-27 09:44 ` wenhui qiu <qiuwenhuifx@gmail.com>
1 sibling, 0 replies; 9+ messages in thread
From: wenhui qiu @ 2025-05-27 09:44 UTC (permalink / raw)
To: ; +Cc: Quan Zongliang <quanzongliang@yeah.net>; pgsql-hackers
HI
> 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.
Agree ,This makes the code neater and easier to understand
On Tue, May 27, 2025 at 11:55 AM Michael Paquier <michael@paquier.xyz>
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 */
>
> s/Vaidate/Validate
> --
> Michael
>
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-27 03:54 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Michael Paquier <michael@paquier.xyz>
@ 2025-05-28 02:26 ` Quan Zongliang <quanzongliang@yeah.net>
2025-05-28 02:30 ` Re: Standardize the definition of the subtype field of AlterDomainStmt wenhui qiu <qiuwenhuifx@gmail.com>
1 sibling, 1 reply; 9+ messages in thread
From: Quan Zongliang @ 2025-05-28 02:26 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-hackers
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) download
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
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-27 03:54 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Michael Paquier <michael@paquier.xyz>
2025-05-28 02:26 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
@ 2025-05-28 02:30 ` wenhui qiu <qiuwenhuifx@gmail.com>
0 siblings, 0 replies; 9+ messages in thread
From: wenhui qiu @ 2025-05-28 02:30 UTC (permalink / raw)
To: Quan Zongliang <quanzongliang@yeah.net>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-hackers
HI
Thank you for your update ，I marked the path as "Ready for Committer"
Thank
On Wed, May 28, 2025 at 10:27 AM Quan Zongliang <quanzongliang@yeah.net>
wrote:
>
>
> 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
>
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
@ 2025-05-28 11:22 ` Peter Eisentraut <peter@eisentraut.org>
2025-05-28 11:30 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Tender Wang <tndrwang@gmail.com>
1 sibling, 1 reply; 9+ messages in thread
From: Peter Eisentraut @ 2025-05-28 11:22 UTC (permalink / raw)
To: Quan Zongliang <quanzongliang@yeah.net>; pgsql-hackers
On 27.05.25 05:06, 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.
You can still make it an enum and assign the currently in use values to
the new symbols, like
enum AlterDomainType
{
AD_AlterDefault = 'T',
AD_DropNotNull = 'N',
...
I would prefer that.
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-28 11:22 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Peter Eisentraut <peter@eisentraut.org>
@ 2025-05-28 11:30 ` Tender Wang <tndrwang@gmail.com>
2025-05-28 22:09 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
0 siblings, 1 reply; 9+ messages in thread
From: Tender Wang @ 2025-05-28 11:30 UTC (permalink / raw)
To: Peter Eisentraut <peter@eisentraut.org>; +Cc: Quan Zongliang <quanzongliang@yeah.net>; pgsql-hackers
Peter Eisentraut <peter@eisentraut.org> 于2025年5月28日周三 19:23写道：
> On 27.05.25 05:06, 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.
>
> You can still make it an enum and assign the currently in use values to
> the new symbols, like
>
> enum AlterDomainType
> {
> AD_AlterDefault = 'T',
> AD_DropNotNull = 'N',
> ...
>
> I would prefer that.
+1
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-28 11:22 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Peter Eisentraut <peter@eisentraut.org>
2025-05-28 11:30 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Tender Wang <tndrwang@gmail.com>
@ 2025-05-28 22:09 ` Quan Zongliang <quanzongliang@yeah.net>
2025-07-03 07:37 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 9+ messages in thread
From: Quan Zongliang @ 2025-05-28 22:09 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; Peter Eisentraut <peter@eisentraut.org>; +Cc: pgsql-hackers
Updated
On 2025/5/28 19:30, Tender Wang wrote:
>
>
> Peter Eisentraut <peter@eisentraut.org <mailto:peter@eisentraut.org>> 于
> 2025年5月28日周三 19:23写道：
>
> On 27.05.25 05:06, 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.
>
> You can still make it an enum and assign the currently in use values to
> the new symbols, like
>
> enum AlterDomainType
> {
> AD_AlterDefault = 'T',
> AD_DropNotNull = 'N',
> ...
>
> I would prefer that.
>
> +1
>
> --
> Thanks,
> Tender Wang
Attachments:
alterdomain_subtype-v2.patch (text/plain, 5.0K) download
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..27f54e933ca 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2531,17 +2531,20 @@ typedef struct AlterCollationStmt
* this command.
* ----------------------
*/
+typedef enum AlterDomainType
+{
+ AD_AlterDefault = 'T', /* SET|DROP DEFAULT */
+ AD_DropNotNull = 'N', /* DROP NOT NULL */
+ AD_SetNotNull = 'O', /* SET NOT NULL */
+ AD_AddConstraint = 'C', /* ADD CONSTRAINT */
+ AD_DropConstraint = 'X', /* DROP CONSTRAINT */
+ AD_ValidateConstraint = 'V', /* VALIDATE CONSTRAINT */
+} AlterDomainType;
+
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
- *------------
- */
+ AlterDomainType subtype; /* Type of domain alteration to apply */
List *typeName; /* domain to work on */
char *name; /* column or constraint name to act on */
Node *def; /* definition of default or constraint */
* Re: Standardize the definition of the subtype field of AlterDomainStmt
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-28 11:22 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Peter Eisentraut <peter@eisentraut.org>
2025-05-28 11:30 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Tender Wang <tndrwang@gmail.com>
2025-05-28 22:09 ` Re: Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
@ 2025-07-03 07:37 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 9+ messages in thread
From: Michael Paquier @ 2025-07-03 07:37 UTC (permalink / raw)
To: Quan Zongliang <quanzongliang@yeah.net>; +Cc: Tender Wang <tndrwang@gmail.com>; Peter Eisentraut <peter@eisentraut.org>; pgsql-hackers
On Thu, May 29, 2025 at 06:09:00AM +0800, Quan Zongliang wrote:
> Updated
Applied, with a fixed indentation.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2025-07-03 07:37 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-27 03:06 Standardize the definition of the subtype field of AlterDomainStmt Quan Zongliang <quanzongliang@yeah.net>
2025-05-27 03:54 ` Michael Paquier <michael@paquier.xyz>
2025-05-27 09:44 ` wenhui qiu <qiuwenhuifx@gmail.com>
2025-05-28 02:26 ` Quan Zongliang <quanzongliang@yeah.net>
2025-05-28 02:30 ` wenhui qiu <qiuwenhuifx@gmail.com>
2025-05-28 11:22 ` Peter Eisentraut <peter@eisentraut.org>
2025-05-28 11:30 ` Tender Wang <tndrwang@gmail.com>
2025-05-28 22:09 ` Quan Zongliang <quanzongliang@yeah.net>
2025-07-03 07:37 ` Michael Paquier <michael@paquier.xyz>
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