public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Reducing connection overhead in pg_upgrade compat check phase
Date: Mon, 10 Jul 2023 16:09:07 -0700
Message-ID: <20230710230907.GA481155@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <20230218054613.GA3360104@nathanxps13>
<[email protected]>
<20230222192006.GA79448@nathanxps13>
<[email protected]>
<[email protected]>
<20230313182147.GA294021@nathanxps13>
<20230704190839.GA3468243@nathanxps13>
<[email protected]>
<20230708214354.GA4044380@nathanxps13>
<[email protected]>
On Mon, Jul 10, 2023 at 04:43:23PM +0200, Daniel Gustafsson wrote:
>> On 8 Jul 2023, at 23:43, Nathan Bossart <[email protected]> wrote:
>> Since "script" will be NULL and "db_used" will be false in the first
>> iteration of the loop, couldn't we move this stuff to before the loop?
>
> We could. It's done this way to match how all the other checks are performing
> the inner loop for consistency. I think being consistent is better than micro
> optimizing in non-hot codepaths even though it adds some redundancy.
>
> [ ... ]
>
>> Won't "script" always be initialized here? If I'm following this code
>> correctly, I think everything except the fclose() can be removed.
>
> You are right that this check is superfluous. This is again an artifact of
> modelling the code around how the other checks work for consistency. At least
> I think that's a good characteristic of the code.
I can't say I agree with this, but I'm not going to hold up the patch over
it. FWIW I was looking at this more from a code simplification/readability
standpoint.
>> +static int n_data_types_usage_checks = 7;
>>
>> Can we determine this programmatically so that folks don't need to remember
>> to update it?
>
> Fair point, I've added a counter loop to the beginning of the check function to
> calculate it.
+ /* Gather number of checks to perform */
+ while (tmp->status != NULL)
+ n_data_types_usage_checks++;
I think we need to tmp++ somewhere here.
>> In fact, I wonder if we could just add the versions directly to
>> data_types_usage_checks so that we don't need the separate hook functions.
>
> We could, but it would be sort of contrived I think since some check <= and
> some == while some check the catversion as well (and new ones may have other
> variants. I think this is the least paint-ourselves-in-a-corner version, if we
> feel it's needlessly complicated and no other variants are added we can revisit
> this.
Makes sense.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
view thread (25+ 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]
Subject: Re: Reducing connection overhead in pg_upgrade compat check phase
In-Reply-To: <20230710230907.GA481155@nathanxps13>
* 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