Re: Reducing connection overhead in pg_upgrade compat check phase - Mailing list pgsql-hackers

From Nathan Bossart
Subject Re: Reducing connection overhead in pg_upgrade compat check phase
Date
Msg-id 20230710230907.GA481155@nathanxps13
Whole thread Raw
In response to Re: Reducing connection overhead in pg_upgrade compat check phase  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Reducing connection overhead in pg_upgrade compat check phase
List pgsql-hackers
On Mon, Jul 10, 2023 at 04:43:23PM +0200, Daniel Gustafsson wrote:
>> On 8 Jul 2023, at 23:43, Nathan Bossart <nathandbossart@gmail.com> 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



pgsql-hackers by date:

Previous
From: Andreas Karlsson
Date:
Subject: Re: [PoC] Implementation of distinct in Window Aggregates
Next
From: Cary Huang
Date:
Subject: Re: sslinfo extension - add notbefore and notafter timestamps