public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 02/18] Update comment obsolete since 69c3936a 6+ messages / 3 participants [nested] [flat]
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: DOC: fixes multiple errors in alter table doc @ 2026-01-02 02:54 Robert Treat <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Robert Treat @ 2026-01-02 02:54 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]> On Thu, Dec 18, 2025 at 2:22 AM Chao Li <[email protected]> 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 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. Robert Treat https://xzilla.net ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: DOC: fixes multiple errors in alter table doc @ 2026-01-04 04:30 Chao Li <[email protected]> parent: Robert Treat <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Chao Li @ 2026-01-04 04:30 UTC (permalink / raw) To: Robert Treat <[email protected]>; +Cc: Postgres hackers <[email protected]> On Jan 2, 2026, at 10:54, Robert Treat <[email protected]> wrote: Hi Robert, Thanks you very much for your review. On Thu, Dec 18, 2025 at 2:22 AM Chao Li <[email protected]> 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. So, v2’s scope is significantly reduced, only a fix for my original point 2 is retained. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ Attachments: [application/octet-stream] v2-0001-docs-reflect-optional-COLUMN-keyword-in-ALTER-TAB.patch (1.6K, ../../CAEoWx2kkh5kCZwYOYaOVShjh3un2mJTGrcFZyhJXd320PHYfJw@mail.gmail.com/3-v2-0001-docs-reflect-optional-COLUMN-keyword-in-ALTER-TAB.patch) download | inline diff: From 16b38fd753af4654ff6399938f6dcc11dbfb625d Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" <[email protected]> Date: Thu, 18 Dec 2025 15:18:22 +0800 Subject: [PATCH v2] docs: reflect optional COLUMN keyword in ALTER TABLE descriptions The ALTER TABLE description entries for ADD COLUMN and DROP COLUMN omitted the fact that the COLUMN keyword is optional. Adjust the command summaries to match the documented syntax by marking COLUMN as optional. This is a documentation-only change; no behavior is altered. Author: Chao Li <[email protected]> --- doc/src/sgml/ref/alter_table.sgml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 1bd479c917a..d69e5de8451 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -163,7 +163,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM <variablelist> <varlistentry id="sql-altertable-desc-add-column"> - <term><literal>ADD COLUMN [ IF NOT EXISTS ]</literal></term> + <term><literal>ADD [ COLUMN ] [ IF NOT EXISTS ]</literal></term> <listitem> <para> This form adds a new column to the table, using the same syntax as @@ -175,7 +175,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM </varlistentry> <varlistentry id="sql-altertable-desc-drop-column"> - <term><literal>DROP COLUMN [ IF EXISTS ]</literal></term> + <term><literal>DROP [ COLUMN ] [ IF EXISTS ]</literal></term> <listitem> <para> This form drops a column from a table. Indexes and -- 2.39.5 (Apple Git-154) ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: DOC: fixes multiple errors in alter table doc @ 2026-01-07 23:13 Robert Treat <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Robert Treat @ 2026-01-07 23:13 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]> On Sat, Jan 3, 2026 at 11:30 PM Chao Li <[email protected]> wrote: > On Jan 2, 2026, at 10:54, Robert Treat <[email protected]> wrote: > Hi Robert, > > Thanks you very much for your review. > > > On Thu, Dec 18, 2025 at 2:22 AM Chao Li <[email protected]> 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. Robert Treat https://xzilla.net ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: DOC: fixes multiple errors in alter table doc @ 2026-01-08 01:38 Chao Li <[email protected]> parent: Robert Treat <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Chao Li @ 2026-01-08 01:38 UTC (permalink / raw) To: Robert Treat <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Jan 8, 2026, at 07:13, Robert Treat <[email protected]> wrote: > > On Sat, Jan 3, 2026 at 11:30 PM Chao Li <[email protected]> wrote: >> On Jan 2, 2026, at 10:54, Robert Treat <[email protected]> wrote: >> Hi Robert, >> >> Thanks you very much for your review. >> >> >> On Thu, Dec 18, 2025 at 2:22 AM Chao Li <[email protected]> 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/ ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: DOC: fixes multiple errors in alter table doc @ 2026-01-22 09:32 Chao Li <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Chao Li @ 2026-01-22 09:32 UTC (permalink / raw) To: Robert Treat <[email protected]>; +Cc: Postgres hackers <[email protected]> > On Jan 8, 2026, at 09:38, Chao Li <[email protected]> wrote: > > > >> On Jan 8, 2026, at 07:13, Robert Treat <[email protected]> wrote: >> >> On Sat, Jan 3, 2026 at 11:30 PM Chao Li <[email protected]> wrote: >>> On Jan 2, 2026, at 10:54, Robert Treat <[email protected]> wrote: >>> Hi Robert, >>> >>> Thanks you very much for your review. >>> >>> >>> On Thu, Dec 18, 2025 at 2:22 AM Chao Li <[email protected]> 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. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-01-22 09:32 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2026-01-02 02:54 Re: DOC: fixes multiple errors in alter table doc Robert Treat <[email protected]> 2026-01-04 04:30 ` Re: DOC: fixes multiple errors in alter table doc Chao Li <[email protected]> 2026-01-07 23:13 ` Re: DOC: fixes multiple errors in alter table doc Robert Treat <[email protected]> 2026-01-08 01:38 ` Re: DOC: fixes multiple errors in alter table doc Chao Li <[email protected]> 2026-01-22 09:32 ` Re: DOC: fixes multiple errors in alter table doc Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox