On Wed, 2025-04-02 at 21:26 -0500, Nathan Bossart wrote:
> Okay, here is an updated patch set.
> * Besides custom format calling WriteToc() twice to update the data
> offsets, tar format ... even if it did, the worst case is that
> the
> content of restore.sql (which isn't used by pg_restore) would be
> different. I noticed some past discussion that seems to suggest
> that
> this format might be a candidate for deprecation [0], so I'm not
> sure
> it's worth doing anything fancier.
I agree that the risk for tar format seems much lower.
> * Our batching code assumes that stats entries are dumped in TOC
> order,
>
...
> I propose moving all
> stats entries in SECTION_POST_DATA to RESTORE_PASS_POST_ACL, which
> ensures that we always dump stats in TOC order. One convenient
> side
> effect of this change is that we can revert a decent chunk of
> commit
> a0a4601765. It might be possible to do better via smarter
> lookahead code
> or a more sophisticated cache, but it's a bit late in the game for
> that.
This simplifies commit a0a4601765. I'd break out that simplification as
a separate commit to make it easier to understand what happened.
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.
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.
Other than that, looks good to me.
Regards,
Jeff Davis
