On Sat, 28 Mar 2026 at 00:33, SATYANARAYANA NARLAPURAM
<satyanarlapuram@gmail.com> wrote:
>
> Hi,
>
> On Fri, Mar 27, 2026 at 12:50 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>>
>>
>> I have addressed the comments. Attached the updated patch.
>>
>> [1]: https://www.postgresql.org/message-id/CAA4eK1Jjm+w3hEGgsDu_r1Pysez=8mmtGu6=XwPE4MuH+eYG8Q@mail.gmail.com
>
>
> A few minor comments:
>
> 1. Add a negative test for missing table keyword for example EXCEPT (t1, t2)
> 2. NIT comment, remove the extra space between parenthesis and TABLE below
>
> else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "ALL", "TABLES"))
> - COMPLETE_WITH("EXCEPT TABLE (", "WITH (");
> + COMPLETE_WITH("EXCEPT ( TABLE", "WITH (");
>
> 3. for pg_dump, to improve readability can you just add TABLE keyword for every tables in the except list?
>
> - /* Include EXCEPT TABLE clause if there are except_tables. */
> + /* Include EXCEPT (TABLE) clause if there are except_tables. */
> for (SimplePtrListCell *cell = pubinfo->except_tables.head; cell; cell = cell->next)
> {
> TableInfo *tbinfo = (TableInfo *) cell->ptr;
>
> if (++n_except == 1)
> - appendPQExpBufferStr(query, " EXCEPT TABLE (");
> + appendPQExpBufferStr(query, " EXCEPT (TABLE ");
> else
> appendPQExpBufferStr(query, ", ");
>
> 4. Is the Assert needed below, code already produces error message.
>
> + Assert(pubobj->pubobjtype == PUBLICATIONOBJ_EXCEPT_TABLE);
> +
> + if (pubobj->pubobjtype == PUBLICATIONOBJ_CONTINUATION)
> + ereport(ERROR,
> + errcode(ERRCODE_SYNTAX_ERROR),
> + errmsg("invalid publication object list"),
> + errdetail("TABLE must be specified before a standalone table."),
> + parser_errposition(pubobj->location));
I think the Assert should stay as a defensive check to ensure this
function is only used for EXCEPT (TABLE) cases.
Thanks for reviewing the patch. I have addressed the remaining
comments and attached the patch in [1].
[1]: https://www.postgresql.org/message-id/CANhcyEUQvEK%2BHOH6Y8Fy30fNvC631ZopWKhwgskXjKnuXiGV5Q%40mail.gmail.com
Thanks,
Shlok Kyal
