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(at)paquier(dot)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