|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||Asif Rehman <asifr(dot)rehman(at)gmail(dot)com>|
|Cc:||Pavel Stehule <pavel(dot)stehule(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?|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
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.
regards, tom lane
|Next Message||Dimitri Fontaine||2012-12-05 20:49:11||Re: Dumping an Extension's Script|
|Previous Message||Alvaro Herrera||2012-12-05 20:43:56||Re: Dumping an Extension's Script|