MIME-Version: 1.0
References: 
 <CABdArM5sw4Q1ZU8HGdo4BSc1A_+8xtUNq17j6wcir=yMUy19Cg@mail.gmail.com>
 <CAHut+PvnH8QHa035Skoh1e9jm_H08DO9fQ=F-NAMsEpYf0RZ2Q@mail.gmail.com>
 <CAJpy0uDu0LcNXcZCP0cR_LHqo+sau33KwPFHemmGVYf_JTxRBQ@mail.gmail.com>
 <CAA4eK1KbCWBmEXH-rhQjKgNwq=onZp8vRR-QkRhPpbKwL-kQdw@mail.gmail.com>
 <CAHut+Pvj4=GWoJEd4EBdp4pi6KxXQ46ioW=PV+=UktiXr2gCvg@mail.gmail.com>
 <CABdArM75F0A+DGP8AOt-_b_XREX40rvFid1jRjnr_+S5b51t8Q@mail.gmail.com>
 <CAJpy0uDTshb243L5yEYWB3uO-JrwSoRqQDNovh03K2GZuuR3Pg@mail.gmail.com>
 <CAJpy0uDy97ULmJUwPacAzc5u2seuPK6RXgCS1rnsW2MfR4eeSw@mail.gmail.com>
 <CABdArM6oXXXSAxxXFktTTfBf4kyxJCvdNtTbUZtSwJ=CepN+Xw@mail.gmail.com>
 <CAJpy0uBqM+fq7+g1ZRATuY16H10MFP9i25wfFCYCE5MGu+PE0Q@mail.gmail.com>
 <CABdArM4uKaS1coCQj6rAwMmHqU_cCJyEWNic-PFF1_ZjDDM82Q@mail.gmail.com>
 <CAHut+Pu5VNakf5JAhKM7T-P_q37eN1Qgv5nvZUe+8RAAT41y4g@mail.gmail.com>
 <CABdArM6WTm2gP4pcjVdHT1Nx6zdLKTq7nLPUkzZOhprc95a6Rw@mail.gmail.com>
 <CAHut+Ptthc1X-UA8-6zG-iFeCDuoNd+oJRBZ1eCnJ9RNOXjfBQ@mail.gmail.com>
 <CABdArM79m7-CTf6KGGGU2QBydFtuonGgfxRSqk-vhwTsH8z1ow@mail.gmail.com>
 <CAHut+PuiK_Pa=BkSgBxYzqf1PYh+mcUcUQCr8r1e69-y1r+hhw@mail.gmail.com>
 <CABdArM7rH+3GekRgufEOwrJxUeQk=LB182CQwJD35e0oN7q8ZA@mail.gmail.com>
In-Reply-To: 
 <CABdArM7rH+3GekRgufEOwrJxUeQk=LB182CQwJD35e0oN7q8ZA@mail.gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
Date: Fri, 29 May 2026 15:31:01 +1000
Message-ID: 
 <CAHut+PtP1mbQT==xo=G-37dV9Lt3q7YO2eMEAqSbZuszy93LcQ@mail.gmail.com>
Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAHut%2BPtP1mbQT%3D%3Dxo%3DG-37dV9Lt3q7YO2eMEAqSbZuszy93LcQ%40mail.gmail.com>
Precedence: bulk

Hi Nisha.

Some review comments for patch v7-0001.

=3D=3D=3D=3D=3D=3D
src/backend/catalog/pg_publication.c

GetTopMostAncestorInPublication:

1.
- else
+ else if (!list_member_oid(except_pubids, puboid))
  {
  aschemaPubids =3D GetSchemaPublications(get_rel_namespace(ancestor));

IIUC this `except_pubids` and `puboid` are not changing, so you do not
need to be doing this member check every loop iteration. Is it better
to assign some variable up-front?

e.g.
bool check_schemas =3D !list_member_oid(except_pubids, puboid);

~~~

publication_add_relation:

2.
+ if (is_except)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("relation \"%s\" cannot be added because it is excluded from
publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel),
+ pub->name)));

I am unsure about the changed wording from "table" to "relation". IIUC
we say "relation" when it could be either a table or a sequence. So
maybe "table" is correct for your patch;l OTHOH this should change to
"relation" by Shlok's EXCEPT SEQUENCE patch [1].

~~~

3.
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("relation \"%s\" is already member of publication \"%s\"",
+ RelationGetQualifiedRelationName(targetrel), pub->name)));

IMO making everything fully qualified like this would be a good
change, but doing it here perhaps does not belong in your patch. I
have resurrected this question in the other thread [2], which would
affect not only this statement. but many others. Please post your
opinion about this on that other thread.

=3D=3D=3D=3D=3D=3D
src/backend/commands/publicationcmds.c

ObjectsInPublicationToOids:

4.
 static void
 ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
-    List **rels, List **exceptrels, List **schemas)
+    List **rels, List **except_rel_names, List **schemas)

Is `except_rel_names` an accurate name? IMO it makes it sound like
it's a list of char* names, but IIUC that is not the case; aren't
these PublicationTable objects? Would something like
`except_pubtables` be more correct?

~~~

CreatePublication:

5.
- List    *exceptrelations =3D NIL;
+ List    *except_rel_names =3D NIL;

Same doubts about this `except_rel_names` variable name.

~~~

AlterPublication:
6.
  List    *relations =3D NIL;
- List    *exceptrelations =3D NIL;
+ List    *except_rel_names =3D NIL;

Same doubts about this `except_rel_names` variable name.

=3D=3D=3D=3D=3D=3D
src/backend/replication/pgoutput/pgoutput.c

get_rel_sync_entry:

7.
+ if (am_partition)
+ {
+ List    *part_ancestors =3D get_partition_ancestors(relid);
+
+ root_relid =3D llast_oid(part_ancestors);
+ list_free(part_ancestors);

I think just call this `ancestors` (not `part_ancestors`) for
consistency with other code in the same function.

=3D=3D=3D=3D=3D=3D
src/bin/psql/tab-complete.in.c

On Thu, May 28, 2026 at 9:27=E2=80=AFPM Nisha Moond <nisha.moond412@gmail.c=
om> wrote:
>
>  On Fri, May 22, 2026 at 7:57=E2=80=AFAM Peter Smith <smithpb2250@gmail.c=
om> wrote:
...
> > 6.
> > + else if (Matches("CREATE", "PUBLICATION", MatchAny, "FOR", "TABLES",
> > "IN", "SCHEMA", MatchAny, "EXCEPT", "(", "TABLE", MatchAnyN) &&
> > ends_with(prev_wd, ','))
> > + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables);
> >
> > I'm not sure if this is working as intended.
> >
> > When testing for multiple except tables I get results like:
> > ----
> > test_pub=3D# create publication pub1 for tables IN SCHEMA myschema <TAB=
>
> > EXCEPT ( TABLE  WITH (
> > test_pub=3D# create publication pub1 for tables IN SCHEMA myschema
> > except ( table <TAB>
> > test_pub=3D# create publication pub1 for tables IN SCHEMA myschema
> > except ( table myschema.t<TAB>
> > myschema.t1  myschema.t2  myschema.t3
> > test_pub=3D# create publication pub1 for tables IN SCHEMA myschema
> > except ( table myschema.t1,<TAB>
> > information_schema.  myschema.            public.              t1
> >              t2                   t3
> > ----
> >
> > Note: it is offering suggstions for schema names outside of the
> > "myschema". Should this code be calling
> > Query_for_list_of_tables_in_schema instead of
> > Query_for_list_of_tables?
> >
>
> For this case, I don't think it's really possible to keep suggesting
> after a comma. Even if we handle a fixed number of comma-separated
> entries, the same problem reappears with the next entry.
> Query_for_list_of_tables_in_schema needs a correct schema reference,
> but with each comma the relative offset changes, so there is no single
> fixed prev*_wd that can reliably point to the schema across all
> entries.
>
> But I see, the current call to Query_for_list_of_tables is clearly
> incorrect here. I have now suppressed suggestions after a comma,
> instead of showing incorrect suggestions.
> Thoughts?
>

8.
REPLY: Yeah, I don't have any good ideas how to fix this, or if a fix
is even possible, but I agree that doing nothing is better than doing
the wrong thing.

~~~

9.
BTW, the current code is not able to handle multiple schemas.

So, this works:
test_pub=3D# CREATE PUBLICATION pub1 for TABLES IN SCHEMA myschema <TAB>
EXCEPT ( TABLE  WITH (

but, this doesn't do anything:
test_pub=3D# CREATE PUBLICATION pub1 for TABLES IN SCHEMA public, myschema =
<TAB>

=3D=3D=3D=3D=3D=3D
[1] Shlok EXCEPT -
https://www.postgresql.org/message-id/flat/CAHut%2BPsUrYmbZ996ZybjMWvpW_ufX=
B8WM94pdvAPyzQpoe%2BHRA%40mail.gmail.com#579d9b99c4f620602085cc59ff0e2b7d
[2] schema-qualified messages -
https://www.postgresql.org/message-id/CAHut%2BPvWoOyLKFb627Ch%2BXg3TYHuHdaO=
Z_XmxYgKVYdOzpqFsw%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia