public inbox for [email protected]  

help / color / mirror / Atom feed

From: Nathan Bossart <[email protected]>
To: Pierre Ducroquet <[email protected]>
Cc: [email protected]
Subject: Re: Improvements in pg_dump/pg_restore toc format and performances
Date: Wed, 20 Sep 2023 13:29:22 -0700
Message-ID: <20230920202922.GA3054233@nathanxps13> (raw)
In-Reply-To: <2575263.Lt9SDvczpP@peanuts2>
References: <2656000.KRxA6XjA2N@peanuts2>
	<20230918215247.GA2661288@nathanxps13>
	<20230918215442.GB2661288@nathanxps13>
	<2575263.Lt9SDvczpP@peanuts2>

On Tue, Sep 19, 2023 at 12:15:55PM +0200, Pierre Ducroquet wrote:
> Attached updated patches fix this regression, I'm sorry I missed that.

Thanks for the new patches.  0001 and 0002 look reasonable to me.  This is
a nitpick, but we might want to use atooid() in 0002, which is just
shorthand for the strtoul() call you are using.

> -		WriteStr(AH, NULL);		/* Terminate List */
> +		WriteInt(AH, -1);		/* Terminate List */

I think we need to be cautious about using WriteInt and ReadInt here.  OIDs
are unsigned, so we probably want to use InvalidOid (0) instead.

> +				if (AH->version >= K_VERS_1_16)
>  				{
> -					depSize *= 2;
> -					deps = (DumpId *) pg_realloc(deps, sizeof(DumpId) * depSize);
> +					depId = ReadInt(AH);
> +					if (depId == -1)
> +						break;		/* end of list */
> +					if (depIdx >= depSize)
> +					{
> +						depSize *= 2;
> +						deps = (DumpId *) pg_realloc(deps, sizeof(DumpId) * depSize);
> +					}
> +					deps[depIdx] = depId;
> +				}
> +				else
> +				{
> +					tmp = ReadStr(AH);
> +					if (!tmp)
> +						break;		/* end of list */
> +					if (depIdx >= depSize)
> +					{
> +						depSize *= 2;
> +						deps = (DumpId *) pg_realloc(deps, sizeof(DumpId) * depSize);
> +					}
> +					deps[depIdx] = strtoul(tmp, NULL, 10);
> +					free(tmp);
>  				}

I would suggest making the resizing logic common.

	if (AH->version >= K_VERS_1_16)
	{
		depId = ReadInt(AH);
		if (depId == InvalidOid)
			break;		/* end of list */
	}
	else
	{
		tmp = ReadStr(AH);
		if (!tmp)
			break;		/* end of list */
		depId = strtoul(tmp, NULL, 10);
		free(tmp);
	}

	if (depIdx >= depSize)
	{
		depSize *= 2;
		deps = (DumpId *) pg_realloc(deps, sizeof(DumpId) * depSize);
	}
	deps[depIdx] = depId;

Also, can we make depId more locally scoped?

I have yet to look into 0004 still.

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

view thread (6+ 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: Improvements in pg_dump/pg_restore toc format and performances
  In-Reply-To: <20230920202922.GA3054233@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