MIME-Version: 1.0
References: 
 <CAKz==b+k7OwOib__LgfDadUzW9jFrbQ9FqdERpsufi8d7F=vkQ@mail.gmail.com>
 <202603261226.uo54xj4rpex2@alvherre.pgsql>
 <CAKFQuwa8i+TFSfs_LVCVZpK2jXyjkD_giKPg1rt57AsXrgW=BA@mail.gmail.com>
In-Reply-To: 
 <CAKFQuwa8i+TFSfs_LVCVZpK2jXyjkD_giKPg1rt57AsXrgW=BA@mail.gmail.com>
From: Roberto Mello <roberto.mello@gmail.com>
Date: Tue, 31 Mar 2026 17:23:11 -0600
Message-ID: 
 <CAKz==b+MwMXSqfXyq90PW4qmNDTqrVAoTLAG3BsK-tpq54WkOg@mail.gmail.com>
Subject: Re: pg_publication_tables: return NULL attnames when no column list
 is specified
To: "David G. Johnston" <david.g.johnston@gmail.com>
Cc: =?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de>,
	PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000c1fbfb064e5a416c"
Archived-At: 
 <https://www.postgresql.org/message-id/CAKz%3D%3Db%2BMwMXSqfXyq90PW4qmNDTqrVAoTLAG3BsK-tpq54WkOg%40mail.gmail.com>
Precedence: bulk

--000000000000c1fbfb064e5a416c
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 31, 2026 at 4:55=E2=80=AFPM David G. Johnston <
david.g.johnston@gmail.com> wrote:

>
> IIUC the wording for v18 and earlier should read more like:
>
> =E2=80=9CSubscriptions having several publications in which the same tabl=
e has
> different sets of columns published are not supported.=E2=80=9D
>
> The claim that this defacto behavior is a bug needing to be fixed is now
> before us (there is no disagreement that the physical column lists are
> different - null vs non-null).  My cursory take at this leads me to belie=
ve
> we should accept what actually got implemented and not call this a bug to
> be fixed (aside from the docs).
>
> That the catalog is the only official source of truth regarding the
> physical column list distinction, and the function represents the logical
> =E2=80=9Cset of columns actually seen=E2=80=9D, makes sense seen in that =
light.
>

The internal code was designed around the NULL/non-NULL distinction. The
SRF
pg_get_publication_tables() is the one place that erased it, and the CASE
WHEN relnatts heuristic
in tablesync was an attempt to reverse that erasure, but it's demonstrably
broken for
tables with dropped columns. That seems like a bug to me regardless of how
we feel about the
behavioral question, but I have no objections to not calling it a bug. I'm
confident the
best thing was intended when the code was committed and hindsight is always
20/20.

I haven=E2=80=99t dived deep enough to understand whether there is C code i=
ssue
> that needs to be resolved.  Or whether we can make dealing with this more
> user-friendly given this constraint.
>
> Removing the limitation would seem more appealing if we are going to make
> a change.  The obvious answer of =E2=80=9Cunion all sets of columns publi=
shed for a
> table and replicate those=E2=80=9D would be the simplest to document thou=
gh I
> suspect the current implementation basically chooses one of the
> publications to pull from which makes that difficult in the general case.
> I do kinda wonder why we need to enforce any kind of error so long as one
> of the publications for a given table includes all columns though.  Or ev=
en
> is a proper superset to be a tiny bit more flexible.  A technically
> uninformed wondering but still.
>

The superset idea would be a significant change to how the WAL output
plugin works. pgoutput.c
doesn't have a concept of "this publication contributes columns X and that
publication contributes
columns Y, send the union."

This would be an interesting improvement but it's a larger project... it
would touch pgoutput, tablesync,
and the subscriber's relation mapping. My patch is trying to fix the
immediate inconsistency (the view
lying about the catalog state, and the broken relnatts heuristic) without
changing the replication protocol
or column merging behavior.

If the view shows {id, name} for both publications, a DBA planning a schema
migration has no way to
know that ALTER TABLE ADD COLUMN email will be replicated for one
publication but not the
other. The catalog stores the information needed to make this
determination, the view actively hides it.
NULL in the view would tell the DBA "this publication replicates
everything, including future columns"
which is actionable information.


Roberto Mello
Snowflake

--000000000000c1fbfb064e5a416c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">On Tue, Mar 31, 2026 at 4:55=E2=80=AFPM D=
avid G. Johnston &lt;<a href=3D"mailto:david.g.johnston@gmail.com">david.g.=
johnston@gmail.com</a>&gt; wrote:</div><div class=3D"gmail_quote gmail_quot=
e_container"><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px =
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><br></d=
iv><div>IIUC the wording for v18 and earlier should read more like:</div><d=
iv><br></div><div><span style=3D"font-family:&quot;Open Sans&quot;,sans-ser=
if;font-size:14.4px">=E2=80=9CSubscriptions having several publications in =
which the same table has different sets of columns published are not suppor=
ted.=E2=80=9D</span><br></div><div><span style=3D"font-family:&quot;Open Sa=
ns&quot;,sans-serif;font-size:14.4px"><br></span></div><div><font face=3D"O=
pen Sans, sans-serif"><span style=3D"font-size:14.4px">The claim that this =
defacto behavior is a bug needing to be fixed is now before us (there is no=
 disagreement that the physical column lists are different - null vs non-nu=
ll).=C2=A0 My cursory take at this leads me to believe we should accept wha=
t actually got implemented and not call this a bug to be fixed (aside from =
the docs).</span></font></div><div><font face=3D"Open Sans, sans-serif"><sp=
an style=3D"font-size:14.4px"><br></span></font></div><div><font face=3D"Op=
en Sans, sans-serif"><span style=3D"font-size:14.4px">That the catalog is t=
he only official source of truth regarding the physical column list distinc=
tion, and the function represents the logical =E2=80=9Cset of columns actua=
lly seen=E2=80=9D, makes sense seen in that light.</span></font></div></blo=
ckquote><div><br></div>The internal code was designed around the NULL/non-N=
ULL distinction. The SRF=C2=A0</div><div class=3D"gmail_quote gmail_quote_c=
ontainer">pg_get_publication_tables() is the one place that=C2=A0erased it,=
 and the CASE WHEN relnatts heuristic=C2=A0</div><div class=3D"gmail_quote =
gmail_quote_container">in tablesync was an attempt to reverse that erasure,=
 but it&#39;s demonstrably broken for<br><div>tables with dropped columns. =
That seems like a bug to me regardless of how we feel about the</div><div>b=
ehavioral question, but I have no objections to not calling it a bug. I&#39=
;m confident the</div><div>best thing was intended when the code was commit=
ted and hindsight is always 20/20.</div><div><br></div><blockquote class=3D=
"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(2=
04,204,204);padding-left:1ex"><div><font face=3D"Open Sans, sans-serif"><sp=
an style=3D"font-size:14.4px">I haven=E2=80=99t dived deep enough to unders=
tand whether there is C code issue that needs to be resolved.=C2=A0 Or whet=
her we can make dealing with this more user-friendly given this constraint.=
</span></font></div><div><font face=3D"Open Sans, sans-serif"><span style=
=3D"font-size:14.4px"><br></span></font></div><div><font face=3D"Open Sans,=
 sans-serif"><span style=3D"font-size:14.4px">Removing the limitation would=
 seem more appealing if we are going to make a change.=C2=A0 The obvious an=
swer of =E2=80=9Cunion all sets of columns published for a table and replic=
ate those=E2=80=9D would be the simplest to document though I suspect the c=
urrent implementation basically chooses one of the publications to pull fro=
m which makes that difficult in the general case.=C2=A0 I do kinda wonder w=
hy we need to enforce any kind of error so long as one of the publications =
for a given table includes all columns though.=C2=A0 Or even is a proper su=
perset to be a tiny bit more flexible.=C2=A0 A technically uninformed wonde=
ring but still.</span></font></div></blockquote><div><br></div><div>The sup=
erset idea would be a significant change to how the WAL output plugin works=
. pgoutput.c</div><div>doesn&#39;t have a concept of=C2=A0&quot;this public=
ation contributes columns X and that publication contributes</div><div>colu=
mns Y, send the union.&quot;</div><div><br></div><div>This would be an inte=
resting improvement but it&#39;s a larger project... it would touch pgoutpu=
t, tablesync,</div><div>and the subscriber&#39;s relation mapping. My patch=
 is trying to fix the immediate inconsistency (the view</div><div>lying abo=
ut the catalog state, and the broken relnatts heuristic) without changing t=
he replication protocol</div><div>or column merging behavior.</div><div><br=
></div><div>If the view shows {id, name} for both publications, a DBA=C2=A0=
planning a schema migration has no way to</div><div>know that ALTER TABLE A=
DD COLUMN email will be replicated for one publication but not the</div><di=
v>other. The catalog stores the information needed to make this determinati=
on, the view actively hides it.</div><div>NULL in the view would tell the D=
BA &quot;this publication replicates everything, including future columns&q=
uot;</div><div>which is actionable information.</div><div><br></div><div><b=
r></div><div>Roberto Mello</div><div>Snowflake</div></div></div>

--000000000000c1fbfb064e5a416c--