Thread: CREATE FUNCTION broken
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
{
> 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