| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(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>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Date: | 2026-08-12 09:10:29 |
| Message-ID: | CAJpy0uCWyUyxGz3CeFZN0_GLGyAmeQoEQniaAQQ0mz3AKzfCxw@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
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..'
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:
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ayush Tiwari | 2026-08-12 09:27:33 | Re: Backup manifests accept out-of-range LSNs |
| Previous Message | Ayush Tiwari | 2026-08-12 09:07:16 | Re: Error handling in after-startup shmem requests |