public inbox for [email protected]  

help / color / mirror / Atom feed

From: Matthias van de Meent <[email protected]>
To: Tom Lane <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: GenBKI emits useless open;close for catalogs without rows
Date: Tue, 12 Sep 2023 17:51:30 +0200
Message-ID: <CAEze2Wi2Xgf9u+oLo3zgOZ-C=VsgpqQ-cXTNRKwSCXTG+y2Rqg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>

On Fri, 1 Sept 2023 at 19:52, Tom Lane <[email protected]> wrote:
>
> Alvaro Herrera <[email protected]> writes:
> > On 2023-Sep-01, Matthias van de Meent wrote:
> >> A potential addition to the patch would to stop manually closing
> >> relations: initdb and check-world succeed without manual 'close'
> >> operations because the 'open' command auto-closes the previous open
> >> relation (in boot_openrel). Testing also suggests that the last opened
> >> relation apparently doesn't need closing - check-world succeeds
> >> without issues (incl. with TAP enabled). That is therefore implemented
> >> in attached patch 2 - it removes the 'close' syntax in its entirety.
>
> > Hmm, what happens with the last relation in the bootstrap process?  Is
> > closerel() called via some other path for that one?
>
> Taking a quick census of existing closerel() callers: there is
> cleanup() in bootstrap.c, but it's called uncomfortably late
> and outside any transaction, so I misdoubt that it works
> properly if asked to actually shoulder any responsibility.
> (A little code reshuffling could fix that.)
> There are also a couple of low-level elog warnings in CREATE
> that would likely get triggered, though I suppose we could just
> remove those elogs.

Yes, that should be easy to fix.

> I guess my reaction to this patch is "why bother?".  It seems
> unlikely to yield any measurable benefit, though of course
> that guess could be wrong.

There is a small but measurable decrease in size of the generated bki
(2kb with both patches, on an initial 945kB), and there is some
related code that can be eliminated. If that's not worth bothering,
then I can drop the patch. Otherwise, I can update the patch to do the
cleanup that was within the transaction boundaries at the end of
boot_yyparse.

If decreasing the size of postgres.bki is not worth the effort, I'll
drop any effort on doing so, but considering that it is about 1MB of
our uncompressed distributables, I'd say decreases in size are worth
the effort, most of the time.

Kind regards,

Matthias van de Meent

view thread (2+ messages)

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]
  Subject: Re: GenBKI emits useless open;close for catalogs without rows
  In-Reply-To: <CAEze2Wi2Xgf9u+oLo3zgOZ-C=VsgpqQ-cXTNRKwSCXTG+y2Rqg@mail.gmail.com>

* 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