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: Thu, 3 Apr 2025 21:19:51 -0500
Message-ID: <Z-9Bx3ml2i7OfHiN@nathan> (raw)
In-Reply-To: <[email protected]>
References: <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]>
<Z-ytSGUZAQNIM9GJ@nathan>
<[email protected]>
<Z-3x2AnPCP331JA3@nathan>
<[email protected]>
Thanks for reviewing.
On Thu, Apr 03, 2025 at 03:23:40PM -0700, Jeff Davis wrote:
> This simplifies commit a0a4601765. I'd break out that simplification as
> a separate commit to make it easier to understand what happened.
Done.
> In patch 0003, there are quite a few static function-scoped variables,
> which is not a style that I'm used to. One idea is to bundle them into
> a struct representing the cache state (including enough information to
> fetch the next batch), and have a single static variable that points to
> that.
As discussed off-list, I didn't take this suggestion for now. Corey did
this originally, and I converted it to static function-scoped variables 1)
to reduce patch size and 2) because I noticed that each of the state
variables were only needed in one function. I agree that a struct might be
slightly more readable, but we can always change this in the future if
desired.
> Also in 0003, the "next_te" variable is a bit confusing, because it's
> actually the last TocEntry, until it's advanced to point to the current
> one.
I've renamed it to expected_te.
> Other than that, looks good to me.
Great. I'm planning to commit the attached patch set tomorrow morning.
For the record, I spent most of today trying very hard to fix the layering
violations in 0002. While I was successful, the result was awkward,
complicated, and nigh unreadable. This is now the second time I've
attempted to fix this and have felt the result was worse than where I
started. So, I added extremely descriptive comments instead. I'm hoping
that it will be possible to clean this up with some additional work in v19.
I have a few ideas, but if anyone has suggestions, I'm all ears.
--
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-9Bx3ml2i7OfHiN@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