public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Neil Conway <[email protected]>
Cc: [email protected]
Subject: Re: small pg_dump code cleanup
Date: Wed, 5 Jun 2024 11:37:54 -0500
Message-ID: <ZmCUYtIhD5wEQR4s@nathan> (raw)
In-Reply-To: <CAOW5sYZY3sdSxzaRwQCMMNtsZ_1jN+AFzB82k58==Vome-G11w@mail.gmail.com>
References: <ZmCAtVaOrHpf31PJ@nathan>
<CAOW5sYZY3sdSxzaRwQCMMNtsZ_1jN+AFzB82k58==Vome-G11w@mail.gmail.com>
On Wed, Jun 05, 2024 at 12:22:03PM -0400, Neil Conway wrote:
> Nice cleanup! Two minor comments:
Thanks for taking a look.
> (1) Names like `getXXX` for these functions suggest to me that they return
> a value, rather than side-effecting. I realize some variants continue to
> return a value, but the majority no longer do. Perhaps a name like
> lookupXXX() or readXXX() would be clearer?
What about collectXXX() to match similar functions in pg_dump.c (e.g.,
collectRoleNames(), collectComments(), collectSecLabels())?
> (2) These functions malloc() a single ntups * sizeof(struct) allocation and
> then index into it to fill-in each struct before entering it into the hash
> table. It might be more straightforward to just malloc each individual
> struct.
That'd increase the number of allocations quite significantly, but I'd be
surprised if that was noticeable outside of extreme scenarios. At the
moment, I'm inclined to leave these as-is for this reason and because I
doubt it'd result in much cleanup, but I'll yield to the majority opinion
here.
--
nathan
view thread (7+ 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]
Subject: Re: small pg_dump code cleanup
In-Reply-To: <ZmCUYtIhD5wEQR4s@nathan>
* 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