Re: Pgoutput not capturing the generated columns
Peter Smith <smithpb2250@gmail.com>
Attachments
- PS_NITPICKS_20240624_v90001.txt (text/plain)
Hi, here are some patch v9-0001 comments. I saw Kuroda-san has already posted comments for this patch so there may be some duplication here. ====== GENERAL 1. The later patches 0002 etc are checking to support only STORED gencols. But, doesn't that restriction belong in this patch 0001 so VIRTUAL columns are not decoded etc in the first place... (??) ~~~ 2. The "Generated Columns" docs mentioned in my previous review comment [2] should be modified by this 0001 patch. ~~~ 3. I think the "Message Format" page mentioned in my previous review comment [3] should be modified by this 0001 patch. ====== Commit message 4. The patch name is still broken as previously mentioned [1, #1] ====== doc/src/sgml/protocol.sgml 5. Should this docs be referring to STORED generated columns, instead of just generated columns? ====== doc/src/sgml/ref/create_subscription.sgml 6. Should this be docs referring to STORED generated columns, instead of just generated columns? ====== src/bin/pg_dump/pg_dump.c getSubscriptions: NITPICK - tabs NITPICK - patch removed a blank line it should not be touching NITPICK = patch altered indents it should not be touching NITPICK - a missing blank line that was previously present 7. + else + appendPQExpBufferStr(query, + " false AS subincludegencols,\n"); There is an unwanted comma here. ~ dumpSubscription NITPICK - patch altered indents it should not be touching ====== src/bin/pg_dump/pg_dump.h NITPICK - unnecessary blank line ====== src/bin/psql/describe.c describeSubscriptions NITPICK - bad indentation 8. In my previous review [1, #4b] I suggested this new column should be in a different order (e.g. adjacent to the other ones ahead of 'Conninfo'), but this is not yet addressed. ====== src/test/subscription/t/011_generated.pl NITPICK - missing space in comment NITPICK - misleading "because" wording in the comment ====== 99. See also my attached nitpicks diff, for cosmetic issues. Please apply whatever you agree with. ====== [1] My v8-0001 review - https://www.postgresql.org/message-id/CAHut%2BPujrRQ63ju8P41tBkdjkQb4X9uEdLK_Wkauxum1MVUdfA%40mail.gmail.com [2] https://www.postgresql.org/message-id/CAHut%2BPvsRWq9t2tEErt5ZWZCVpNFVZjfZ_owqfdjOhh4yXb_3Q%40mail.gmail.com [3] https://www.postgresql.org/message-id/CAHut%2BPsHsT3V1wQ5uoH9ynbmWn4ZQqOe34X%2Bg37LSi7sgE_i2g%40mail.gmail.com Kind Regards, Peter Smith. Fujitsu Australia.
Commits
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Doc: Generated column replication.
- 6252b1eaf82b 18.0 landed
-
Rename pubgencols_type to pubgencols in pg_publication.
- 75eb9766ec20 18.0 landed
-
Doc: Fix column name in pg_publication catalog.
- 117f9f328e0f 18.0 landed
-
Fix buildfarm failure introduced by commit e65dbc9927.
- b35434b134b1 18.0 landed
-
Change publication's publish_generated_columns option type to enum.
- e65dbc9927fb 18.0 landed
-
Fix \dRp+ output when describing publications with a lower server version.
- 991974bb4888 18.0 landed
-
Replicate generated columns when 'publish_generated_columns' is set.
- 7054186c4ebe 18.0 landed
-
Doc: Update the behavior of generated columns in Logical Replication.
- 5b0c46ea0932 18.0 landed
-
Replicate generated columns when specified in the column list.
- 745217a051a9 18.0 landed
-
Doc: Generated columns are skipped for logical replication.
- 7a089f6e6a14 17.0 cited