Re: why can't plpgsql return a row-expression?

On Thu, Dec 6, 2012 at 12:31 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2012/12/5 Tom Lane <tgl@sss.pgh.pa.us>:
>> Asif Rehman <asifr.rehman@gmail.com> writes:
>>> Here is the updated patch. I overlooked the loop, checking to free the
>>> conversions map. Here are the results now.
>>
>> I looked at this patch briefly.  It seems to me to be completely
>> schizophrenic about the coercion rules.  This bit:
>>
>> -                       if (atttypid != att->atttypid ||
>> -                               (atttypmod != att->atttypmod && atttypmod >= 0))
>> +                       if ((atttypmod != att->atttypmod && atttypmod >= 0) ||
>> +                               !can_coerce_type(1, &atttypid, &(att->atttypid), COERCE_IMPLICIT_CAST))
>>
>> says that we'll allow column types to differ if there is an implicit
>> cast from the source to the target (or at least I think that's what's
>> intended, although it's got the source and target backwards).  Fine, but
>> then why don't we use the cast machinery to do the conversion?  This
>> is taking plpgsql's cowboy let's-coerce-via-IO-no-matter-whether-that's-
>> right-or-not approach and sticking it into a core part of the system.
>> There's no guarantee at all that applying typoutput then typinput
>> will match the conversion semantics you get from an actual cast, and
>> in many practical cases such as int4 to int8 it'll be drastically less
>> efficient anyway.  It would make more sense to do something similar to
>> coerce_record_to_complex(), that is modify the expression tree to
>> coerce the columns using the regular cast machinery.
>>
>> Also, the typmod part of the test seems completely broken.  For one
>> thing, comparing typmods isn't sane if the types themselves aren't
>> the same.  And it's quite unclear to me why we'd want to have an
>> anything-goes policy for type coercion, or even an implicit-casts-only
>> policy, but then insist that the typmods match exactly.  This coding
>> will allow varchar(42) to text, but not varchar(42) to varchar(43)
>> ... where's the sense in that?
>>
>> The patch also seems to go a great deal further than what was asked for
>> originally, or indeed is mentioned in the documentation patch, namely
>> fixing the restriction on RETURN to allow composite-typed expressions.
>> Specifically it's changing the code that accepts composite input
>> arguments.  Do we actually want that?  If so, shouldn't it be
>> documented?
>>
>> I'm inclined to suggest that we drop all the coercion stuff and just
>> do what Robert actually asked for originally, which was the mere
>> ability to return a composite value as long as it matched the function's
>> result type.  I'm not convinced that we want automatic implicit type
>> coercions here.  In any case I'm very much against sticking such a thing
>> into general-purpose support code like tupconvert.c.  That will create a
>> strong likelihood that plpgsql's poorly-designed coercion semantics will
>> leak into other aspects of the system.
>
> I think so without some change of coercion this patch is not too
> useful because very simply test fail
>
> create type foo(a int, b text);
>
> create or replace function foo_func()
> returns foo as $$
> begin
>   ...
>   return (10, 'hello');
>
> end;
>
> but we can limit a implicit coercion in tupconvert via new parameter -
> because we would to forward plpgsql behave just from this direction.
> Then when this parameter - maybe "allowIOCoercion" will be false, then
> tupconvert will have same behave like before.

It would be nice to make that work, but it could be left for a
separate patch, I suppose.  I'm OK with Tom's proposal to go ahead and
commit the core mechanic first, if doing more than that is
controversial.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company