| From: | vignesh C <vignesh21(at)gmail(dot)com> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Peter Smith <smithpb2250(at)gmail(dot)com> |
| Subject: | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Date: | 2026-08-14 14:22:41 |
| Message-ID: | CALDaNm3j7dqu5BOR2Mb+CLG60-KcszqAc4qyaTbgBNAgWZfvsw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 12 Aug 2026 at 14:40, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Tue, Aug 11, 2026 at 2:28 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> >
> > Apart from these, the rest of the comments are also fixed in the v26
> > version attached.
>
> Thanks Vignesh. A few comments on v26-0001:
>
> 1)
>
> publication_add_relation() accepts 'if_not_exists' i.e. add the new
> entry 'if already not present' else skip it (no error if it is a
> duplicate addition). Most flows pass it as 'true'. The current code
> skips raising error if if_not_exists=true and entry exists. It made
> sense earlier, but in our implementation, I feel it should still raise
> an error if entries are cross wired (i.e., if an exclusion is present
> and we are trying to add it as an inclusion, or vice versa). The
> 'if_not_exists' based 'skip logic' should only be exercised if the
> nature of existing entity is of same kind as user is trying to add.
>
> Let me know if you have different understanding.
That is my understanding as well.
I tested a few scenarios to determine whether there was an actual
issue and found that the following cases were succeeding when they
should have thrown an error:
Scenario 1:
CREATE PUBLICATION p FOR TABLES IN SCHEMA s1 EXCEPT (TABLE s1.t);
ALTER PUBLICATION p ADD TABLE s1.t;
Scenario 2:
CREATE PUBLICATION p2 FOR TABLE s1.t;
ALTER PUBLICATION p2 ADD TABLES IN SCHEMA s1 EXCEPT (TABLE s1.t);
Both scenarios should have resulted in an error, but they currently
succeed. I have addressed these cases as well.
> 2)
> ProcessSchemaExceptTables:
>
> + * Also rejects a schema being mentioned more than once with an EXCEPT
> + * clause, even if the EXCEPT clauses are identical — much like
> + * OpenTableList() rejects "FOR TABLE t1(a), t1(a)" despite the column
> + * lists matching. A schema can still be mentioned multiple times, just
> + * not more than once with EXCEPT.
> + *
> + * Qualify unqualified EXCEPT table names with the given schema (rejecting
> + * any explicitly qualified with a different schema), and append them to
> + * *except_pubtables.
>
> We changed the order of comments above, but missed to change these:
>
> 'Also rejects a schema ' -> Reject a schema
> Qualify unqualified -> 'Also qualify unqualified EXCEPT table..'
Modified
> 3)
> CheckExceptNotInTableList:
> + * Both checks are performed here, on the lists supplied by the statement,
> + * because at this point the catalog cannot answer either question. The
> + * corresponding checks in publication_add_relation() and
> + * check_publication_add_relation() rely on pg_publication_rel entries and
> + * ancestor lookups, neither of which is sufficient in the cases below:
>
> Since we have changed comment prior to it, this too needs to be
> changed as 'Both checks' does not make much sense.
>
> Suggestion:
> Similar checks are present in publication_add_relation() and
> check_publication_add_relation(), but they rely on pg_publication_rel
> entries and ancestor lookups. Here, the checks are performed on the
> object lists collected during the statement, as the catalog state is
> not sufficient in the cases below:
Modified
The v27 version patch attached at [1] has the changes for the same.
[1] - https://www.postgresql.org/message-id/CALDaNm0802S%3D557rKtYcV%2BoEbXFw23n%3DXtPHMCbKGSgOgOgk2A%40mail.gmail.com
Regards,
Vignesh
| From | Date | Subject | |
|---|---|---|---|
| Next Message | vignesh C | 2026-08-14 14:29:23 | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Previous Message | Ayush Tiwari | 2026-08-14 14:16:09 | Re: Add a pg_wal_preallocate() SQL function to eagerly create future WAL segments |