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

diff --git a/src/pl/plpgsql/src/expected/plpgsql_domain.out b/src/pl/plpgsql/src/expected/plpgsql_domain.out
index efc877cdd10094d08afce082a812630ff0a41c6d..516c2b9e08ee1c1b77763ceebdbeb6f7395a6273 100644 (file)
--- 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 3801dccc95a963ada0e67de27dd7f0c6358a4478..25115a02bd85faa00a6bd3cc8cadc8c8464e56d2 100644 (file)
--- 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 0e1cfa3df621320b3f6e12122b004d062728d6d0..6dbfdb7be03e1f1b5b38c3af926f49928e30d829 100644 (file)
--- 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 ebd3a5d3c8a33d9726120083c0ce7e11cb3ae16f..3936866bb71cd5fd659923aaf4e17f3b03383803 100644 (file)
--- 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 a04bd00ac69b935a6e62cfe18f89421e07f6ff19..8faf46d4047fb3617b1302b04be4d227c59b9ccc 100644 (file)
--- 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 ae6fc824b60cefa1344502f118a7c2bac16f13db..33eb5e54b9c02e24ad1ccc9f01768ef876b123b9 100644 (file)
--- 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;