public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Jeff Davis <[email protected]>
Cc: Corey Huinker <[email protected]>
Cc: Robert Treat <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: jian he <[email protected]>
Cc: Bruce Momjian <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Magnus Hagander <[email protected]>
Cc: Stephen Frost <[email protected]>
Cc: Ashutosh Bapat <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: [email protected]
Subject: Re: Statistics Import and Export
Date: Tue, 1 Apr 2025 22:21:44 -0500
Message-ID: <Z-ytSGUZAQNIM9GJ@nathan> (raw)
In-Reply-To: <[email protected]>
References: <CADkLM=f5Dy3RtGOq=2RH2z4xLXNTwSYCAu-d5ggzQFrb8nCsyg@mail.gmail.com>
<CADkLM=ftC94muGKH+z1irdBUXO2+tmBMLdqDAAxcct=H+LE1Eg@mail.gmail.com>
<CADkLM=desCuf3dVHasADvdUVRmb-5gO0mhMO5u9nzgv6i7U86Q@mail.gmail.com>
<[email protected]>
<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>
<[email protected]>
On Tue, Apr 01, 2025 at 03:05:59PM -0700, Jeff Davis wrote:
> To restate the problem: one of the problems being solved here is that
> the existing code for custom-format dumps calls WriteToc twice. That
> was not a big problem before this patch, when the contents of the
> entries was easily accessible in memory. But the point of 0002 is to
> avoid keeping all of the stats in memory at once, because that causes
> bloat; and instead to query it on demand.
>
> In theory, we could fix the pre-existing code by making the second pass
> able to jump over the other contents of the entry and just update the
> data offsets. But that seems invasive, at least to do it properly.
>
> 0001 sidesteps the problem by skipping the second pass if data's not
> being dumped (because there are no offsets that need updating). The
> worst case is when there are a lot of objects with a small amount of
> data. But that's a worst case for stats in general, so I don't think
> that needs to be solved here.
>
> Issuing the stats queries twice is not great, though. If there's any
> non-deterministic output in the query, that could lead to strangeness.
> How bad can that be? If the results change in some way that looks
> benign, but changes the length of the definition string, can it lead to
> corruption of a ToC entry? I'm not saying there's a problem, but trying
> to understand the risk of future problems.
It certainly feels risky. 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'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). 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. Plus, custom format doesn't appear to compress anything written
via WriteStr().
We might be able to improve this by inventing a new callback that fails for
all formats except for custom with feesko() available. That would at least
ensure hard failures if these assumptions change. That problably wouldn't
be terribly invasive. I'm curious what you think.
> For 0003, it makes an assumption about the way the scan happens in
> WriteToc(). Can you add some additional sanity checks to verify that
> something doesn't happen in a different order than we expect?
Hm. 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). Does that sound like a strong enough check? FWIW the
pg_dump tests failed miserably until Corey and I got this part right, so
our usual tests should also offer some assurance.
> Also, why do we need the clause "WHERE s.tablename = ANY($2)"? Isn't
> that already implied by "JOIN unnest($1, $2) ... s.tablename =
> u.tablename"?
Good question. Corey, do you recall why this was needed?
--
nathan
view thread (87+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Statistics Import and Export
In-Reply-To: <Z-ytSGUZAQNIM9GJ@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox