Skip to content

Commit

Permalink
plpgsql: report proper line number for errors in variable initializat…
Browse files Browse the repository at this point in the history
…ion.

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
  • Loading branch information
tglsfdc committed Oct 31, 2021
1 parent fd27065 commit acb2d7d
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 31 deletions.
14 changes: 7 additions & 7 deletions src/pl/plpgsql/src/expected/plpgsql_domain.out
Expand Up @@ -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
Expand Down Expand Up @@ -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
-------------------
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
--
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)];
Expand Down Expand Up @@ -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
---------------------------------
Expand Down
22 changes: 11 additions & 11 deletions src/pl/plpgsql/src/expected/plpgsql_varprops.out
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
35 changes: 26 additions & 9 deletions src/pl/plpgsql/src/pl_exec.c
Expand Up @@ -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)
{
Expand All @@ -1222,21 +1236,16 @@ 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
* local variable initialization"
*/
errcontext("PL/pgSQL function %s line %d %s",
estate->func->fn_signature,
estate->err_stmt->lineno,
err_lineno,
_(estate->err_text));
}
else
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1717,6 +1731,8 @@ exec_stmt_block(PLpgSQL_execstate *estate, PLpgSQL_stmt_block *block)
}
}

estate->err_var = NULL;

if (block->exceptions)
{
/*
Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/pl/plpgsql/src/plpgsql.h
Expand Up @@ -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 */
Expand Down
4 changes: 2 additions & 2 deletions src/test/regress/expected/domain.out
Expand Up @@ -847,15 +847,15 @@ 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
return p1;
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
Expand Down
4 changes: 2 additions & 2 deletions src/test/regress/expected/plpgsql.out
Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit acb2d7d

Please sign in to comment.