Message-ID: <ec43b87247fd8700c6fcf4fe0b68cdb2fafecf2d.camel@j-davis.com>
Subject: Re: Statistics Import and Export
From: Jeff Davis <pgsql@j-davis.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Corey Huinker <corey.huinker@gmail.com>, Robert Treat <rob@xzilla.net>,
 Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>,
 Tom Lane <tgl@sss.pgh.pa.us>,  Michael Paquier <michael@paquier.xyz>, jian
 he <jian.universality@gmail.com>, Bruce Momjian <bruce@momjian.us>,
 Matthias van de Meent <boekewurm+postgres@gmail.com>, Magnus Hagander
 <magnus@hagander.net>, Stephen Frost <sfrost@snowman.net>,  Ashutosh Bapat
 <ashutosh.bapat.oss@gmail.com>, Peter Smith <smithpb2250@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 alvherre@alvh.no-ip.org
Date: Tue, 01 Apr 2025 22:44:19 -0700
In-Reply-To: <Z-ytSGUZAQNIM9GJ@nathan>
References: 
	<CADkLM=f5Dy3RtGOq=2RH2z4xLXNTwSYCAu-d5ggzQFrb8nCsyg@mail.gmail.com>
	 <CADkLM=ftC94muGKH+z1irdBUXO2+tmBMLdqDAAxcct=H+LE1Eg@mail.gmail.com>
	 <CADkLM=desCuf3dVHasADvdUVRmb-5gO0mhMO5u9nzgv6i7U86Q@mail.gmail.com>
	 <f248676c32cbb3a4dd0da13bc84af43760239bf2.camel@j-davis.com>
	 <CADkLM=dG+8jW4UAwP69o7Q92FOXpoN8Pb9kD_Fck1_BBn+gwgQ@mail.gmail.com>
	 <CADkLM=domd5+CvjKMHGbOfvSuY6J8G-x+9M2D6Ss2HamYefE9w@mail.gmail.com>
	 <Z-tQa5zsVkcCyYin@nathan> <Z-wubg4vnItKnOCj@nathan>
	 <Z-wz_sZdceGJ-fuI@nathan>
	 <88af35fe0ff24cc4e0700b841aa60a0865f11648.camel@j-davis.com>
	 <Z-ytSGUZAQNIM9GJ@nathan>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/ec43b87247fd8700c6fcf4fe0b68cdb2fafecf2d.camel%40j-davis.com>
Precedence: bulk

On Tue, 2025-04-01 at 22:21 -0500, Nathan Bossart wrote:
> It certainly feels risky.=C2=A0 I was able to avoid executing the queries
> twice
> in all cases by saving the definition length in the TOC entry and
> skipping
> that many bytes the second time round.

That feels like a better approach.

> =C2=A0 That's simple enough, but it relies
> on various assumptions such as fseeko() being available (IIUC the
> file will
> only be open for writing so we cannot fall back on fread()) and
> WriteStr()
> returning an accurate value (which I'm skeptical of because some
> formats
> compress this data).=C2=A0 But AFAICT custom format is the only format
> that does
> a second WriteToc() pass at the moment, and it only does so when
> fseeko()
> is usable.

Even with those assumptions, I think it's much better than querying
twice and assuming that the results are the same.

> =C2=A0 Plus, custom format doesn't appear to compress anything written
> via WriteStr().

If WriteStr() was doing compression, that would make the second
WriteToc() pass to update the data offsets scary even in the existing
code.

> We might be able to improve this by inventing a new callback that
> fails for
> all formats except for custom with feesko() available.=C2=A0 That would a=
t
> least
> ensure hard failures if these assumptions change.=C2=A0 That problably
> wouldn't
> be terribly invasive.=C2=A0 I'm curious what you think.

That sounds fine, I'd say do that if it feels reasonable, and if the
extra callbacks get too messy, we can just document the assumptions
instead.

>=20
> Hm.=C2=A0 One thing we could do is to send the TocEntry to the callback
> and
> verify that matches the one we were expecting to see next (as set by
> a
> previous call).=C2=A0 Does that sound like a strong enough check?

Again, I'd just be practical here and do the check if it feels natural,
and if not, improve the comments so that someone modifying the code
would know where to look.


Regards,
	Jeff Davis