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

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Asif Rehman <asifr(dot)rehman(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: why can't plpgsql return a row-expression?
Date: 2012-12-06 05:31:16
Message-ID: CAFj8pRB7p4TXP-q1zA2DzCAxwM-pMdikwxpZ3vYmCoDsyrehuA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2012/12/5 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Asif Rehman <asifr(dot)rehman(at)gmail(dot)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.

Regards

Pavel

>
> regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Vlad Arkhipov 2012-12-06 08:58:22 How to check whether the row was modified by this transaction before?
Previous Message Alvaro Herrera 2012-12-06 04:59:31 Re: Review: Extra Daemons / bgworker