plpgsql: report proper line number for errors in variable initialization.

Previously, we pointed at the surrounding block's BEGIN keyword.
If there are multiple variables being initialized in a DECLARE section,
this isn't good enough: it can be quite confusing and unhelpful.
We do know where the variable's declaration started, so it just takes
a tiny bit more error-reporting infrastructure to use that.

Discussion: https://postgr.es/m/713975.1635530414@sss.pgh.pa.us

6 files changed, 49 insertions, 31 deletions

diff --git a/src/pl/plpgsql/src/expected/plpgsql_domain.out b/src/pl/plpgsql/src/expected/plpgsql_domain.out
index efc877cdd10..516c2b9e08e 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_domain.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_domain.out
@@ -33,7 +33,7 @@ SELECT * FROM test_assign_booltrue(true, true);
 
 SELECT * FROM test_assign_booltrue(false, true);
 ERROR:  value for domain booltrue violates check constraint "booltrue_check"
-CONTEXT:  PL/pgSQL function test_assign_booltrue(boolean,boolean) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_booltrue(boolean,boolean) line 2 during statement block local variable initialization
 SELECT * FROM test_assign_booltrue(true, false);
 ERROR:  value for domain booltrue violates check constraint "booltrue_check"
 CONTEXT:  PL/pgSQL function test_assign_booltrue(boolean,boolean) line 4 at assignment
@@ -76,7 +76,7 @@ ERROR:  value for domain uint2 violates check constraint "uint2_check"
 CONTEXT:  PL/pgSQL function test_assign_uint2(integer,integer) line 4 at assignment
 SELECT * FROM test_assign_uint2(-100, 50);
 ERROR:  value for domain uint2 violates check constraint "uint2_check"
-CONTEXT:  PL/pgSQL function test_assign_uint2(integer,integer) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_uint2(integer,integer) line 2 during statement block local variable initialization
 SELECT * FROM test_assign_uint2(null, 1);
  test_assign_uint2 
 -------------------
@@ -115,7 +115,7 @@ SELECT * FROM test_assign_nnint(10, 20);
 
 SELECT * FROM test_assign_nnint(null, 20);
 ERROR:  domain nnint does not allow null values
-CONTEXT:  PL/pgSQL function test_assign_nnint(integer,integer) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_nnint(integer,integer) line 2 during statement block local variable initialization
 SELECT * FROM test_assign_nnint(10, null);
 ERROR:  domain nnint does not allow null values
 CONTEXT:  PL/pgSQL function test_assign_nnint(integer,integer) line 4 at assignment
@@ -168,7 +168,7 @@ ERROR:  value for domain ordered_pair_domain violates check constraint "ordered_
 CONTEXT:  PL/pgSQL function test_assign_ordered_pair_domain(integer,integer,integer) line 4 at assignment
 SELECT * FROM test_assign_ordered_pair_domain(2,1,3);
 ERROR:  value for domain ordered_pair_domain violates check constraint "ordered_pair_domain_check"
-CONTEXT:  PL/pgSQL function test_assign_ordered_pair_domain(integer,integer,integer) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_ordered_pair_domain(integer,integer,integer) line 2 during statement block local variable initialization
 --
 -- Arrays of domains
 --
@@ -276,7 +276,7 @@ ERROR:  domain nnint does not allow null values
 CONTEXT:  PL/pgSQL function test_assign_nnint_container(integer,integer,integer) line 4 at assignment
 SELECT * FROM test_assign_nnint_container(1,null,3);
 ERROR:  domain nnint does not allow null values
-CONTEXT:  PL/pgSQL function test_assign_nnint_container(integer,integer,integer) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_nnint_container(integer,integer,integer) line 2 during statement block local variable initialization
 -- Since core system allows this:
 SELECT null::nnint_container;
  nnint_container 
@@ -356,7 +356,7 @@ ERROR:  value for domain ordered_named_pair violates check constraint "ordered_n
 CONTEXT:  PL/pgSQL function test_assign_ordered_named_pair(integer,integer,integer) line 4 at assignment
 SELECT * FROM test_assign_ordered_named_pair(2,1,3);
 ERROR:  value for domain ordered_named_pair violates check constraint "ordered_named_pair_check"
-CONTEXT:  PL/pgSQL function test_assign_ordered_named_pair(integer,integer,integer) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_ordered_named_pair(integer,integer,integer) line 2 during statement block local variable initialization
 CREATE FUNCTION build_ordered_named_pairs(i int, j int) RETURNS ordered_named_pair[] AS $$
 begin
 return array[row(i, j), row(i, j+1)];
@@ -388,7 +388,7 @@ SELECT * FROM test_assign_ordered_named_pairs(1,2,3);
 
 SELECT * FROM test_assign_ordered_named_pairs(2,1,3);
 ERROR:  value for domain ordered_named_pair violates check constraint "ordered_named_pair_check"
-CONTEXT:  PL/pgSQL function test_assign_ordered_named_pairs(integer,integer,integer) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function test_assign_ordered_named_pairs(integer,integer,integer) line 2 during statement block local variable initialization
 SELECT * FROM test_assign_ordered_named_pairs(1,2,0);  -- should fail someday
  test_assign_ordered_named_pairs 
 ---------------------------------
diff --git a/src/pl/plpgsql/src/expected/plpgsql_varprops.out b/src/pl/plpgsql/src/expected/plpgsql_varprops.out
index 3801dccc95a..25115a02bd8 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_varprops.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_varprops.out
@@ -77,7 +77,7 @@ begin
 end$$;
 ERROR:  division by zero
 CONTEXT:  SQL expression "1/0"
-PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x bigint[] := array[1,3,5];
 begin
@@ -120,7 +120,7 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  null value cannot be assigned to variable "x" declared NOT NULL
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x record not null;  -- fail
 begin
@@ -147,7 +147,7 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  null value cannot be assigned to variable "x" declared NOT NULL
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x var_record not null;  -- fail
 begin
@@ -174,7 +174,7 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  null value cannot be assigned to variable "x" declared NOT NULL
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 -- Check that variables are reinitialized on block re-entry.
 do $$
 begin
@@ -216,14 +216,14 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  domain int_nn does not allow null values
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x int_nn := null;  -- fail
 begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  domain int_nn does not allow null values
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x int_nn := 42;
 begin
@@ -239,14 +239,14 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  domain var_record_nn does not allow null values
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x var_record_nn := null;  -- fail
 begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  domain var_record_nn does not allow null values
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x var_record_nn := row(1,2);
 begin
@@ -263,21 +263,21 @@ begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  value for domain var_record_colnn violates check constraint "var_record_colnn_check"
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x var_record_colnn := null;  -- fail
 begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  value for domain var_record_colnn violates check constraint "var_record_colnn_check"
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x var_record_colnn := row(1,null);  -- fail
 begin
   raise notice 'x = %', x;
 end$$;
 ERROR:  value for domain var_record_colnn violates check constraint "var_record_colnn_check"
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x var_record_colnn := row(1,2);
 begin
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index 0e1cfa3df62..6dbfdb7be03 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -1214,6 +1214,20 @@ static void
 plpgsql_exec_error_callback(void *arg)
 {
 	PLpgSQL_execstate *estate = (PLpgSQL_execstate *) arg;
+	int			err_lineno;
+
+	/*
+	 * If err_var is set, report the variable's declaration line number.
+	 * Otherwise, if err_stmt is set, report the err_stmt's line number.  When
+	 * err_stmt is not set, we're in function entry/exit, or some such place
+	 * not attached to a specific line number.
+	 */
+	if (estate->err_var != NULL)
+		err_lineno = estate->err_var->lineno;
+	else if (estate->err_stmt != NULL)
+		err_lineno = estate->err_stmt->lineno;
+	else
+		err_lineno = 0;
 
 	if (estate->err_text != NULL)
 	{
@@ -1222,13 +1236,8 @@ plpgsql_exec_error_callback(void *arg)
 		 * actually need it.  Therefore, places that set up err_text should
 		 * use gettext_noop() to ensure the strings get recorded in the
 		 * message dictionary.
-		 *
-		 * If both err_text and err_stmt are set, use the err_text as
-		 * description, but report the err_stmt's line number.  When err_stmt
-		 * is not set, we're in function entry/exit, or some such place not
-		 * attached to a specific line number.
 		 */
-		if (estate->err_stmt != NULL)
+		if (err_lineno > 0)
 		{
 			/*
 			 * translator: last %s is a phrase such as "during statement block
@@ -1236,7 +1245,7 @@ plpgsql_exec_error_callback(void *arg)
 			 */
 			errcontext("PL/pgSQL function %s line %d %s",
 					   estate->func->fn_signature,
-					   estate->err_stmt->lineno,
+					   err_lineno,
 					   _(estate->err_text));
 		}
 		else
@@ -1250,12 +1259,12 @@ plpgsql_exec_error_callback(void *arg)
 					   _(estate->err_text));
 		}
 	}
-	else if (estate->err_stmt != NULL)
+	else if (estate->err_stmt != NULL && err_lineno > 0)
 	{
 		/* translator: last %s is a plpgsql statement type name */
 		errcontext("PL/pgSQL function %s line %d at %s",
 				   estate->func->fn_signature,
-				   estate->err_stmt->lineno,
+				   err_lineno,
 				   plpgsql_stmt_typename(estate->err_stmt));
 	}
 	else
@@ -1643,7 +1652,12 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
 		 * Note that we currently don't support promise datums within blocks,
 		 * only at a function's outermost scope, so we needn't handle those
 		 * here.
+		 *
+		 * Since RECFIELD isn't a supported case either, it's okay to cast the
+		 * PLpgSQL_datum to PLpgSQL_variable.
 		 */
+		estate->err_var = (PLpgSQL_variable *) datum;
+
 		switch (datum->dtype)
 		{
 			case PLPGSQL_DTYPE_VAR:
@@ -1717,6 +1731,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
 		}
 	}
 
+	estate->err_var = NULL;
+
 	if (block->exceptions)
 	{
 		/*
@@ -4041,6 +4057,7 @@ plpgsql_estate_setup(PLpgSQL_execstate *estate,
 	estate->eval_econtext = NULL;
 
 	estate->err_stmt = NULL;
+	estate->err_var = NULL;
 	estate->err_text = NULL;
 
 	estate->plugin_info = NULL;
diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h
index ebd3a5d3c8a..3936866bb71 100644
--- a/src/pl/plpgsql/src/plpgsql.h
+++ b/src/pl/plpgsql/src/plpgsql.h
@@ -1088,6 +1088,7 @@ typedef struct PLpgSQL_execstate
 
 	/* status information for error context reporting */
 	PLpgSQL_stmt *err_stmt;		/* current stmt */
+	PLpgSQL_variable *err_var;	/* current variable, if in a DECLARE section */
 	const char *err_text;		/* additional state info */
 
 	void	   *plugin_info;	/* reserved for use by optional plugin */
diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out
index a04bd00ac69..8faf46d4047 100644
--- a/src/test/regress/expected/domain.out
+++ b/src/test/regress/expected/domain.out
@@ -847,7 +847,7 @@ begin
 end$$ language plpgsql;
 select doubledecrement(3); -- fail because of implicit null assignment
 ERROR:  domain pos_int does not allow null values
-CONTEXT:  PL/pgSQL function doubledecrement(pos_int) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function doubledecrement(pos_int) line 2 during statement block local variable initialization
 create or replace function doubledecrement(p1 pos_int) returns pos_int as $$
 declare v pos_int := 0;
 begin
@@ -855,7 +855,7 @@ begin
 end$$ language plpgsql;
 select doubledecrement(3); -- fail at initialization assignment
 ERROR:  value for domain pos_int violates check constraint "pos_int_check"
-CONTEXT:  PL/pgSQL function doubledecrement(pos_int) line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function doubledecrement(pos_int) line 2 during statement block local variable initialization
 create or replace function doubledecrement(p1 pos_int) returns pos_int as $$
 declare v pos_int := 1;
 begin
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index ae6fc824b60..33eb5e54b9c 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -4648,7 +4648,7 @@ ERROR:  column "x" does not exist
 LINE 1: x + 1
         ^
 QUERY:  x + 1
-CONTEXT:  PL/pgSQL function inline_code_block line 3 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare y int := x + 1;  -- error
         x int := 42;
@@ -4660,7 +4660,7 @@ ERROR:  column "x" does not exist
 LINE 1: x + 1
         ^
 QUERY:  x + 1
-CONTEXT:  PL/pgSQL function inline_code_block line 4 during statement block local variable initialization
+CONTEXT:  PL/pgSQL function inline_code_block line 2 during statement block local variable initialization
 do $$
 declare x int := 42;
         y int := x + 1;