Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Refactor subtype field of AlterDomainStmt

  1. Standardize the definition of the subtype field of AlterDomainStmt

    Quan Zongliang <quanzongliang@yeah.net> — 2025-05-27T03:06:46Z

    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

  2. Re: Standardize the definition of the subtype field of AlterDomainStmt

    Michael Paquier <michael@paquier.xyz> — 2025-05-27T03:54:39Z

    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

  3. Re: Standardize the definition of the subtype field of AlterDomainStmt

    wenhui qiu <qiuwenhuifx@gmail.com> — 2025-05-27T09:44:41Z

    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
>

  4. Re: Standardize the definition of the subtype field of AlterDomainStmt

    Quan Zongliang <quanzongliang@yeah.net> — 2025-05-28T02:26:57Z

    
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

  5. Re: Standardize the definition of the subtype field of AlterDomainStmt

    wenhui qiu <qiuwenhuifx@gmail.com> — 2025-05-28T02:30:02Z

    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
>

  6. Re: Standardize the definition of the subtype field of AlterDomainStmt

    Peter Eisentraut <peter@eisentraut.org> — 2025-05-28T11:22:51Z

    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.

  7. Re: Standardize the definition of the subtype field of AlterDomainStmt

    Tender Wang <tndrwang@gmail.com> — 2025-05-28T11:30:09Z

    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

  8. Re: Standardize the definition of the subtype field of AlterDomainStmt

    Quan Zongliang <quanzongliang@yeah.net> — 2025-05-28T22:09:00Z

    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

  9. Re: Standardize the definition of the subtype field of AlterDomainStmt

    Michael Paquier <michael@paquier.xyz> — 2025-07-03T07:37:04Z

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

Applied, with a fixed indentation.
--
Michael