Skip site navigation (1) Skip section navigation (2)

Re: WIP: pl/pgsql cleanup

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: pgsql-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: WIP: pl/pgsql cleanup
Date: 2005-02-18 22:22:37
Message-ID: 791.1108765357@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-patches
Neil Conway <neilc(at)samurai(dot)com> writes:
> Attached is a revised patch that incorporates your suggestions. Sorry
> for the delay in posting this.

Still a few bricks shy of a load I fear:

regression=# create or replace function foo() returns int language plpgsql as $$
regression$# begin
regression$#   return ;
regression$# end$$;
CREATE FUNCTION
regression=# select foo();
server closed the connection unexpectedly

If you're going to stick a NULL into the return's expression then you'd
better check for same when it's used.

regression=# create or replace function foo() returns int language plpgsql as $$
regression$# begin
regression$#   for x in
regression$# select 33
regression$# loop
regression$# end loop;
regression$# end$$;
server closed the connection unexpectedly

This one is from the Assert at line 966 failing:

TRAP: FailedAssertion("!(strcmp(expr1->query, "SELECT ") == 0)", File: "gram.y", Line: 966)

What I was actually intending to check when I ran into that is why some
of the error paths in your FOR-loop rewrite use
	plpgsql_error_lineno = $1;
while others have
	plpgsql_error_lineno = plpgsql_scanner_lineno();
I suspect the former is more appropriate, at least for errors that
reference the FOR variable and not some other part of the statement.
Also why the inconsistent use of yyerror() vs ereport()?

One really minor quibble:

***************
*** 524,531 ****
                          char    *name;
  
                          plpgsql_convert_ident(yytext, &name, 1);
!                         /* name should be malloc'd for use as varname */
!                         $$.name = strdup(name);
                          $$.lineno  = plpgsql_scanner_lineno();
                          pfree(name);
                      }
--- 496,502 ----
                          char    *name;
  
                          plpgsql_convert_ident(yytext, &name, 1);
!                         $$.name = pstrdup(name);
                          $$.lineno  = plpgsql_scanner_lineno();
                          pfree(name);
                      }

The pstrdup and pfree could be canceled out.  (This seems to be the only
call of plpgsql_convert_ident where you missed this.)

			regards, tom lane

In response to

Responses

pgsql-patches by date

Next:From: Bruce MomjianDate: 2005-02-19 03:35:07
Subject: Re: UTF8 or Unicode
Previous:From: Dave PageDate: 2005-02-18 12:49:36
Subject: Re: UTF8 or Unicode

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group