Re: pg_dump throwing "column number -1 is out of range 0..36" on HEAD

Hi,

On 2019-05-22 09:46:19 -0400, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> > Trying to do pg_dump[all] on a 9.5 or older server results in spurious
> > failures:
> > pg_dump: column number -1 is out of range 0..36
> 
> > After looking around, the problem comes from
> > check_tuple_field_number(), more specifically from getTables() where
> > someone has forgotten to add NULL values for amname when querying
> > older server versions.

Thanks for catching!


> > Attached is a patch to fix that.

Wouldn't the better fix be to change

        if (PQgetisnull(res, i, i_amname))
            tblinfo[i].amname = NULL;

into

        if (i_amname == -1 || PQgetisnull(res, i, i_amname))
            tblinfo[i].amname = NULL;

it's much more scalable than adding useless columns everywhere, and we
already use that approach with i_checkoption (and at a number of other
places).


> > Attached is a patch to fix that.  I am not seeing other failures with
> > an instance that includes all the contents of installcheck, so it
> > seems that the rest is fine.
> 
> Looks like the right fix.  I'm sad that the buildfarm did not catch
> this ... why wouldn't the cross-version-upgrade tests have seen it?

I suspect we just didn't notice that it saw that:

    if (field_num < 0 || field_num >= res->numAttributes)
    {
        pqInternalNotice(&res->noticeHooks,
                         "column number %d is out of range 0..%d",
                         field_num, res->numAttributes - 1);
        return false;
    }

as it's just a notice, not a failure.

Greetings,

Andres Freund