Modernize plpgsql's handling of parse locations, making it look a lot more

like the core parser's code.  In particular, track locations at the character
rather than line level during parsing, allowing many more parse-time error
conditions to be reported with precise error pointers rather than just
"near line N".

Also, exploit the fact that we no longer need to substitute $N for variable
references by making extracted SQL queries and expressions be exact copies
of subranges of the function text, rather than having random whitespace
changes within them.  This makes it possible to directly map parse error
positions from the core parser onto positions in the function text, which
lets us report them without the previous kluge of showing the intermediate
internal-query form.  (Later it might be good to do that for core
parse-analysis errors too, but this patch is just touching plpgsql's
lexer/parser, not what happens at runtime.)

In passing, make plpgsql's lexer use palloc not malloc.

These changes make plpgsql's parse-time error reports noticeably nicer
(as illustrated by the regression test changes), and will also simplify
the planned removal of plpgsql's separate lexer by reducing the impedance
mismatch between what it does and what the core lexer does.

src/pl/plpgsql/src/pl_comp.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.142 2009/11/07 00:52:26 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.143 2009/11/09 00:26:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -46,7 +46,6 @@ int			plpgsql_nDatums;
 PLpgSQL_datum **plpgsql_Datums;
 static int	datums_last = 0;
 
-int			plpgsql_error_lineno;
 char	   *plpgsql_error_funcname;
 bool		plpgsql_DumpExecTree = false;
 bool		plpgsql_check_syntax = false;
@@ -95,6 +94,7 @@ static PLpgSQL_function *do_compile(FunctionCallInfo fcinfo,
 		   PLpgSQL_function *function,
 		   PLpgSQL_func_hashkey *hashkey,
 		   bool forValidator);
+static void plpgsql_compile_error_callback(void *arg);
 static void add_dummy_return(PLpgSQL_function *function);
 static Node *plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref);
 static Node *plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var);
@@ -301,7 +301,6 @@ do_compile(FunctionCallInfo fcinfo,
 	plpgsql_scanner_init(proc_source);
 
 	plpgsql_error_funcname = pstrdup(NameStr(procStruct->proname));
-	plpgsql_error_lineno = 0;
 
 	/*
 	 * Setup error traceback support for ereport()
@@ -713,7 +712,6 @@ do_compile(FunctionCallInfo fcinfo,
 	 */
 	error_context_stack = plerrcontext.previous;
 	plpgsql_error_funcname = NULL;
-	plpgsql_error_lineno = 0;
 
 	plpgsql_check_syntax = false;
 
@@ -752,7 +750,6 @@ plpgsql_compile_inline(char *proc_source)
 	plpgsql_scanner_init(proc_source);
 
 	plpgsql_error_funcname = func_name;
-	plpgsql_error_lineno = 0;
 
 	/*
 	 * Setup error traceback support for ereport()
@@ -851,7 +848,6 @@ plpgsql_compile_inline(char *proc_source)
 	 */
 	error_context_stack = plerrcontext.previous;
 	plpgsql_error_funcname = NULL;
-	plpgsql_error_lineno = 0;
 
 	plpgsql_check_syntax = false;
 
@@ -865,10 +861,8 @@ plpgsql_compile_inline(char *proc_source)
  * error context callback to let us supply a call-stack traceback.
  * If we are validating or executing an anonymous code block, the function
  * source text is passed as an argument.
- *
- * This function is public only for the sake of an assertion in gram.y
  */
-void
+static void
 plpgsql_compile_error_callback(void *arg)
 {
 	if (arg)
@@ -888,7 +882,7 @@ plpgsql_compile_error_callback(void *arg)
 
 	if (plpgsql_error_funcname)
 		errcontext("compilation of PL/pgSQL function \"%s\" near line %d",
-				   plpgsql_error_funcname, plpgsql_error_lineno);
+				   plpgsql_error_funcname, plpgsql_latest_lineno());
 }
 
 
@@ -2065,25 +2059,6 @@ build_row_from_vars(PLpgSQL_variable **vars, int numvars)
 	return row;
 }
 
-
-/* ----------
- * plpgsql_parse_datatype			Scanner found something that should
- *					be a datatype name.
- * ----------
- */
-PLpgSQL_type *
-plpgsql_parse_datatype(const char *string)
-{
-	Oid			type_id;
-	int32		typmod;
-
-	/* Let the main parser try to parse it under standard SQL rules */
-	parseTypeString(string, &type_id, &typmod);
-
-	/* Okay, build a PLpgSQL_type data structure for it */
-	return plpgsql_build_datatype(type_id, typmod);
-}
-
 /*
  * plpgsql_build_datatype
  *		Build PLpgSQL_type struct given type OID and typmod.

src/pl/plpgsql/src/pl_exec.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.250 2009/11/06 18:37:54 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_exec.c,v 1.251 2009/11/09 00:26:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -23,7 +23,6 @@
 #include "catalog/pg_type.h"
 #include "executor/spi_priv.h"
 #include "funcapi.h"
-#include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "nodes/nodeFuncs.h"
 #include "parser/scansup.h"

src/pl/plpgsql/src/plpgsql.h

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.121 2009/11/07 00:52:26 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.122 2009/11/09 00:26:55 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -22,6 +22,7 @@
 #include "fmgr.h"
 #include "commands/trigger.h"
 #include "executor/spi.h"
+#include "lib/stringinfo.h"
 #include "nodes/bitmapset.h"
 #include "utils/tuplestore.h"
 
@@ -774,11 +775,9 @@ typedef struct
 
 extern bool plpgsql_DumpExecTree;
 extern bool plpgsql_LookupIdentifiers;
-extern bool plpgsql_SpaceScanned;
 extern int	plpgsql_nDatums;
 extern PLpgSQL_datum **plpgsql_Datums;
 
-extern int	plpgsql_error_lineno;
 extern char *plpgsql_error_funcname;
 
 /* linkage to the real yytext variable */
@@ -813,7 +812,6 @@ extern PLpgSQL_type *plpgsql_parse_dblwordtype(const char *word);
 extern PLpgSQL_type *plpgsql_parse_tripwordtype(const char *word);
 extern PLpgSQL_type *plpgsql_parse_wordrowtype(const char *word);
 extern PLpgSQL_type *plpgsql_parse_dblwordrowtype(const char *word);
-extern PLpgSQL_type *plpgsql_parse_datatype(const char *string);
 extern PLpgSQL_type *plpgsql_build_datatype(Oid typeOid, int32 typmod);
 extern PLpgSQL_variable *plpgsql_build_variable(const char *refname, int lineno,
 					   PLpgSQL_type *dtype,
@@ -826,7 +824,6 @@ extern PLpgSQL_condition *plpgsql_parse_err_condition(char *condname);
 extern void plpgsql_adddatum(PLpgSQL_datum *new);
 extern int	plpgsql_add_initdatums(int **varnos);
 extern void plpgsql_HashTableInit(void);
-extern void plpgsql_compile_error_callback(void *arg);
 
 /* ----------
  * Functions in pl_handler.c
@@ -885,8 +882,12 @@ extern int	plpgsql_yyparse(void);
 extern int	plpgsql_base_yylex(void);
 extern int	plpgsql_yylex(void);
 extern void plpgsql_push_back_token(int token);
+extern void plpgsql_append_source_text(StringInfo buf,
+									   int startlocation, int endlocation);
+extern int	plpgsql_scanner_errposition(int location);
 extern void plpgsql_yyerror(const char *message);
-extern int	plpgsql_scanner_lineno(void);
+extern int	plpgsql_location_to_lineno(int location);
+extern int	plpgsql_latest_lineno(void);
 extern void plpgsql_scanner_init(const char *str);
 extern void plpgsql_scanner_finish(void);
 

src/test/regress/expected/plpgsql.out

@@ -1747,7 +1747,7 @@ create function f1(in i int, out j int) returns int as $$
 begin
   return i+1;
 end$$ language plpgsql;
-ERROR:  RETURN cannot have a parameter in function with OUT parameters at or near "i"
+ERROR:  RETURN cannot have a parameter in function with OUT parameters
 LINE 3:   return i+1;
                  ^
 create function f1(in i int, out j int) as $$
@@ -2066,13 +2066,13 @@ begin
 end$$ language plpgsql;
 select test_variable_storage();
 NOTICE:  should see this
-CONTEXT:  SQL statement "SELECT  trap_zero_divide(-100)"
+CONTEXT:  SQL statement "SELECT trap_zero_divide(-100)"
 PL/pgSQL function "test_variable_storage" line 7 at PERFORM
 NOTICE:  should see this only if -100 <> 0
-CONTEXT:  SQL statement "SELECT  trap_zero_divide(-100)"
+CONTEXT:  SQL statement "SELECT trap_zero_divide(-100)"
 PL/pgSQL function "test_variable_storage" line 7 at PERFORM
 NOTICE:  should see this only if -100 fits in smallint
-CONTEXT:  SQL statement "SELECT  trap_zero_divide(-100)"
+CONTEXT:  SQL statement "SELECT trap_zero_divide(-100)"
 PL/pgSQL function "test_variable_storage" line 7 at PERFORM
  test_variable_storage 
 -----------------------
@@ -2325,10 +2325,8 @@ begin
     return a;
 end$$ language plpgsql;
 ERROR:  syntax error at or near "Johnny"
-LINE 1: Johnny Yuma
-        ^
-QUERY:  Johnny Yuma
-CONTEXT:  SQL statement in PL/PgSQL function "bad_sql1" near line 4
+LINE 5:     Johnny Yuma;
+            ^
 create function bad_sql2() returns int as $$
 declare r record;
 begin
@@ -2338,26 +2336,22 @@ begin
     return 5;
 end;$$ language plpgsql;
 ERROR:  syntax error at or near "the"
-LINE 1:  select I fought the law, the law won
-                         ^
-QUERY:   select I fought the law, the law won
-CONTEXT:  SQL statement in PL/PgSQL function "bad_sql2" near line 3
+LINE 4:     for r in select I fought the law, the law won LOOP
+                                     ^
 -- a RETURN expression is mandatory, except for void-returning
 -- functions, where it is not allowed
 create function missing_return_expr() returns int as $$
 begin
     return ;
 end;$$ language plpgsql;
-ERROR:  syntax error at end of input
-LINE 1: SELECT 
-               ^
-QUERY:  SELECT 
-CONTEXT:  SQL statement in PL/PgSQL function "missing_return_expr" near line 2
+ERROR:  missing expression at or near ";"
+LINE 3:     return ;
+                   ^
 create function void_return_expr() returns void as $$
 begin
     return 5;
 end;$$ language plpgsql;
-ERROR:  RETURN cannot have a parameter in function returning void at or near "5"
+ERROR:  RETURN cannot have a parameter in function returning void
 LINE 3:     return 5;
                    ^
 -- VOID functions are allowed to omit RETURN
@@ -2427,9 +2421,9 @@ end; $$ language plpgsql;
 -- blocks
 select excpt_test1();
 ERROR:  column "sqlstate" does not exist
-LINE 1: SELECT  sqlstate
-                ^
-QUERY:  SELECT  sqlstate
+LINE 1: SELECT sqlstate
+               ^
+QUERY:  SELECT sqlstate
 CONTEXT:  PL/pgSQL function "excpt_test1" line 2 at RAISE
 create function excpt_test2() returns void as $$
 begin
@@ -2442,9 +2436,9 @@ end; $$ language plpgsql;
 -- should fail
 select excpt_test2();
 ERROR:  column "sqlstate" does not exist
-LINE 1: SELECT  sqlstate
-                ^
-QUERY:  SELECT  sqlstate
+LINE 1: SELECT sqlstate
+               ^
+QUERY:  SELECT sqlstate
 CONTEXT:  PL/pgSQL function "excpt_test2" line 4 at RAISE
 create function excpt_test3() returns void as $$
 begin
@@ -2714,7 +2708,8 @@ begin
 end;
 $$ language plpgsql;
 ERROR:  end label "outer_label" differs from block's label "inner_label"
-CONTEXT:  compilation of PL/pgSQL function "end_label3" near line 6
+LINE 7:   end loop outer_label;
+                   ^
 -- should fail: end label on a block without a start label
 create function end_label4() returns void as $$
 <<outer_label>>
@@ -2725,7 +2720,8 @@ begin
 end;
 $$ language plpgsql;
 ERROR:  end label "outer_label" specified for unlabelled block
-CONTEXT:  compilation of PL/pgSQL function "end_label4" near line 5
+LINE 6:   end loop outer_label;
+                   ^
 -- using list of scalars in fori and fore stmts
 create function for_vect() returns void as $proc$
 <<lbl>>declare a integer; b varchar; c varchar; r record;
@@ -3308,7 +3304,8 @@ begin
 end;
 $$ language plpgsql;
 ERROR:  cursor FOR loop must use a bound cursor variable
-CONTEXT:  compilation of PL/pgSQL function "forc_bad" near line 4
+LINE 5:   for r in c loop
+                   ^
 -- test RETURN QUERY EXECUTE
 create or replace function return_dquery()
 returns setof int as $$
@@ -3839,21 +3836,20 @@ begin
 end
 $$ language plpgsql;
 WARNING:  nonstandard use of \\ in a string literal
+LINE 3:   raise notice 'foo\\bar\041baz';
+                       ^
 HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
-CONTEXT:  string literal in PL/PgSQL function "strtest" near line 2
 WARNING:  nonstandard use of \\ in a string literal
-LINE 1: SELECT  'foo\\bar\041baz'
-                ^
+LINE 4:   return 'foo\\bar\041baz';
+                 ^
 HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
-QUERY:  SELECT  'foo\\bar\041baz'
-CONTEXT:  SQL statement in PL/PgSQL function "strtest" near line 3
 select strtest();
 NOTICE:  foo\bar!baz
 WARNING:  nonstandard use of \\ in a string literal
-LINE 1: SELECT  'foo\\bar\041baz'
-                ^
+LINE 1: SELECT 'foo\\bar\041baz'
+               ^
 HINT:  Use the escape string syntax for backslashes, e.g., E'\\'.
-QUERY:  SELECT  'foo\\bar\041baz'
+QUERY:  SELECT 'foo\\bar\041baz'
 CONTEXT:  PL/pgSQL function "strtest" line 3 at RETURN
    strtest   
 -------------
@@ -3922,7 +3918,7 @@ NOTICE:  105, Office
 NOTICE:  106, Office
 -- these are to check syntax error reporting
 DO LANGUAGE plpgsql $$begin return 1; end$$;
-ERROR:  RETURN cannot have a parameter in function returning void at or near "1"
+ERROR:  RETURN cannot have a parameter in function returning void
 LINE 1: DO LANGUAGE plpgsql $$begin return 1; end$$;
                                            ^
 DO LANGUAGE plpgsql $$
@@ -3934,9 +3930,9 @@ BEGIN
     END LOOP;
 END$$;
 ERROR:  column "foo" does not exist
-LINE 1:  SELECT rtrim(roomno) AS roomno, foo FROM Room ORDER BY room...
-                                         ^
-QUERY:   SELECT rtrim(roomno) AS roomno, foo FROM Room ORDER BY roomno
+LINE 1: SELECT rtrim(roomno) AS roomno, foo FROM Room ORDER BY roomn...
+                                        ^
+QUERY:  SELECT rtrim(roomno) AS roomno, foo FROM Room ORDER BY roomno
 CONTEXT:  PL/pgSQL function "inline_code_block" line 3 at FOR over SELECT rows
 -- Check variable scoping -- a var is not available in its own or prior
 -- default expressions.