From acb2d7d5d2301f07d5857ee252995e62ce9e7055 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 31 Oct 2021 12:43:47 -0400 Subject: [PATCH] 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 --- .../plpgsql/src/expected/plpgsql_domain.out | 14 ++++---- .../plpgsql/src/expected/plpgsql_varprops.out | 22 ++++++------ src/pl/plpgsql/src/pl_exec.c | 35 ++++++++++++++----- src/pl/plpgsql/src/plpgsql.h | 1 + src/test/regress/expected/domain.out | 4 +-- src/test/regress/expected/plpgsql.out | 4 +-- 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 efc877cdd1..516c2b9e08 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 3801dccc95..25115a02bd 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 0e1cfa3df6..6dbfdb7be0 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 ebd3a5d3c8..3936866bb7 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 a04bd00ac6..8faf46d404 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 ae6fc824b6..33eb5e54b9 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;