From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andrew Dunstan <andrew(at)dunslane(dot)net> |
Subject: | Re: pg_dump throwing "column number -1 is out of range 0..36" on HEAD |
Date: | 2019-05-22 18:17:41 |
Message-ID: | 7594.1558549061@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andres Freund <andres(at)anarazel(dot)de> writes:
> 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).
FWIW, I think that's a pretty awful idea, and the fact that some
people have had it before doesn't make it less awful. It's giving
up the ability to detect errors-of-omission, which might easily
be harmful rather than harmless errors.
It does seem like we're overdue to rethink how pg_dump handles
cross-version query differences ... but inconsistently lobotomizing
its internal error detection is not a good start on that.
>> 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:
> as it's just a notice, not a failure.
Hm. But shouldn't we have gotten garbage output from the pg_dump run?
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2019-05-22 18:20:27 | Re: Teach pg_upgrade test to honor NO_TEMP_INSTALL |
Previous Message | Tom Lane | 2019-05-22 18:06:47 | Re: Teach pg_upgrade test to honor NO_TEMP_INSTALL |