| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com> |
| Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, YeXiu <1518981153(at)qq(dot)com>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Skipping schema changes in publication |
| Date: | 2026-03-27 20:50:57 |
| Message-ID: | CANhcyEXdc6R6MrrC-Co_mmWLkync9q0GodCgegcC3VUC=k3Hbw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, 28 Mar 2026 at 00:33, SATYANARAYANA NARLAPURAM
<satyanarlapuram(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Fri, Mar 27, 2026 at 12:50 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Corey Huinker | 2026-03-27 20:59:49 | Re: CAST(... ON DEFAULT) - WIP build on top of Error-Safe User Functions |
| Previous Message | Shlok Kyal | 2026-03-27 20:49:34 | Re: Skipping schema changes in publication |