CREATE FUNCTION broken

Someone changed the parser to build a TypeName node on CREATE
FUNCTION in any case. As a side effect, ALL! functions
created got the proretset attribute to true. Thus for a
SELECT the parser wrapped an Iter node around the Expr and
since singleton functions set isDone the Iter returns no
tuple up.

Ah. I broke it (though the regression tests did not find the problem). What
I changed was the code in gram.y, which used to just create a string node
for the return type clause _unless_ the return type was a "SETOF type". In
that case a Typename node was created, and the setof attribute was
explicitly set.

What you found is that farther along, the setof attribute was forced to be
true if _any_ Typename node is present.

It looks like your patch will completely fix things, and is better than my
reverting the gram.y code. Can you suggest a small test case to include in
the regression suite?

Unless there are objections from others (with a preference for reverting
the gram.y code) I'll go ahead and apply Jan's patch.

- Tom

Tom wrote:

Someone changed the parser to build a TypeName node on CREATE
FUNCTION in any case. As a side effect, ALL! functions
created got the proretset attribute to true. Thus for a
SELECT the parser wrapped an Iter node around the Expr and
since singleton functions set isDone the Iter returns no
tuple up.

Ah. I broke it (though the regression tests did not find the problem). What
I changed was the code in gram.y, which used to just create a string node
for the return type clause _unless_ the return type was a "SETOF type". In
that case a Typename node was created, and the setof attribute was
explicitly set.

What you found is that farther along, the setof attribute was forced to be
true if _any_ Typename node is present.

Haven't spend time to analyze if other places might have been
affected by that - just thought we should trust the parser
about the SETOF flag in TypeName node instead of knowing it
better deep down in the utility commands.

;-)

It looks like your patch will completely fix things, and is better than my
reverting the gram.y code. Can you suggest a small test case to include in
the regression suite?

Small test case - hmmm.

The regression tests found it - but you wouldn't expect it
there. It's in the trigger test, where at some places SELECT
set_ttdummy(0) returns 0 columns instead of one.

Anyway - add a little function in regress.c returning a
basetype value. Then add tests that use it in SELECT queries.

int32
allways_one()
{
return 1;
}

SELECT allways_one() AS one;

SELECT a, allways_one() AS one FROM t;

Unless there are objections from others (with a preference for reverting
the gram.y code) I'll go ahead and apply Jan's patch.

Even if reverting the gram.y code - my patch could only make
things better.

Until later, Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

Someone changed the parser to build a TypeName node on CREATE
FUNCTION in any case. As a side effect, ALL! functions
created got the proretset attribute to true. Thus for a
SELECT the parser wrapped an Iter node around the Expr and
since singleton functions set isDone the Iter returns no
tuple up.

It looks like your patch will completely fix things, and is better than my
reverting the gram.y code. Can you suggest a small test case to include in
the regression suite?

The regression tests found it - but you wouldn't expect it
there. It's in the trigger test, where at some places SELECT
set_ttdummy(0) returns 0 columns instead of one.

Ah! This might be all of the problem with the trigger regression test then? I
had wanted Vadim to look at it because I wasn't sure what the behavior should
be. Does this test look good to you now?

Even if reverting the gram.y code - my patch could only make
things better.

Yes, and scrappy already applied it :)

- Tom

Tom wrote:

Someone changed the parser to build a TypeName node on CREATE
FUNCTION in any case. As a side effect, ALL! functions
created got the proretset attribute to true. Thus for a
SELECT the parser wrapped an Iter node around the Expr and
since singleton functions set isDone the Iter returns no
tuple up.

It looks like your patch will completely fix things, and is better than my
reverting the gram.y code. Can you suggest a small test case to include in
the regression suite?

The regression tests found it - but you wouldn't expect it
there. It's in the trigger test, where at some places SELECT
set_ttdummy(0) returns 0 columns instead of one.

Ah! This might be all of the problem with the trigger regression test then? I
had wanted Vadim to look at it because I wasn't sure what the behavior should
be. Does this test look good to you now?

Looks better now. In some places the triggers.sql has
comments that an error is expected. And these errors now
happen :-)

But for the different NOTICE messages I get I'm not sure too.
Who's the one who created the trigger test's (Vadim)? Could
that guy please have a look at the results now and update the
expected/triggers.out to what's really expected?

In addition to the triggers I get a failed on the float8
tests too. The test in question is

SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;

====== float8 ======
200c200,208
< ERROR: pow() result is out of range
---

bad|?column?
---+--------
|0
|NaN
|NaN
|NaN
|NaN
(5 rows)

Content of table is

QUERY: SELECT '' AS five, FLOAT8_TBL.*;
five|f1
----+--------------------
|0
|1004.3
|-34.84
|1.2345678901234e+200
|1.2345678901234e-200
(5 rows)

What's correct on the overflow - NaN or ERROR?

Even if reverting the gram.y code - my patch could only make
things better.

Yes, and scrappy already applied it :)

- Tom

Until later, Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

I added typmod to the TypeName structure, but am not aware of adding any
TypeName structure instance to anything.

Nice you have a patch for us.

Hi,

Someone changed the parser to build a TypeName node on CREATE
FUNCTION in any case. As a side effect, ALL! functions
created got the proretset attribute to true. Thus for a
SELECT the parser wrapped an Iter node around the Expr and
since singleton functions set isDone the Iter returns no
tuple up.

Until later, Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#======================================== jwieck@debis.com (Jan Wieck) #

*** define.c.orig	Fri Feb 13 12:14:17 1998
--- define.c	Fri Feb 13 12:14:38 1998
***************
*** 94,100 ****
TypeName   *setType = (TypeName *) returnType;

*prorettype_p = setType->name;
! 		*returnsSet_p = true;
}
else
{
--- 94,100 ----
TypeName   *setType = (TypeName *) returnType;

*prorettype_p = setType->name;
! *returnsSet_p = setType->setof;
}
else
{

--
Bruce Momjian
maillist@candle.pha.pa.us

Jan Wieck wrote:

Tom wrote:

Ah! This might be all of the problem with the trigger regression test then? I
had wanted Vadim to look at it because I wasn't sure what the behavior should
be. Does this test look good to you now?

Looks better now. In some places the triggers.sql has
comments that an error is expected. And these errors now
happen :-)

But for the different NOTICE messages I get I'm not sure too.
Who's the one who created the trigger test's (Vadim)? Could
that guy please have a look at the results now and update the
expected/triggers.out to what's really expected?

I'll take a look...

Vadim