Thread

  1. CREATE FUNCTION broken

    Jan Wieck <jwieck@debis.com> — 1998-02-13T11:41:14Z

    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
      	{
    
    
  2. Re: [HACKERS] CREATE FUNCTION broken

    Thomas Lockhart <lockhart@alumni.caltech.edu> — 1998-02-13T13:50:04Z

    >     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
    
    
    
  3. Re: [HACKERS] CREATE FUNCTION broken

    Jan Wieck <jwieck@debis.com> — 1998-02-13T15:34:51Z

    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) #
    
    
    
    
  4. Re: [HACKERS] CREATE FUNCTION broken

    Thomas Lockhart <lockhart@alumni.caltech.edu> — 1998-02-13T16:16:52Z

    > > >     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
    
    
    
  5. Re: [HACKERS] CREATE FUNCTION broken

    Jan Wieck <jwieck@debis.com> — 1998-02-13T16:53:44Z

    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) #
    
    
    
    
  6. Re: [HACKERS] CREATE FUNCTION broken

    Bruce Momjian <maillist@candle.pha.pa.us> — 1998-02-13T18:28:07Z

    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
    
    
  7. Re: [HACKERS] CREATE FUNCTION broken

    Vadim B. Mikheev <vadim@sable.krasnoyarsk.su> — 1998-02-15T09:26:40Z

    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