Re: Improvements in pg_dump/pg_restore toc format and performances

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