Date: Mon, 13 Mar 2023 11:21:47 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Reducing connection overhead in pg_upgrade compat check phase
Message-ID: <20230313182147.GA294021@nathanxps13>
References: <BBB4C76F-D416-4F9F-949E-DBE950D37787@yesql.se>
 <20230218054613.GA3360104@nathanxps13>
 <542946CF-0D77-446E-9FA6-028982175320@yesql.se>
 <20230222192006.GA79448@nathanxps13>
 <235E7E38-6DCA-4E5F-812C-7F3033FC800A@yesql.se>
 <89A64E5D-D3DC-4806-B038-CD493F5F5510@yesql.se>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <89A64E5D-D3DC-4806-B038-CD493F5F5510@yesql.se>
Archived-At: <https://www.postgresql.org/message-id/20230313182147.GA294021%40nathanxps13>
Precedence: bulk

On Mon, Mar 13, 2023 at 03:10:58PM +0100, Daniel Gustafsson wrote:
> The attached v3 is a rebase to handle conflicts and with the above comments
> adressed.

Thanks for the new version of the patch.

I noticed that git-am complained when I applied the patch:

    Applying: pg_upgrade: run all data type checks per connection
    .git/rebase-apply/patch:1023: new blank line at EOF.
    +
    warning: 1 line adds whitespace errors.

+				for (int rowno = 0; rowno < ntups; rowno++)
+				{
+					found = true;

It looks like "found" is set unconditionally a few lines above, so I think
this is redundant.

Also, I think it would be worth breaking check_for_data_types_usage() into
a few separate functions (or doing some other similar refactoring) to
improve readability.  At this point, the function is quite lengthy, and I
count 6 levels of indentation at some lines.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com