Re: Pgoutput not capturing the generated columns
От | Peter Smith |
---|---|
Тема | Re: Pgoutput not capturing the generated columns |
Дата | |
Msg-id | CAHut+PvQ8CLq-JysTTeRj4u5SC9vTVcx3AgwTHcPUEOh-UnKcQ@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Pgoutput not capturing the generated columns (Shubham Khanna <khannashubham1197@gmail.com>) |
Ответы |
Re: Pgoutput not capturing the generated columns
|
Список | pgsql-hackers |
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.
Вложения
В списке pgsql-hackers по дате отправления: