public inbox for [email protected]  

help / color / mirror / Atom feed

From: 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, 13 Mar 2023 11:21:47 -0700
Message-ID: <20230313182147.GA294021@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<20230218054613.GA3360104@nathanxps13>
	<[email protected]>
	<20230222192006.GA79448@nathanxps13>
	<[email protected]>
	<[email protected]>

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

view thread (8+ 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: <20230313182147.GA294021@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