MIME-Version: 1.0
References: <!2751089.1710544721@sss.pgh.pa.us>
 <4a3ebf7d81bfc6dd4d545e5b27d6e8f6c32d8937.camel@cybertec.at>
 <3023817.1710629175@sss.pgh.pa.us>
 <6603e4e0.500a0220.a557f.4f39@mx.google.com>
 <3304322.1711551245@sss.pgh.pa.us> <20240327150826.GB3994937@nathanxps13>
 <20240401191930.GA2302032@nathanxps13> <1217588.1711999706@sss.pgh.pa.us>
 <ZqEND4ZcTDBmcv31@pryzbyj2023>
 <CAPpHfduUSA7c0YrTxN443SwXXCL41vmXkavYJjN8GS3Wc5Z42w@mail.gmail.com>
 <ZqQIxIsaPWO6coil@pryzbyj2023> <1870579.1722033430@sss.pgh.pa.us>
 <CAPpHfdtpr2FYVYqmARZdSp=2zsndgJgmSsHFxFFNTiYRfX8uaA@mail.gmail.com>
 <1873872.1722035181@sss.pgh.pa.us>
 <CAPpHfdu63MK5k4qBNc4YV14MFdNq8rCsvz0x7+Z=ijbFm1y6wQ@mail.gmail.com>
 <1903479.1722049686@sss.pgh.pa.us> <2422717.1722201869@sss.pgh.pa.us>
 <CAPpHfduszAcQk9YAQd8PyUhtzY=SFXhBHjrhRzBn0vLaVJny2g@mail.gmail.com>
In-Reply-To: 
 <CAPpHfduszAcQk9YAQd8PyUhtzY=SFXhBHjrhRzBn0vLaVJny2g@mail.gmail.com>
From: PP L <flowerstair@gmail.com>
Date: Mon, 23 Mar 2026 12:47:28 -0700
Message-ID: 
 <CAGzayZug6y2+3fz=5G6Vs2wr5hCnyc+i2tysgTXc70E52aoCfg@mail.gmail.com>
Subject: Re: pg_upgrade failing for 200+ million Large Objects
To: Alexander Korotkov <aekorotkov@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
 Nathan Bossart <nathandbossart@gmail.com>,
	Michael Banck <mbanck@gmx.net>, Laurenz Albe <laurenz.albe@cybertec.at>,
	vignesh C <vignesh21@gmail.com>, "Kumar, Sachin" <ssetiya@amazon.com>,
	Robins Tharakan <tharakan@gmail.com>, Jan Wieck <jan@wi3ck.info>,
 Bruce Momjian <bruce@momjian.us>,
	Andrew Dunstan <andrew@dunslane.net>, Magnus Hagander <magnus@hagander.net>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
 pgsql-hackers@postgresql.org
Content-Type: multipart/alternative; boundary="000000000000a168d3064db64e8e"
Archived-At: 
 <https://www.postgresql.org/message-id/CAGzayZug6y2%2B3fz%3D5G6Vs2wr5hCnyc%2Bi2tysgTXc70E52aoCfg%40mail.gmail.com>
Precedence: bulk

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

Hello hackers,

I wanted to revive this thread specifically around the attislocal
optimization discussion. As part of
https://github.com/postgres/postgres/commit/b3f0e0503f3, we now batch all
the attislocal UPDATEs together, hence making it more performant.

I think we might be able to go one step further and completely skip the
attislocal UPDATE for partition tables. This is because the attislocal
UPDATE is done immediately after 'CREATE TABLE', during the 'ATTACH
PARTITION' step(see attislocal being set to false in
MergeAttributesIntoExisting). The UPDATEs emitted by pg_dump are therefore
redundant. Even with batching, the single UPDATE still modifies N(no of
columns) rows causing N relcache invalidations. This same workflow is then
repeated by ATTACH PARTITION causing another N relcache invalidations.

Skipping the attislocal UPDATE definitely speeds up the runtime if there
are a lot of partition tables because we will avoid quite a lot of relcache
invalidations and rebuild calls. Since this optimization removes the
attislocal UPDATE completely, the effect will be even more pronounced for
wider partition tables.

There's already precedent for this:
* attinhcount is never explicitly set by pg_dump. It is only modified by
MergeAttributesIntoExisting during ATTACH PARTITION
* conislocal for CHECK constraints is explicitly not fixed for partitions.
See comment "No need to fix conislocal: ATTACH PARTITION does that" in
dumpTableSchema

The only risk I can foresee is the window between CREATE TABLE and ATTACH
PARTITION where attislocal will be incorrectly set to true. But I think
this window is small enough to not worry about since ATTACH PARTITION
immediately succeeds CREATE TABLE(maybe barring some other minor updates)

Here is a simple patch
```
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 137161aa5e0..d3d7403228a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -17734,7 +17734,8 @@ dumpTableSchema(Archive *fout, const TableInfo
*tbinfo)
                        for (j =3D 0; j < tbinfo->numatts; j++)
                        {
                                if (!tbinfo->attisdropped[j] &&
-                                       !tbinfo->attislocal[j])
+                                       !tbinfo->attislocal[j] &&
+                                       !tbinfo->ispartition)
                                {
                                        if (firstitem)
                                        {
```

I tried a few experiments on my local macbook and noticed an improvement of
around ~33-36% (% can vary mostly depending on the number of columns)

Setup(master branch): 300 partitioned root tables with 200 leaves each =3D
60000 partition tables

300 columns per partition:
    baseline    : ~30 mins
    with patch : ~20 mins (~33% faster)

700 columns per partition:
    baseline   : ~66 mins
    with patch : ~42 mins (~36% faster)

Thanks
Nikhil
Broadcom Inc.


On Mon, 23 Mar 2026 at 11:50, Alexander Korotkov <aekorotkov@gmail.com>
wrote:

> On Mon, Jul 29, 2024 at 12:24=E2=80=AFAM Tom Lane <tgl@sss.pgh.pa.us> wro=
te:
> > So I'm forced to the conclusion that we'd better make the transaction
> > size adaptive as per Alexander's suggestion.
> >
> > In addition to the patches attached, I experimented with making
> > dumpTableSchema fold all the ALTER TABLE commands for a single table
> > into one command.  That's do-able without too much effort, but I'm now
> > convinced that we shouldn't.  It would break the semicolon-counting
> > hack for detecting that tables like these involve extra work.
> > I'm also not very confident that the backend won't have trouble with
> > ALTER TABLE commands containing hundreds of subcommands.  That's
> > something we ought to work on probably, but it's not a project that
> > I want to condition v17 pg_upgrade's stability on.
> >
> > Anyway, proposed patches attached.  0001 is some trivial cleanup
> > that I noticed while working on the failed single-ALTER-TABLE idea.
> > 0002 merges the catalog-UPDATE commands that dumpTableSchema issues,
> > and 0003 is Alexander's suggestion.
>
> Nice to see you picked up my idea.  I took a look over the patchset.
> Looks good to me.
>
> ------
> Regards,
> Alexander Korotkov
> Supabase
>
>
>
>
>

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

<div dir=3D"ltr"><div>Hello hackers,</div><div><br></div><div><span>I wante=
d to revive this thread specifically around the attislocal optimization dis=
cussion.=C2=A0</span>As part of <a href=3D"https://github.com/postgres/post=
gres/commit/b3f0e0503f3">https://github.com/postgres/postgres/commit/b3f0e0=
503f3</a>, we now batch all the attislocal UPDATEs together, hence making i=
t more performant.</div><br>I think we might be able to go one step further=
 and completely skip the attislocal UPDATE for partition tables. This is be=
cause the attislocal UPDATE is done immediately after &#39;CREATE TABLE&#39=
;, during the &#39;ATTACH PARTITION&#39; step(see attislocal being set to f=
alse in MergeAttributesIntoExisting). The UPDATEs emitted by pg_dump are th=
erefore redundant. Even with batching, the single UPDATE=C2=A0still modifie=
s N(no of columns) rows causing N relcache invalidations. This same workflo=
w is then repeated by ATTACH PARTITION causing another N relcache invalidat=
ions.<br><br>Skipping the attislocal UPDATE definitely speeds up the runtim=
e if there are a lot of partition tables because we will avoid quite a lot =
of relcache invalidations and rebuild calls. Since this optimization remove=
s the attislocal UPDATE completely, the effect will be even more pronounced=
 for wider partition tables.<br><br>There&#39;s already precedent for this:=
<br>* attinhcount is never explicitly set by pg_dump. It is only modified b=
y MergeAttributesIntoExisting during ATTACH PARTITION<br>* conislocal for C=
HECK constraints is explicitly not fixed for partitions. See comment &quot;=
No need to fix conislocal: ATTACH PARTITION does that&quot; in dumpTableSch=
ema<br><br>The only risk I can foresee is the window between CREATE TABLE a=
nd ATTACH PARTITION where attislocal will be incorrectly set to true. But I=
 think this window is small enough to not worry about since ATTACH PARTITIO=
N immediately succeeds CREATE TABLE(maybe barring some other minor updates)=
<br><br>Here is a simple patch<br>```<br>diff --git a/src/bin/pg_dump/pg_du=
mp.c b/src/bin/pg_dump/pg_dump.c<br>index 137161aa5e0..d3d7403228a 100644<b=
r>--- a/src/bin/pg_dump/pg_dump.c<br>+++ b/src/bin/pg_dump/pg_dump.c<br>@@ =
-17734,7 +17734,8 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo=
)<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 for (j =3D 0; j &lt; tbinfo-&gt;numatts; j++)<br>=C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {=
<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!tbinfo-&gt;attisdropped[j] &=
amp;&amp;<br>- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 !=
tbinfo-&gt;attislocal[j])<br>+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 !tbinfo-&gt;attislocal[j] &amp;&amp;<br>+ =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 !tbinfo-&gt;ispartition)<br>=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {<br>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (firstitem)<br>=C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 {<br>```<br><br>I tried a few=
 experiments on my local macbook and noticed an improvement of around ~33-3=
6% (% can vary mostly depending on the number of columns)<br><br>Setup(mast=
er branch): 300 partitioned root tables with 200 leaves each =3D 60000 part=
ition tables<br><br>300 columns per partition:<br>=C2=A0 =C2=A0 baseline=C2=
=A0 =C2=A0 : ~30 mins<br>=C2=A0 =C2=A0 with patch : ~20 mins (~33% faster)<=
br><br>700 columns per partition:<br>=C2=A0 =C2=A0 baseline =C2=A0 : ~66 mi=
ns<br>=C2=A0 =C2=A0 with patch : ~42 mins (~36% faster)<br><div><br></div><=
div>Thanks<br>Nikhil<br>Broadcom Inc.<br><br></div></div><br><div class=3D"=
gmail_quote gmail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">On=
 Mon, 23 Mar 2026 at 11:50, Alexander Korotkov &lt;<a href=3D"mailto:aekoro=
tkov@gmail.com">aekorotkov@gmail.com</a>&gt; wrote:<br></div><blockquote cl=
ass=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid=
 rgb(204,204,204);padding-left:1ex">On Mon, Jul 29, 2024 at 12:24=E2=80=AFA=
M Tom Lane &lt;<a href=3D"mailto:tgl@sss.pgh.pa.us" target=3D"_blank">tgl@s=
ss.pgh.pa.us</a>&gt; wrote:<br>
&gt; So I&#39;m forced to the conclusion that we&#39;d better make the tran=
saction<br>
&gt; size adaptive as per Alexander&#39;s suggestion.<br>
&gt;<br>
&gt; In addition to the patches attached, I experimented with making<br>
&gt; dumpTableSchema fold all the ALTER TABLE commands for a single table<b=
r>
&gt; into one command.=C2=A0 That&#39;s do-able without too much effort, bu=
t I&#39;m now<br>
&gt; convinced that we shouldn&#39;t.=C2=A0 It would break the semicolon-co=
unting<br>
&gt; hack for detecting that tables like these involve extra work.<br>
&gt; I&#39;m also not very confident that the backend won&#39;t have troubl=
e with<br>
&gt; ALTER TABLE commands containing hundreds of subcommands.=C2=A0 That=
9;s<br>
&gt; something we ought to work on probably, but it&#39;s not a project tha=
t<br>
&gt; I want to condition v17 pg_upgrade&#39;s stability on.<br>
&gt;<br>
&gt; Anyway, proposed patches attached.=C2=A0 0001 is some trivial cleanup<=
br>
&gt; that I noticed while working on the failed single-ALTER-TABLE idea.<br=
>
&gt; 0002 merges the catalog-UPDATE commands that dumpTableSchema issues,<b=
r>
&gt; and 0003 is Alexander&#39;s suggestion.<br>
<br>
Nice to see you picked up my idea.=C2=A0 I took a look over the patchset.<b=
r>
Looks good to me.<br>
<br>
------<br>
Regards,<br>
Alexander Korotkov<br>
Supabase<br>
<br>
<br>
<br>
<br>
</blockquote></div>

--000000000000a168d3064db64e8e--