Standardize the definition of the subtype field of AlterDomainStmt

Lists: pgsql-hackers
From: Quan Zongliang <quanzongliang(at)yeah(dot)net>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-27 03:06:46
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: 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

Attachment Content-Type Size
alterdomain_subtype.patch text/plain 5.1 KB

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Quan Zongliang <quanzongliang(at)yeah(dot)net>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-27 03:54:39
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: 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


From: wenhui qiu <qiuwenhuifx(at)gmail(dot)com>
To:
Cc: Quan Zongliang <quanzongliang(at)yeah(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-27 09:44:41
Message-ID: CAGjGUAK5vhoAe_XsHjsvJfsXG0Bvey5QS4cZR9M-fVBC9fw1xA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: 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(at)paquier(dot)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
>


From: Quan Zongliang <quanzongliang(at)yeah(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-28 02:26:57
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: 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

Attachment Content-Type Size
alterdomain_subtype.patch text/plain 5.1 KB

From: wenhui qiu <qiuwenhuifx(at)gmail(dot)com>
To: Quan Zongliang <quanzongliang(at)yeah(dot)net>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-28 02:30:02
Message-ID: CAGjGUAKndKuiE3ds-Nq9ir0oTwb+5KxSR=SGy-JMk7SPnv5T+w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: 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(at)yeah(dot)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
>


From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Quan Zongliang <quanzongliang(at)yeah(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-28 11:22:51
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: 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.


From: Tender Wang <tndrwang(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: Quan Zongliang <quanzongliang(at)yeah(dot)net>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-28 11:30:09
Message-ID: CAHewXN=PiSgK9i-pf1JbAZwiPd_RW4nHW=QQQ1thb0MieRyWcg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Peter Eisentraut <peter(at)eisentraut(dot)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


From: Quan Zongliang <quanzongliang(at)yeah(dot)net>
To: Tender Wang <tndrwang(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-05-28 22:09:00
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

Updated

On 2025/5/28 19:30, Tender Wang wrote:
>
>
> Peter Eisentraut <peter(at)eisentraut(dot)org <mailto:peter(at)eisentraut(dot)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

Attachment Content-Type Size
alterdomain_subtype-v2.patch text/plain 5.0 KB

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Quan Zongliang <quanzongliang(at)yeah(dot)net>
Cc: Tender Wang <tndrwang(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Standardize the definition of the subtype field of AlterDomainStmt
Date: 2025-07-03 07:37:04
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, May 29, 2025 at 06:09:00AM +0800, Quan Zongliang wrote:
> Updated

Applied, with a fixed indentation.
--
Michael