On 06/21/2018 01:44 PM, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> On 06/21/2018 01:18 PM, Tom Lane wrote:
>>> I might be OK with a patch that converts *all* of pg_dump's cross-version
>>> difference handling code to depend on PQfnumber silently returning -1
>>> rather than failing, but I don't want to see it done like that in just
>>> one or two places.
>> I don't mind changing it. But please note that I wouldn't have done it
>> that way unless there were a precedent. I fully expected to add dummy
>> values to all the previous queries, but when I couldn't find attidentity
>> in them to put them next to I followed that example.
> Actually, now that I think about it, there is a concrete reason for the
> historical pattern: it provides a cross-check that you did not fat-finger
> the query, ie misspell the column alias vs the PQfnumber parameter. This
> gets more valuable the more per-version variants of the query there are.
> With the way the attidentity code does it, it would just silently act as
> though the column has its default value, which you might or might not
> notice in cursory testing. Getting visible bleats about column number -1
> is much more likely to get your attention.
>
> So I'm thinking that the attidentity code is just wrong, and you should
> change that too while you're at it.
>
>
That should be backpatched if changed, no? I don't think we'd want this
to get out of sync between the branches. It would make later
backpatching more difficult for one thing.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services