[HACKERS] Another problem with result type selection in inline_function

2007-05-01 Thread Tom Lane
I just realized that there's a problem with the patch I applied here
http://archives.postgresql.org/pgsql-committers/2007-03/msg00057.php
to ensure that the result type of an inline'd SQL function is correctly
marked in the resulting expression tree.  To wit, it doesn't work for
functions returning pseudotypes:

regression=# create function if(bool,anyelement,anyelement) returns anyelement
regression-# as $$ select case when $1 then $2 else $3 end $$ language sql;
CREATE FUNCTION
regression=# explain verbose select if(true,1,2) from int4_tbl;
server closed the connection unexpectedly

The crash is because this newly-added assert fails:

Assert(IsBinaryCoercible(exprType(newexpr), funcform-prorettype));

and beyond that, we don't actually want the other added line (adding a
RelabelType node) either, because it'd be relabeling the expression as
type ANYELEMENT which is entirely unhelpful.

I think the correct thing is to do nothing, and assume the expanded
expression must have the right type already, if the function is declared
to return a pseudotype.  The only pseudotypes allowed as SQL-function
results are RECORD, VOID, and polymorphic, and this seems OK and maybe
even required in each case.  But having gotten this wrong once already,
maybe I better call for comments...

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [HACKERS] Another problem with result type selection in inline_function

2007-05-01 Thread Tom Lane
I wrote:
 I think the correct thing is to do nothing, and assume the expanded
 expression must have the right type already, if the function is declared
 to return a pseudotype.  The only pseudotypes allowed as SQL-function
 results are RECORD, VOID, and polymorphic, and this seems OK and maybe
 even required in each case.  But having gotten this wrong once already,
 maybe I better call for comments...

Make that 0 for 2 :-(.  On closer inspection the correct patch seems to
be just to use result_type, ie the result type the function call node
was already labeled with, not funcform-prorettype (the function's
declared result type).  This can be seen to be correct from two
arguments:

1. The whole point of the RelabelType insertion is to ensure that the
exposed type of the expression tree (as reported by exprType say)
remains the same as before.  And result_type is exactly what it was
before.

2. result_type, not prorettype, is in fact what check_sql_fn_retval()
was checking against.  That Assert was intended to back up that we were
in sync with check_sql_fn_retval(), but we weren't.

So this is just a pure thinko in the previous patch.  Sigh.

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq