Re: Improvements in pg_dump/pg_restore toc format and performances - Mailing list pgsql-hackers

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