Обсуждение: DOC: fixes multiple errors in alter table doc
Hi Hacker,
While working on a patch these days, my eyes are on the “alter table” doc, and found multiple errors:
1. Several sub-commands are missed in the top “action” list:
* ALTER COLUMN SET <sequence-option>
* ALTER COLUMN RESTART
* RENAME
* SET SCHEMA
* ATTACH PARTITION
* DETACH PARTITION
* MERGE PARTITION
* SPLIT PARTITION
2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because “COLUMN” is actually optional.
3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of other sub-commands omit their prefix part.
This patch fixed all these issue.
Вложения
> On Dec 18, 2025, at 15:22, Chao Li <li.evan.chao@gmail.com> wrote: > > Hi Hacker, > > While working on a patch these days, my eyes are on the “alter table” doc, and found multiple errors: > > 1. Several sub-commands are missed in the top “action” list: > > * ALTER COLUMN SET <sequence-option> > * ALTER COLUMN RESTART > * RENAME > * SET SCHEMA > * ATTACH PARTITION > * DETACH PARTITION > * MERGE PARTITION > * SPLIT PARTITION > > 2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because“COLUMN” is actually optional. > > 3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of othersub-commands omit their prefix part. > > This patch fixed all these issue. > > <v1-0001-doc-clarify-and-complete-ALTER-TABLE-syntax-in-re.patch> CF entry created: https://commitfest.postgresql.org/patch/6328/ Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On Thu, Dec 18, 2025 at 2:22 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi Hacker,
>
> While working on a patch these days, my eyes are on the “alter table” doc, and found multiple errors:
>
> 1. Several sub-commands are missed in the top “action” list:
>
> * ALTER COLUMN SET <sequence-option>
> * ALTER COLUMN RESTART
I believe these are covered by the line above them:
ALTER [ COLUMN ] column_name { SET GENERATED { ALWAYS | BY DEFAULT
} | SET sequence_option | RESTART [ [ WITH ] restart ] } [...]
> * RENAME
This is covered by the 4th line
ALTER TABLE [ IF EXISTS ] name
RENAME TO new_name
> * SET SCHEMA
5th option...
ALTER TABLE [ IF EXISTS ] name
SET SCHEMA new_schema
> * ATTACH PARTITION
> * DETACH PARTITION
> * MERGE PARTITION
> * SPLIT PARTITION
>
And the above are the 7,8,9,10th options at the top.
> 2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading,
because“COLUMN” is actually optional.
>
Seems technically correct and potentially useful, and I see you
handled this for the DROP COLUMN variant as well, so I could see a +1
on this one.
> 3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of
othersub-commands omit their prefix part.
>
Hmm... I'm curious what you find confusing about this. Is the
confusion in trying to find or understand the information presented,
or confusing as to why it isn't all documented the same way? The
downside of your "fix" is that this introduces a lot of extra text
that is more or less noise, especially for folks trying to skim the
documents looking for very specific command references. And while I
agree that we aren't 100% consistent on this within the ALTER TABLE
subcommands, we use this same mixed pattern of omission on other pages
(see ALTER TYPE for instance). If we were to insist on making this
consistent here, I think we'd probably need to look at other pages as
well and evaluate or update them too. I'm not sure that would be an
improvement though.
Robert Treat
https://xzilla.net
On Jan 2, 2026, at 10:54, Robert Treat <rob@xzilla.net> wrote:
Hi Robert,
Thanks you very much for your review.
On Thu, Dec 18, 2025 at 2:22 AM Chao Li <li.evan.chao@gmail.com> wrote:
Hi Hacker,
While working on a patch these days, my eyes are on the “alter table” doc, and found multiple errors:
1. Several sub-commands are missed in the top “action” list:
* ALTER COLUMN SET <sequence-option>
* ALTER COLUMN RESTART
I believe these are covered by the line above them:
ALTER [ COLUMN ] column_name { SET GENERATED { ALWAYS | BY DEFAULT
} | SET sequence_option | RESTART [ [ WITH ] restart ] } [...]* RENAME
This is covered by the 4th line
ALTER TABLE [ IF EXISTS ] name
RENAME TO new_name* SET SCHEMA
5th option...
ALTER TABLE [ IF EXISTS ] name
SET SCHEMA new_schema* ATTACH PARTITION
* DETACH PARTITION
* MERGE PARTITION
* SPLIT PARTITION
And the above are the 7,8,9,10th options at the top.
That’s true. I reverted all above changes.
2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because “COLUMN” is actually optional.
Seems technically correct and potentially useful, and I see you
handled this for the DROP COLUMN variant as well, so I could see a +1
on this one.
Thanks for confirming.
3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of other sub-commands omit their prefix part.
Hmm... I'm curious what you find confusing about this. Is the
confusion in trying to find or understand the information presented,
or confusing as to why it isn't all documented the same way? The
downside of your "fix" is that this introduces a lot of extra text
that is more or less noise, especially for folks trying to skim the
documents looking for very specific command references. And while I
agree that we aren't 100% consistent on this within the ALTER TABLE
subcommands, we use this same mixed pattern of omission on other pages
(see ALTER TYPE for instance). If we were to insist on making this
consistent here, I think we'd probably need to look at other pages as
well and evaluate or update them too. I'm not sure that would be an
improvement though.
The confusion came from my own first-time reading of the documentation. Since the page is quite long, when I was reading the action descriptions and wanted to confirm the exact sub-command syntax, I often had to scroll back up to the syntax section. That led me to think it might be helpful to include the full sub-command form directly with the action descriptions.
That said, I understand your concern. The change did make the text longer and added noise. In v2, I’ve therefore reverted that broader change. As you pointed out, if we were to pursue this kind of consistency, it would need to be handled across other similar pages as well, which would be better done as a dedicated and more carefully scoped patch.
Best regards,
Вложения
On Sat, Jan 3, 2026 at 11:30 PM Chao Li <li.evan.chao@gmail.com> wrote: > On Jan 2, 2026, at 10:54, Robert Treat <rob@xzilla.net> wrote: > Hi Robert, > > Thanks you very much for your review. > > > On Thu, Dec 18, 2025 at 2:22 AM Chao Li <li.evan.chao@gmail.com> wrote: > Hi Hacker, <snip> > 2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because“COLUMN” is actually optional. > > Seems technically correct and potentially useful, and I see you > handled this for the DROP COLUMN variant as well, so I could see a +1 > on this one. > > Thanks for confirming. > > > 3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of othersub-commands omit their prefix part. > > > Hmm... I'm curious what you find confusing about this. Is the > confusion in trying to find or understand the information presented, > or confusing as to why it isn't all documented the same way? The > downside of your "fix" is that this introduces a lot of extra text > that is more or less noise, especially for folks trying to skim the > documents looking for very specific command references. And while I > agree that we aren't 100% consistent on this within the ALTER TABLE > subcommands, we use this same mixed pattern of omission on other pages > (see ALTER TYPE for instance). If we were to insist on making this > consistent here, I think we'd probably need to look at other pages as > well and evaluate or update them too. I'm not sure that would be an > improvement though. > > > The confusion came from my own first-time reading of the documentation. Since the page is quite long, when I was readingthe action descriptions and wanted to confirm the exact sub-command syntax, I often had to scroll back up to the syntaxsection. That led me to think it might be helpful to include the full sub-command form directly with the action descriptions. > > That said, I understand your concern. The change did make the text longer and added noise. In v2, I’ve therefore revertedthat broader change. As you pointed out, if we were to pursue this kind of consistency, it would need to be handledacross other similar pages as well, which would be better done as a dedicated and more carefully scoped patch. > > So, v2’s scope is significantly reduced, only a fix for my original point 2 is retained. > Makes sense to me and seems like an improvement, so +1. Robert Treat https://xzilla.net
> On Jan 8, 2026, at 07:13, Robert Treat <rob@xzilla.net> wrote: > > On Sat, Jan 3, 2026 at 11:30 PM Chao Li <li.evan.chao@gmail.com> wrote: >> On Jan 2, 2026, at 10:54, Robert Treat <rob@xzilla.net> wrote: >> Hi Robert, >> >> Thanks you very much for your review. >> >> >> On Thu, Dec 18, 2025 at 2:22 AM Chao Li <li.evan.chao@gmail.com> wrote: >> Hi Hacker, > <snip> >> 2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because“COLUMN” is actually optional. >> >> Seems technically correct and potentially useful, and I see you >> handled this for the DROP COLUMN variant as well, so I could see a +1 >> on this one. >> >> Thanks for confirming. >> >> >> 3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of othersub-commands omit their prefix part. >> >> >> Hmm... I'm curious what you find confusing about this. Is the >> confusion in trying to find or understand the information presented, >> or confusing as to why it isn't all documented the same way? The >> downside of your "fix" is that this introduces a lot of extra text >> that is more or less noise, especially for folks trying to skim the >> documents looking for very specific command references. And while I >> agree that we aren't 100% consistent on this within the ALTER TABLE >> subcommands, we use this same mixed pattern of omission on other pages >> (see ALTER TYPE for instance). If we were to insist on making this >> consistent here, I think we'd probably need to look at other pages as >> well and evaluate or update them too. I'm not sure that would be an >> improvement though. >> >> >> The confusion came from my own first-time reading of the documentation. Since the page is quite long, when I was readingthe action descriptions and wanted to confirm the exact sub-command syntax, I often had to scroll back up to the syntaxsection. That led me to think it might be helpful to include the full sub-command form directly with the action descriptions. >> >> That said, I understand your concern. The change did make the text longer and added noise. In v2, I’ve therefore revertedthat broader change. As you pointed out, if we were to pursue this kind of consistency, it would need to be handledacross other similar pages as well, which would be better done as a dedicated and more carefully scoped patch. >> >> So, v2’s scope is significantly reduced, only a fix for my original point 2 is retained. >> > > Makes sense to me and seems like an improvement, so +1. > Hi Robert, Thank you very much for your review. This is the CF entry https://commitfest.postgresql.org/patch/6328/, you may add youas a reviewer. And I just changed the status to Ready for Committer. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
> On Jan 8, 2026, at 09:38, Chao Li <li.evan.chao@gmail.com> wrote: > > > >> On Jan 8, 2026, at 07:13, Robert Treat <rob@xzilla.net> wrote: >> >> On Sat, Jan 3, 2026 at 11:30 PM Chao Li <li.evan.chao@gmail.com> wrote: >>> On Jan 2, 2026, at 10:54, Robert Treat <rob@xzilla.net> wrote: >>> Hi Robert, >>> >>> Thanks you very much for your review. >>> >>> >>> On Thu, Dec 18, 2025 at 2:22 AM Chao Li <li.evan.chao@gmail.com> wrote: >>> Hi Hacker, >> <snip> >>> 2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because“COLUMN” is actually optional. >>> >>> Seems technically correct and potentially useful, and I see you >>> handled this for the DROP COLUMN variant as well, so I could see a +1 >>> on this one. >>> >>> Thanks for confirming. >>> >>> >>> 3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of othersub-commands omit their prefix part. >>> >>> >>> Hmm... I'm curious what you find confusing about this. Is the >>> confusion in trying to find or understand the information presented, >>> or confusing as to why it isn't all documented the same way? The >>> downside of your "fix" is that this introduces a lot of extra text >>> that is more or less noise, especially for folks trying to skim the >>> documents looking for very specific command references. And while I >>> agree that we aren't 100% consistent on this within the ALTER TABLE >>> subcommands, we use this same mixed pattern of omission on other pages >>> (see ALTER TYPE for instance). If we were to insist on making this >>> consistent here, I think we'd probably need to look at other pages as >>> well and evaluate or update them too. I'm not sure that would be an >>> improvement though. >>> >>> >>> The confusion came from my own first-time reading of the documentation. Since the page is quite long, when I was readingthe action descriptions and wanted to confirm the exact sub-command syntax, I often had to scroll back up to the syntaxsection. That led me to think it might be helpful to include the full sub-command form directly with the action descriptions. >>> >>> That said, I understand your concern. The change did make the text longer and added noise. In v2, I’ve therefore revertedthat broader change. As you pointed out, if we were to pursue this kind of consistency, it would need to be handledacross other similar pages as well, which would be better done as a dedicated and more carefully scoped patch. >>> >>> So, v2’s scope is significantly reduced, only a fix for my original point 2 is retained. >>> >> >> Makes sense to me and seems like an improvement, so +1. >> > > Hi Robert, > > Thank you very much for your review. This is the CF entry https://commitfest.postgresql.org/patch/6328/, you may add youas a reviewer. And I just changed the status to Ready for Committer. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ Bump. This is a tiny doc fix. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On Thu, Jan 22, 2026 at 5:33 PM Chao Li <li.evan.chao@gmail.com> wrote:
> On Jan 8, 2026, at 09:38, Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
>> On Jan 8, 2026, at 07:13, Robert Treat <rob@xzilla.net> wrote:
>>
>> On Sat, Jan 3, 2026 at 11:30 PM Chao Li <li.evan.chao@gmail.com> wrote:
>>> On Jan 2, 2026, at 10:54, Robert Treat <rob@xzilla.net> wrote:
>>> Hi Robert,
>>>
>>> Thanks you very much for your review.
>>>
>>>
>>> On Thu, Dec 18, 2025 at 2:22 AM Chao Li <li.evan.chao@gmail.com> wrote:
>>> Hi Hacker,
>> <snip>
>>> 2. In sub-command details section, "ADD COLUMN [ IF NOT EXISTS ]” missed “[]" with “COLUMN”, which is misleading, because “COLUMN” is actually optional.
>>>
>>> Seems technically correct and potentially useful, and I see you
>>> handled this for the DROP COLUMN variant as well, so I could see a +1
>>> on this one.
>>>
>>> Thanks for confirming.
>>>
>>>
>>> 3. For all “alter column” sub-commands, "ALTER [ COLUMN ]” are omitted, which is also confusing, because none of other sub-commands omit their prefix part.
>>>
>>>
>>> Hmm... I'm curious what you find confusing about this. Is the
>>> confusion in trying to find or understand the information presented,
>>> or confusing as to why it isn't all documented the same way? The
>>> downside of your "fix" is that this introduces a lot of extra text
>>> that is more or less noise, especially for folks trying to skim the
>>> documents looking for very specific command references. And while I
>>> agree that we aren't 100% consistent on this within the ALTER TABLE
>>> subcommands, we use this same mixed pattern of omission on other pages
>>> (see ALTER TYPE for instance). If we were to insist on making this
>>> consistent here, I think we'd probably need to look at other pages as
>>> well and evaluate or update them too. I'm not sure that would be an
>>> improvement though.
>>>
>>>
>>> The confusion came from my own first-time reading of the documentation. Since the page is quite long, when I was reading the action descriptions and wanted to confirm the exact sub-command syntax, I often had to scroll back up to the syntax section. That led me to think it might be helpful to include the full sub-command form directly with the action descriptions.
>>>
>>> That said, I understand your concern. The change did make the text longer and added noise. In v2, I’ve therefore reverted that broader change. As you pointed out, if we were to pursue this kind of consistency, it would need to be handled across other similar pages as well, which would be better done as a dedicated and more carefully scoped patch.
>>>
>>> So, v2’s scope is significantly reduced, only a fix for my original point 2 is retained.
>>>
>>
>> Makes sense to me and seems like an improvement, so +1.
>>
>
> Hi Robert,
>
> Thank you very much for your review. This is the CF entry https://commitfest.postgresql.org/patch/6328/, you may add you as a reviewer. And I just changed the status to Ready for Committer.
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/
Bump. This is a tiny doc fix.
PFA v3: Rebased, and added reviewer and discussion information.
Best regards,
--
Chao Li (Evan)HighGo Software Co., Ltd.
Вложения
On Thu, Jan 22, 2026 at 6:38 PM Chao Li <li.evan.chao@gmail.com> wrote: > PFA v3: Rebased, and added reviewer and discussion information. LGTM. Should we apply the same change to the ALTER FOREIGN TABLE docs as well? While reviewing that section, I noticed that ADD COLUMN IF NOT EXISTS appears to work for ALTER FOREIGN TABLE, but it isn't documented. I'm not sure why it was left out, but perhaps we should document it and add a regression test in a separate patch? Regards, -- Fujii Masao
> On Mar 2, 2026, at 17:04, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Thu, Jan 22, 2026 at 6:38 PM Chao Li <li.evan.chao@gmail.com> wrote: >> PFA v3: Rebased, and added reviewer and discussion information. > > LGTM. > > Should we apply the same change to the ALTER FOREIGN TABLE docs as well? Sure, I can make the change. > > While reviewing that section, I noticed that ADD COLUMN IF NOT EXISTS appears > to work for ALTER FOREIGN TABLE, but it isn't documented. I'm not sure why > it was left out, but perhaps we should document it and add a regression test in > a separate patch? > I will verify that, then update the doc and regression test accordingly. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
> On Mar 2, 2026, at 17:22, Chao Li <li.evan.chao@gmail.com> wrote: > > > >> On Mar 2, 2026, at 17:04, Fujii Masao <masao.fujii@gmail.com> wrote: >> >> On Thu, Jan 22, 2026 at 6:38 PM Chao Li <li.evan.chao@gmail.com> wrote: >>> PFA v3: Rebased, and added reviewer and discussion information. >> >> LGTM. >> >> Should we apply the same change to the ALTER FOREIGN TABLE docs as well? > > Sure, I can make the change. > >> >> While reviewing that section, I noticed that ADD COLUMN IF NOT EXISTS appears >> to work for ALTER FOREIGN TABLE, but it isn't documented. I'm not sure why >> it was left out, but perhaps we should document it and add a regression test in >> a separate patch? >> > > I will verify that, then update the doc and regression test accordingly. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > PFA v4: 0001 - alter table related changes * Add test cases for omitting COLUMN of ALTER TABLE ADD/DROP [COLUMN] * Add a test case for ADD COLUMN IF NOT EXIST (already had DROP COLUMN IF EXIST) 0002 - alter foreign table related changes * doc: Add IF NOT EXSTS for ADD COLUMN * doc: Mark COLUMN as optional for ADD/DROP COLUMN in the same way as 0001 * Add test cases for omitting COLUMN of ALTER FOREIGN TABLE ADD/DROP [COLUMN] * Add a test cases for for ADD COLUMN IF NOT EXIST * Add a test cases for for DROP COLUMN IF EXIST Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Вложения
On Mon, Mar 2, 2026 at 9:09 PM Chao Li <li.evan.chao@gmail.com> wrote: > > On Mar 2, 2026, at 17:22, Chao Li <li.evan.chao@gmail.com> wrote: > >> On Mar 2, 2026, at 17:04, Fujii Masao <masao.fujii@gmail.com> wrote: > >> On Thu, Jan 22, 2026 at 6:38 PM Chao Li <li.evan.chao@gmail.com> wrote: > >>> PFA v3: Rebased, and added reviewer and discussion information. > >> > >> LGTM. > >> > >> Should we apply the same change to the ALTER FOREIGN TABLE docs as well? > > > > Sure, I can make the change. > > > >> > >> While reviewing that section, I noticed that ADD COLUMN IF NOT EXISTS appears > >> to work for ALTER FOREIGN TABLE, but it isn't documented. I'm not sure why > >> it was left out, but perhaps we should document it and add a regression test in > >> a separate patch? > >> > > > > I will verify that, then update the doc and regression test accordingly. > > > > Best regards, > > -- > > Chao Li (Evan) > > HighGo Software Co., Ltd. > > https://www.highgo.com/ > > > > PFA v4: > > 0001 - alter table related changes > > * Add test cases for omitting COLUMN of ALTER TABLE ADD/DROP [COLUMN] > * Add a test case for ADD COLUMN IF NOT EXIST (already had DROP COLUMN IF EXIST) > > 0002 - alter foreign table related changes > > * doc: Add IF NOT EXSTS for ADD COLUMN > * doc: Mark COLUMN as optional for ADD/DROP COLUMN in the same way as 0001 > * Add test cases for omitting COLUMN of ALTER FOREIGN TABLE ADD/DROP [COLUMN] > * Add a test cases for for ADD COLUMN IF NOT EXIST > * Add a test cases for for DROP COLUMN IF EXIST > LGTM, although I was curious why you went with c12 vs c11 in +ALTER FOREIGN TABLE ft1 ADD c12 integer; -- omit COLUMN or maybe that should be changed? Robert Treat https://xzilla.net
> On Mar 3, 2026, at 12:41, Robert Treat <rob@xzilla.net> wrote: > > On Mon, Mar 2, 2026 at 9:09 PM Chao Li <li.evan.chao@gmail.com> wrote: >>> On Mar 2, 2026, at 17:22, Chao Li <li.evan.chao@gmail.com> wrote: >>>> On Mar 2, 2026, at 17:04, Fujii Masao <masao.fujii@gmail.com> wrote: >>>> On Thu, Jan 22, 2026 at 6:38 PM Chao Li <li.evan.chao@gmail.com> wrote: >>>>> PFA v3: Rebased, and added reviewer and discussion information. >>>> >>>> LGTM. >>>> >>>> Should we apply the same change to the ALTER FOREIGN TABLE docs as well? >>> >>> Sure, I can make the change. >>> >>>> >>>> While reviewing that section, I noticed that ADD COLUMN IF NOT EXISTS appears >>>> to work for ALTER FOREIGN TABLE, but it isn't documented. I'm not sure why >>>> it was left out, but perhaps we should document it and add a regression test in >>>> a separate patch? >>>> >>> >>> I will verify that, then update the doc and regression test accordingly. >>> >>> Best regards, >>> -- >>> Chao Li (Evan) >>> HighGo Software Co., Ltd. >>> https://www.highgo.com/ >>> >> >> PFA v4: >> >> 0001 - alter table related changes >> >> * Add test cases for omitting COLUMN of ALTER TABLE ADD/DROP [COLUMN] >> * Add a test case for ADD COLUMN IF NOT EXIST (already had DROP COLUMN IF EXIST) >> >> 0002 - alter foreign table related changes >> >> * doc: Add IF NOT EXSTS for ADD COLUMN >> * doc: Mark COLUMN as optional for ADD/DROP COLUMN in the same way as 0001 >> * Add test cases for omitting COLUMN of ALTER FOREIGN TABLE ADD/DROP [COLUMN] >> * Add a test cases for for ADD COLUMN IF NOT EXIST >> * Add a test cases for for DROP COLUMN IF EXIST >> > > LGTM, Thanks for confirming. > although I was curious why you went with c12 vs c11 in > +ALTER FOREIGN TABLE ft1 ADD c12 integer; -- omit COLUMN > or maybe that should be changed? > Your eagle eyes! I originally added both c11 and c12 test cases. After some tuning, I removed the c11 case, but forgot torename c12 to c11 accordingly. Thanks for catching that. PFA v5: * In 0002, renamed c12 to c11 in the ALTER FOREIGN TABLE tests. * In 0002, added one more test case: ALTER FOREIGN TABLE ft1 DROP IF EXISTS no_column; Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Вложения
On Tue, Mar 3, 2026 at 5:05 PM Chao Li <li.evan.chao@gmail.com> wrote: > PFA v5: > > * In 0002, renamed c12 to c11 in the ALTER FOREIGN TABLE tests. > * In 0002, added one more test case: ALTER FOREIGN TABLE ft1 DROP IF EXISTS no_column; Thanks for updating the patches! I've committed the 0001 patch and backpatched it to all supported versions. -- alter noexisting table ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD COLUMN c4 integer; +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD c5 integer; <snip> ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP COLUMN IF EXISTS no_column; +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP IF EXISTS no_column; Regarding 0002 patch, could you explain the reason for adding these two additional tests? I was just curious why because other four tests that the patch adds seem sufficient. Aside from that point, the patch looks good to me. Regards, -- Fujii Masao
> On Mar 5, 2026, at 12:42, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Tue, Mar 3, 2026 at 5:05 PM Chao Li <li.evan.chao@gmail.com> wrote: >> PFA v5: >> >> * In 0002, renamed c12 to c11 in the ALTER FOREIGN TABLE tests. >> * In 0002, added one more test case: ALTER FOREIGN TABLE ft1 DROP IF EXISTS no_column; > > Thanks for updating the patches! > > I've committed the 0001 patch and backpatched it to all supported versions. > > > -- alter noexisting table > ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD COLUMN c4 integer; > +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD c5 integer; > <snip> > ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP COLUMN IF EXISTS no_column; > +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP IF EXISTS no_column; > > Regarding 0002 patch, could you explain the reason for adding these > two additional tests? I was just curious why because other four tests that > the patch adds seem sufficient. They are combinations of “IF [NOT] EXISTS” and omitting “COLUMN", I added them just for better coverage. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
On Thu, Mar 5, 2026 at 2:06 PM Chao Li <li.evan.chao@gmail.com> wrote: > > -- alter noexisting table > > ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD COLUMN c4 integer; > > +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD c5 integer; > > <snip> > > ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP COLUMN IF EXISTS no_column; > > +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP IF EXISTS no_column; > > > > Regarding 0002 patch, could you explain the reason for adding these > > two additional tests? I was just curious why because other four tests that > > the patch adds seem sufficient. > > They are combinations of “IF [NOT] EXISTS” and omitting “COLUMN", I added them just for better coverage. Which table-level or column-level "IF [NOT] EXISTS" are you referring to? If you mean the combination of column-level "IF [NOT] EXISTS" with "COLUMN" omitted, then the first test above should be "ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD IF NOT EXISTS c5 integer"? Regards, -- Fujii Masao
> On Mar 6, 2026, at 14:40, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Thu, Mar 5, 2026 at 2:06 PM Chao Li <li.evan.chao@gmail.com> wrote: >>> -- alter noexisting table >>> ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD COLUMN c4 integer; >>> +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD c5 integer; >>> <snip> >>> ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP COLUMN IF EXISTS no_column; >>> +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP IF EXISTS no_column; >>> >>> Regarding 0002 patch, could you explain the reason for adding these >>> two additional tests? I was just curious why because other four tests that >>> the patch adds seem sufficient. >> >> They are combinations of “IF [NOT] EXISTS” and omitting “COLUMN", I added them just for better coverage. > > Which table-level or column-level "IF [NOT] EXISTS" are you referring to? > If you mean the combination of column-level "IF [NOT] EXISTS" with > "COLUMN" omitted, then the first test above should be "ALTER FOREIGN TABLE > IF EXISTS doesnt_exist_ft1 ADD IF NOT EXISTS c5 integer"? > > Regards, > > -- > Fujii Masao I meant to say column-level. Yes, the first test seems not needed. My brain was buffering! I will revisit the test casesand make an update later. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
> On Mar 6, 2026, at 14:48, Chao Li <li.evan.chao@gmail.com> wrote: > > > >> On Mar 6, 2026, at 14:40, Fujii Masao <masao.fujii@gmail.com> wrote: >> >> On Thu, Mar 5, 2026 at 2:06 PM Chao Li <li.evan.chao@gmail.com> wrote: >>>> -- alter noexisting table >>>> ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD COLUMN c4 integer; >>>> +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 ADD c5 integer; >>>> <snip> >>>> ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP COLUMN IF EXISTS no_column; >>>> +ALTER FOREIGN TABLE IF EXISTS doesnt_exist_ft1 DROP IF EXISTS no_column; >>>> >>>> Regarding 0002 patch, could you explain the reason for adding these >>>> two additional tests? I was just curious why because other four tests that >>>> the patch adds seem sufficient. >>> >>> They are combinations of “IF [NOT] EXISTS” and omitting “COLUMN", I added them just for better coverage. >> >> Which table-level or column-level "IF [NOT] EXISTS" are you referring to? >> If you mean the combination of column-level "IF [NOT] EXISTS" with >> "COLUMN" omitted, then the first test above should be "ALTER FOREIGN TABLE >> IF EXISTS doesnt_exist_ft1 ADD IF NOT EXISTS c5 integer"? >> >> Regards, >> >> -- >> Fujii Masao > > I meant to say column-level. Yes, the first test seems not needed. My brain was buffering! I will revisit the test casesand make an update later. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > PFA v6. The two test cases on table level “IF EXISTS” are removed. So, now the patch adds totally 4 test cases: 2 for omittingCOLUMN, 2 for combinations of column level “IF [NOT] EXISTS” and omitting COUMN. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Вложения
On Fri, Mar 6, 2026 at 4:53 PM Chao Li <li.evan.chao@gmail.com> wrote: > PFA v6. Thanks for updating the patch! > The two test cases on table level “IF EXISTS” are removed. So, now the patch adds totally 4 test cases: 2 for omittingCOLUMN, 2 for combinations of column level “IF [NOT] EXISTS” and omitting COUMN. The latest patch doesn't seem to include the combination of column-level IF NOT EXISTS for ADD COLUMN and omitting COLUMN. Based on your patch, I updated the regression tests as follows and pushed the changes: +ALTER FOREIGN TABLE ft1 ADD c11 integer; -- omit COLUMN +ALTER FOREIGN TABLE ft1 DROP c11; -- omit COLUMN I moved the ALTER FOREIGN TABLE DROP test into the section that lists the other existing ALTER FOREIGN TABLE DROP COLUMN tests. Regarding table-level IF EXISTS tests, it seemed to me that most ALTER FOREIGN TABLE ADD/DROP variants should also be tested there, following the existing structure in foreign_data.sql. So I added some additional cases, for example ALTER FOREIGN TABLE IF EXISTS ADD COLUMN IF NOT EXISTS. Regards, -- Fujii Masao
> On Mar 9, 2026, at 17:43, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Fri, Mar 6, 2026 at 4:53 PM Chao Li <li.evan.chao@gmail.com> wrote: >> PFA v6. > > Thanks for updating the patch! > >> The two test cases on table level “IF EXISTS” are removed. So, now the patch adds totally 4 test cases: 2 for omittingCOLUMN, 2 for combinations of column level “IF [NOT] EXISTS” and omitting COUMN. > > The latest patch doesn't seem to include the combination of column-level > IF NOT EXISTS for ADD COLUMN and omitting COLUMN. > > > Based on your patch, I updated the regression tests as follows and > pushed the changes: > > +ALTER FOREIGN TABLE ft1 ADD c11 integer; -- omit COLUMN > +ALTER FOREIGN TABLE ft1 DROP c11; -- omit COLUMN > > I moved the ALTER FOREIGN TABLE DROP test into the section that lists the other > existing ALTER FOREIGN TABLE DROP COLUMN tests. > > Regarding table-level IF EXISTS tests, it seemed to me that most > ALTER FOREIGN TABLE ADD/DROP variants should also be tested there, > following the existing structure in foreign_data.sql. So I added some > additional cases, for example ALTER FOREIGN TABLE IF EXISTS ADD COLUMN IF > NOT EXISTS. > > Regards, > > -- > Fujii Masao Thank you so much for taking care of that. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/