Date: Tue, 12 Mar 2024 04:51:34 -0400
From: Stephen Frost <sfrost@snowman.net>
To: Corey Huinker <corey.huinker@gmail.com>
Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	Peter Smith <smithpb2250@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Tomas Vondra <tomas.vondra@enterprisedb.com>
Subject: Re: Statistics Import and Export
Message-ID: <ZfAXln76MPzwtkLU@tamriel.snowman.net>
References: 
 <CAOuzzgpw1ridazdKsedj=_srNGvyT2wSoHR9AT77BZ567BVtbg@mail.gmail.com>
 <CAEze2WidTCpSkPaL_R=ojmmAyYq_RHSG+dS27upbfF__5KGKZg@mail.gmail.com>
 <Zei1xMGH8SJS9yoc@tamriel.snowman.net>
 <CADkLM=dB-qbU81LuAQVSQxAabYXghXe6ptaatTYmQAzVT44V_g@mail.gmail.com>
 <Zer+8KR36gzST29w@tamriel.snowman.net>
 <CADkLM=eM7rqJpArtRrxsVbOn=3GBRUyKtQbC_TxtFe010QLR8Q@mail.gmail.com>
 <Ze7WQBMPvzXoRCCj@tamriel.snowman.net>
 <CADkLM=d3XhRSySPX+CHL7KmBCgt7ZpqL-0NLxdSWNYHx=MBjag@mail.gmail.com>
 <Ze9SD2gPVwklwvEZ@tamriel.snowman.net>
 <CADkLM=eo4rmN8fFnxNycQtzTnsKwBxdXbAJMTvMC_A1_7HzRdA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="wcRNTwCrQUoGUxVf"
Content-Disposition: inline
In-Reply-To: 
 <CADkLM=eo4rmN8fFnxNycQtzTnsKwBxdXbAJMTvMC_A1_7HzRdA@mail.gmail.com>
User-Agent: Mutt/2.1.4 (2021-12-11)
Archived-At: 
 <https://www.postgresql.org/message-id/ZfAXln76MPzwtkLU%40tamriel.snowman.net>
Precedence: bulk


--wcRNTwCrQUoGUxVf
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Corey Huinker (corey.huinker@gmail.com) wrote:
> > > One persistent problem is that there is no _safe equivalent to ARRAY_=
IN,
> > so
> > > that can always fail on us, though it should only do so if the string
> > > passed in wasn't a valid array input format, or the values in the arr=
ay
> > > can't coerce to the attribute's basetype.
> >
> > That would happen before we even get to being called and there's not
> > much to do about it anyway.
>=20
> Not sure I follow you here. the ARRAY_IN function calls happen once for
> every non-null stavaluesN parameter, and it's done inside the function
> because the result type could be the base type for a domain/array type, or
> could be the type itself. I suppose we could move that determination to t=
he
> caller, but then we'd need to call get_base_element_type() inside a clien=
t,
> and that seems wrong if it's even possible.

Ah, yeah, ok, I see what you're saying here and sure, there's a risk
those might ERROR too, but that's outright invalid data then as opposed
to a NULL getting passed in.

> > > Or one compound command
> > >
> > >     SELECT pg_set_relation_stats(t.oid, ...)
> > >          pg_set_attribute_stats(t.oid, 'id'::name, ...),
> > >          pg_set_attribute_stats(t.oid, 'last_name'::name, ...),
> > >          ...
> > >     FROM (VALUES('foo.bar'::regclass)) AS t(oid);
> > >
> > > The second one has the feature that if any one attribute fails, then =
the
> > > whole update fails, except, of course, for the in-place update of
> > pg_class.
> > > This avoids having an explicit transaction block, but we could get th=
at
> > > back by having restore wrap the list of commands in a transaction blo=
ck
> > > (and adding the explicit lock commands) when it is safe to do so.
> >
> > Hm, I like this approach as it should essentially give us the
> > transaction block we had been talking about wanting but without needing
> > to explicitly do a begin/commit, which would add in some annoying
> > complications.  This would hopefully also reduce the locking concern
> > mentioned previously, since we'd get the lock needed in the first
> > function call and then the others would be able to just see that we've
> > already got the lock pretty quickly.
>=20
> True, we'd get the lock needed in the first function call, but wouldn't we
> also release that very lock before the subsequent call? Obviously we'd be
> shrinking the window in which another process could get in line and take a
> superior lock, and the universe of other processes that would even want a
> lock that blocks us is nil in the case of an upgrade, identical to existi=
ng
> behavior in the case of an FDW ANALYZE, and perfectly fine in the case of
> someone tinkering with stats.

No, we should be keeping the lock until the end of the transaction
(which in this case would be just the one statement, but it would be the
whole statement and all of the calls in it).  See analyze.c:268 or
so, where we call relation_close(onerel, NoLock); meaning we're closing
the relation but we're *not* releasing the lock on it- it'll get
released at the end of the transaction.

Thanks!

Stephen

--wcRNTwCrQUoGUxVf
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEwf6gbxKhD863zrx/7WyKOINHZFUFAmXwF5QACgkQ7WyKOINH
ZFXJDBAAluRUwbnFoK61vcCEztLNElzy6/KMVhiZ8nhA16yOro4Ac346q18uKMdG
LuL32doNxlR1RPH4OzKZbyx2qFN2oDpzDt17wapqL9qWMrFPz04DQhwpC6mO/Gho
pC14CRyyGSOoJwF3wFihfwo2Y1ge22SkSmwPR3l1StvT/gfWP/dp85ME0R4m0FsR
iHW5zKwCXz7rPCTKTKtPk7w4Fn0Jx7yjp4OT73kq5mKUWyyPqwa9iY+zjoLaNJCi
I2ICFKtq/BGBq+j+phjc1wOdtxCj49sGYNLXuJi4431rvsElxUerH5Qt4sKKbxVE
RXc9BblJoDFU7/vRnTVACxRG0FWbNC9/p97gt9UKIxt/bMJs3ITDBxqREnWdaMjU
r52PsdE2rRXCOrr92bA8SiimnX3ISlYRuI6FOWvQ2hRr/37cAkE+TTFa9MzdKPAx
qDzefQYRbl3wczJGGeZocafS456lnYkXidR6AUSz+OCM7Uy7sPcpD6n0dxIs4MsC
jsEB1iOIR/yVsxWiWG3mCPU0E8qKvos3laZUtQ67DmN9h60iU7e9lzKWpozPMnee
VwWhmBcFTgLykTYrKqigA0N1fIi6qnjLoevelHeeGBzwGw16j3GHiC5Qz98SADjB
dbIHp3zNZAFKMDVdYqgY5KvKCdnzRmjZXRQBjzuzpTOa18FX6Lg=
=oJd1
-----END PGP SIGNATURE-----

--wcRNTwCrQUoGUxVf--