Re: mismatching proargtypes/proallargtypes (bug #10122)

Hi,

'trinque' on irc reported that "SELECT pg_catalog.pg_identify_object(oid,
integer, integer)'::regprocedure" doesn't work and Andrew Gierth noticed
it's because proargtypes doesn't match proallargtypes.

Surpringsly there's no regression check testing for that. I recall
running into problems with mismatches there myself in the past. The
attached patch fixes the two wrong cases and adds a regression test to
prevent further occurances.
The broken functions are:
oid | oid | proargtypes | proallargtypes | filtered_allargtypes | proargmodes
------+-------------------------------------+-------------+------------------------+----------------------+-----------------
3078 | pg_sequence_parameters(oid) | {26} | {23,20,20,20,20,16} | {23} | {i,o,o,o,o,o}
3839 | pg_identify_object(oid,oid,integer) | {26,26,23} | {26,23,23,25,25,25,25} | {26,23,23} | {i,i,i,o,o,o,o}

sequence_parameters is wrong in 9.1+, identify_object in 9.3+.

Luckily neither is particularly important. It's really a pity that we
don't have infrastructure for fixing this like this :/.

Do we need a note in the release notes for fixing those functions?

I am sending this as a separate thread as the actual bugreport is
still stuck in moderation and I going to bed. Don't want somebody to
waste their time duplicating this.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> 'trinque' on irc reported that "SELECT pg_catalog.pg_identify_object(oid,
> integer, integer)'::regprocedure" doesn't work and Andrew Gierth noticed
> it's because proargtypes doesn't match proallargtypes.

Ugh, how annoying.

> Surpringsly there's no regression check testing for that.

Yeah, that's clearly an oversight :-(

> sequence_parameters is wrong in 9.1+, identify_object in 9.3+.
> Luckily neither is particularly important. It's really a pity that we
> don't have infrastructure for fixing this like this :/.
> Do we need a note in the release notes for fixing those functions?

I think it's probably non-critical, especially since it took this
long for anybody to notice. It's fortunate that the two bad cases
are just integer vs oid --- if the types weren't binary-compatible
then we'd have had bigger issues.

What I'm inclined to do in the back branches is just correct the
catalogs without bumping catversion (since we can't do that).
At least installations initdb'd in future will have it right.

regards, tom lane

Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > 'trinque' on irc reported that "SELECT pg_catalog.pg_identify_object(oid,
> > integer, integer)'::regprocedure" doesn't work and Andrew Gierth noticed
> > it's because proargtypes doesn't match proallargtypes.
>
> Ugh, how annoying.

Argh!

> What I'm inclined to do in the back branches is just correct the
> catalogs without bumping catversion (since we can't do that).
> At least installations initdb'd in future will have it right.

Maybe we can provide an UPDATE to fix it in existing installations, for
those interested in doing so.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Tom Lane wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>>> 'trinque' on irc reported that "SELECT pg_catalog.pg_identify_object(oid,
>>> integer, integer)'::regprocedure" doesn't work and Andrew Gierth noticed
>>> it's because proargtypes doesn't match proallargtypes.

>> Ugh, how annoying.

> Argh!

On reflection it's not quite as bad as it seems: the correct signature of
the function is pg_identify_object(oid,oid,integer) and the regprocedure
code does accept that. AFAICT the only readily visible effect of the bug
is that psql's \df lies about what the input argument datatypes are.

> Maybe we can provide an UPDATE to fix it in existing installations, for
> those interested in doing so.

Doubt it's worth the trouble really ...

regards, tom lane