| From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
|---|---|
| To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Reducing connection overhead in pg_upgrade compat check phase |
| Date: | 2023-07-06 15:58:33 |
| Message-ID: | 9C9D8A09-9794-46DD-8074-8961924B99F1@yesql.se |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 4 Jul 2023, at 21:08, Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> I put together a rebased version of the patch for cfbot.
Thanks for doing that, much appreciated! I was busy looking at other peoples
patches and hadn't gotten to my own yet =)
> On 13 Mar 2023, at 19:21, Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> 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.
Fixed.
> + 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.
Correct, this must've been a leftover from a previous coding that changed.
Removed.
> 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.
It it is pretty big for sure, but it's also IMHO not terribly complicated as
it's not really performing any hard to follow logic.
I have no issues refactoring it, but trying my hand at I was only making (what
I consider) less readable code by having to jump around so I consider it a
failure. If you have any suggestions, I would be more than happy to review and
incorporate those though.
Attached is a v5 with the above fixes and a pgindenting to fix up a few runaway
comments and indentations.
--
Daniel Gustafsson
| Attachment | Content-Type | Size |
|---|---|---|
| v5-0001-pg_upgrade-run-all-data-type-checks-per-connectio.patch | application/octet-stream | 37.1 KB |
| unknown_filename | text/plain | 2 bytes |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Daniel Gustafsson | 2023-07-06 16:10:28 | Re: Add SPLIT PARTITION/MERGE PARTITIONS commands |
| Previous Message | Andrew Dunstan | 2023-07-06 15:47:33 | Re: pgindent vs. pgperltidy command-line arguments |