v2-0002-handle-statement-locations-better.patch

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b1d4642c59..4d5f197a1c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -67,39 +67,23 @@
 
 
 /*
- * Location tracking support --- simpler than bison's default, since we only
- * want to track the start position not the end position of each nonterminal.
+ * Location tracking support.  Unlike bison's default, we only want
+ * to track the start position not the end position of each nonterminal.
+ * Nonterminals that reduce to empty receive position "-1".
  */
 #define YYLLOC_DEFAULT(Current, Rhs, N) \
 	do { \
-		if ((N) > 0) \
-			(Current) = (Rhs)[1]; \
-		else \
-			(Current) = (-1); \
+		(Current) = (-1); \
+		for (int _i_ = 1; _i_ <= (N); _i_++) \
+		{ \
+			if ((Rhs)[_i_] >= 0) \
+			{ \
+				(Current) = (Rhs)[_i_]; \
+				break; \
+			} \
+		} \
 	} while (0)
 
-/*
- * The above macro assigns -1 (unknown) as the parse location of any
- * nonterminal that was reduced from an empty rule, or whose leftmost
- * component was reduced from an empty rule.  This is problematic
- * for nonterminals defined like
- *		OptFooList: / * EMPTY * / { ... } | OptFooList Foo { ... } ;
- * because we'll set -1 as the location during the first reduction and then
- * copy it during each subsequent reduction, leaving us with -1 for the
- * location even when the list is not empty.  To fix that, do this in the
- * action for the nonempty rule(s):
- *		if (@$ < 0) @$ = @2;
- * (Although we have many nonterminals that follow this pattern, we only
- * bother with fixing @$ like this when the nonterminal's parse location
- * is actually referenced in some rule.)
- *
- * A cleaner answer would be to make YYLLOC_DEFAULT scan all the Rhs
- * locations until it's found one that's not -1.  Then we'd get a correct
- * location for any nonterminal that isn't entirely empty.  But this way
- * would add overhead to every rule reduction, and so far there's not been
- * a compelling reason to pay that overhead.
- */
-
 /*
  * Bison doesn't allocate anything that needs to live across parser calls,
  * so we can easily have it use palloc instead of malloc.  This prevents
@@ -930,7 +914,7 @@ parse_toplevel:
 			| MODE_PLPGSQL_EXPR PLpgSQL_Expr
 			{
 				pg_yyget_extra(yyscanner)->parsetree =
-					list_make1(makeRawStmt($2, 0));
+					list_make1(makeRawStmt($2, @2));
 			}
 			| MODE_PLPGSQL_ASSIGN1 PLAssignStmt
 			{
@@ -938,7 +922,7 @@ parse_toplevel:
 
 				n->nnames = 1;
 				pg_yyget_extra(yyscanner)->parsetree =
-					list_make1(makeRawStmt((Node *) n, 0));
+					list_make1(makeRawStmt((Node *) n, @2));
 			}
 			| MODE_PLPGSQL_ASSIGN2 PLAssignStmt
 			{
@@ -946,7 +930,7 @@ parse_toplevel:
 
 				n->nnames = 2;
 				pg_yyget_extra(yyscanner)->parsetree =
-					list_make1(makeRawStmt((Node *) n, 0));
+					list_make1(makeRawStmt((Node *) n, @2));
 			}
 			| MODE_PLPGSQL_ASSIGN3 PLAssignStmt
 			{
@@ -954,18 +938,13 @@ parse_toplevel:
 
 				n->nnames = 3;
 				pg_yyget_extra(yyscanner)->parsetree =
-					list_make1(makeRawStmt((Node *) n, 0));
+					list_make1(makeRawStmt((Node *) n, @2));
 			}
 		;
 
 /*
  * At top level, we wrap each stmt with a RawStmt node carrying start location
- * and length of the stmt's text.  Notice that the start loc/len are driven
- * entirely from semicolon locations (@2).  It would seem natural to use
- * @1 or @3 to get the true start location of a stmt, but that doesn't work
- * for statements that can start with empty nonterminals (opt_with_clause is
- * the main offender here); as noted in the comments for YYLLOC_DEFAULT,
- * we'd get -1 for the location in such cases.
+ * and length of the stmt's text.
  * We also take care to discard empty statements entirely.
  */
 stmtmulti:	stmtmulti ';' toplevel_stmt
@@ -976,14 +955,14 @@ stmtmulti:	stmtmulti ';' toplevel_stmt
 						updateRawStmtEnd(llast_node(RawStmt, $1), @2);
 					}
 					if ($3 != NULL)
-						$$ = lappend($1, makeRawStmt($3, @2 + 1));
+						$$ = lappend($1, makeRawStmt($3, @3));
 					else
 						$$ = $1;
 				}
 			| toplevel_stmt
 				{
 					if ($1 != NULL)
-						$$ = list_make1(makeRawStmt($1, 0));
+						$$ = list_make1(makeRawStmt($1, @1));
 					else
 						$$ = NIL;
 				}
@@ -1584,8 +1563,6 @@ CreateSchemaStmt:
 OptSchemaEltList:
 			OptSchemaEltList schema_stmt
 				{
-					if (@$ < 0)			/* see comments for YYLLOC_DEFAULT */
-						@$ = @2;
 					$$ = lappend($1, $2);
 				}
 			| /* EMPTY */
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index b6370b3b4c..f77760d58e 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -295,14 +295,7 @@ CREATE FUNCTION ext_cor_func() RETURNS text
 CREATE EXTENSION test_ext_cor;  -- fail
 ERROR:  function ext_cor_func() is not a member of extension "test_ext_cor"
 DETAIL:  An extension is not allowed to replace an object that it does not own.
-QUERY:  /* src/test/modules/test_extensions/test_ext_cor--1.0.sql */
--- complain if script is sourced in psql, rather than via CREATE EXTENSION
-
-
--- It's generally bad style to use CREATE OR REPLACE unnecessarily.
--- Test what happens if an extension does it anyway.
-
-CREATE OR REPLACE FUNCTION ext_cor_func() RETURNS text
+QUERY:  CREATE OR REPLACE FUNCTION ext_cor_func() RETURNS text
   AS $$ SELECT 'ext_cor_func: from extension'::text $$ LANGUAGE sql
 CONTEXT:  extension script file "test_ext_cor--1.0.sql"
 SELECT ext_cor_func();
@@ -336,10 +329,7 @@ CREATE TYPE test_ext_type;
 CREATE EXTENSION test_ext_cor;  -- fail
 ERROR:  type test_ext_type is not a member of extension "test_ext_cor"
 DETAIL:  An extension is not allowed to replace an object that it does not own.
-QUERY:  -- These are for testing replacement of a shell type/operator, which works
--- enough like an implicit OR REPLACE to be important to check.
-
-CREATE TYPE test_ext_type AS ENUM('x', 'y')
+QUERY:  CREATE TYPE test_ext_type AS ENUM('x', 'y')
 CONTEXT:  extension script file "test_ext_cor--1.0.sql"
 DROP TYPE test_ext_type;
 -- this makes a shell "point <<@@ polygon" operator too
@@ -400,16 +390,7 @@ CREATE COLLATION ext_cine_coll
 CREATE EXTENSION test_ext_cine;  -- fail
 ERROR:  collation ext_cine_coll is not a member of extension "test_ext_cine"
 DETAIL:  An extension may only use CREATE ... IF NOT EXISTS to skip object creation if the conflicting object is one that it already owns.
-QUERY:  /* src/test/modules/test_extensions/test_ext_cine--1.0.sql */
--- complain if script is sourced in psql, rather than via CREATE EXTENSION
-
-
---
--- CREATE IF NOT EXISTS is an entirely unsound thing for an extension
--- to be doing, but let's at least plug the major security hole in it.
---
-
-CREATE COLLATION IF NOT EXISTS ext_cine_coll
+QUERY:  CREATE COLLATION IF NOT EXISTS ext_cine_coll
   ( LC_COLLATE = "POSIX", LC_CTYPE = "POSIX" )
 CONTEXT:  extension script file "test_ext_cine--1.0.sql"
 DROP COLLATION ext_cine_coll;