On Thu, Feb 19, 2026 at 10:13 AM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> Thanks for reviewing the patch. I have addressed the remaining
> comments in the v46 patch..
>
Can we think of some ideas to split this patch? One possibility is
that in the first patch we give an ERROR, if a non-root partitioned
table is present in EXCEPT Clause. I see that a lot of code is written
to handle partitions in EXCEPT clause. I feel such a split will make
code easier to review and validate.
Few other comments:
=================
1.
if (stmt->for_all_tables)
{
+ /* Process EXCEPT table list */
+ if (relations != NIL)
+ {
+ Assert(rels != NIL);
+ PublicationAddTables(puboid, rels, true, NULL);
+ }
+
/*
* Invalidate relcache so that publication info is rebuilt. Sequences
* publication doesn't require invalidation, as replica identity
CacheInvalidateRelcacheAll();
Here, the relations listed in the except table list will be
invalidated twice, once inside
PublicationAddTables->publication_add_relation, and second time via
CacheInvalidateRelcacheAll. Can we avoid that by adding a parameter to
PublicationAddTables?
2.
- root_relids = GetPublicationRelations(pubform->oid,
- PUBLICATION_PART_ROOT);
+ root_relids = GetIncludedRelationsByPub(pubform->oid,
+ PUBLICATION_PART_ROOT);
To follow the previous function naming pattern, can we rename
GetIncludedRelationsByPub to GetIncludedPublicationRelations? If we
agree to this then rename the excluded version of the function as
well.
3.
+/*
+ * Return the list of relation Oids for a publication.
+ *
+ * For a FOR TABLE publication, this returns the list of relations explicitly
+ * included in the publication.
+ *
+ * Publications declared with FOR ALL TABLES/SEQUENCES should use
+ * GetAllPublicationRelations() to obtain the complete set of tables/sequences
+ * covered by the publication.
+ */
+List *
+GetIncludedRelationsByPub(Oid pubid, PublicationPartOpt pub_partopt)
This is equivalent to the existing function GetPublicationRelations()
which has more precise comments atop. We can use the same comments
unless there is any functionality difference.
4.
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -29,6 +29,7 @@
#include "catalog/pg_publication.h"
#include "catalog/pg_publication_namespace.h"
#include "catalog/pg_publication_rel.h"
+#include "catalog/pg_subscription.h"
It appears odd to include pg_subscription.h in the publication code.
Is there a reason for the same? If not then avoid it.
Apart from above, a few cosmetic changes are attached.
--
With Regards,
Amit Kapila.
