Skip to content

Commit fcdfce6

Browse files
tglsfdctglsfdc
committed
Detect mismatched CONTINUE and EXIT statements at plpgsql compile time.
With a bit of tweaking of the compile namestack data structure, we can verify at compile time whether a CONTINUE or EXIT is legal. This is surely better than leaving it to runtime, both because earlier is better and because we can issue a proper error pointer. Also, we can get rid of the ad-hoc old way of detecting the problem, which only took care of CONTINUE not EXIT. Jim Nasby, adjusted a bit by me
1 parent 072710d commit fcdfce6

File tree

7 files changed

+226
-85
lines changed

7 files changed

+226
-85
lines changed

‎src/pl/plpgsql/src/pl_comp.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ do_compile(FunctionCallInfo fcinfo,
371371
* variables (such as FOUND), and is named after the function itself.
372372
*/
373373
plpgsql_ns_init();
374-
plpgsql_ns_push(NameStr(procStruct->proname));
374+
plpgsql_ns_push(NameStr(procStruct->proname), PLPGSQL_LABEL_BLOCK);
375375
plpgsql_DumpExecTree = false;
376376
plpgsql_start_datums();
377377

@@ -851,7 +851,7 @@ plpgsql_compile_inline(char *proc_source)
851851
function->extra_errors = 0;
852852

853853
plpgsql_ns_init();
854-
plpgsql_ns_push(func_name);
854+
plpgsql_ns_push(func_name, PLPGSQL_LABEL_BLOCK);
855855
plpgsql_DumpExecTree = false;
856856
plpgsql_start_datums();
857857

‎src/pl/plpgsql/src/pl_exec.c‎

Lines changed: 9 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -437,19 +437,9 @@ plpgsql_exec_function(PLpgSQL_function *func, FunctionCallInfo fcinfo,
437437
{
438438
estate.err_stmt = NULL;
439439
estate.err_text = NULL;
440-
441-
/*
442-
* Provide a more helpful message if a CONTINUE has been used outside
443-
* the context it can work in.
444-
*/
445-
if (rc == PLPGSQL_RC_CONTINUE)
446-
ereport(ERROR,
447-
(errcode(ERRCODE_SYNTAX_ERROR),
448-
errmsg("CONTINUE cannot be used outside a loop")));
449-
else
450-
ereport(ERROR,
451-
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
452-
errmsg("control reached end of function without RETURN")));
440+
ereport(ERROR,
441+
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
442+
errmsg("control reached end of function without RETURN")));
453443
}
454444

455445
/*
@@ -791,19 +781,9 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
791781
{
792782
estate.err_stmt = NULL;
793783
estate.err_text = NULL;
794-
795-
/*
796-
* Provide a more helpful message if a CONTINUE has been used outside
797-
* the context it can work in.
798-
*/
799-
if (rc == PLPGSQL_RC_CONTINUE)
800-
ereport(ERROR,
801-
(errcode(ERRCODE_SYNTAX_ERROR),
802-
errmsg("CONTINUE cannot be used outside a loop")));
803-
else
804-
ereport(ERROR,
805-
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
806-
errmsg("control reached end of trigger procedure without RETURN")));
784+
ereport(ERROR,
785+
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
786+
errmsg("control reached end of trigger procedure without RETURN")));
807787
}
808788

809789
estate.err_stmt = NULL;
@@ -919,19 +899,9 @@ plpgsql_exec_event_trigger(PLpgSQL_function *func, EventTriggerData *trigdata)
919899
{
920900
estate.err_stmt = NULL;
921901
estate.err_text = NULL;
922-
923-
/*
924-
* Provide a more helpful message if a CONTINUE has been used outside
925-
* the context it can work in.
926-
*/
927-
if (rc == PLPGSQL_RC_CONTINUE)
928-
ereport(ERROR,
929-
(errcode(ERRCODE_SYNTAX_ERROR),
930-
errmsg("CONTINUE cannot be used outside a loop")));
931-
else
932-
ereport(ERROR,
933-
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
934-
errmsg("control reached end of trigger procedure without RETURN")));
902+
ereport(ERROR,
903+
(errcode(ERRCODE_S_R_E_FUNCTION_EXECUTED_NO_RETURN_STATEMENT),
904+
errmsg("control reached end of trigger procedure without RETURN")));
935905
}
936906

937907
estate.err_stmt = NULL;

‎src/pl/plpgsql/src/pl_funcs.c‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,11 @@ plpgsql_ns_init(void)
5151
* ----------
5252
*/
5353
void
54-
plpgsql_ns_push(const char *label)
54+
plpgsql_ns_push(const char *label, enum PLpgSQL_label_types label_type)
5555
{
5656
if (label == NULL)
5757
label = "";
58-
plpgsql_ns_additem(PLPGSQL_NSTYPE_LABEL, 0, label);
58+
plpgsql_ns_additem(PLPGSQL_NSTYPE_LABEL, (int) label_type, label);
5959
}
6060

6161

@@ -206,6 +206,25 @@ plpgsql_ns_lookup_label(PLpgSQL_nsitem *ns_cur, const char *name)
206206
}
207207

208208

209+
/* ----------
210+
* plpgsql_ns_find_nearest_loop Find innermost loop label in namespace chain
211+
* ----------
212+
*/
213+
PLpgSQL_nsitem *
214+
plpgsql_ns_find_nearest_loop(PLpgSQL_nsitem *ns_cur)
215+
{
216+
while (ns_cur != NULL)
217+
{
218+
if (ns_cur->itemtype == PLPGSQL_NSTYPE_LABEL &&
219+
ns_cur->itemno == PLPGSQL_LABEL_LOOP)
220+
return ns_cur;
221+
ns_cur = ns_cur->prev;
222+
}
223+
224+
return NULL; /* no loop found */
225+
}
226+
227+
209228
/*
210229
* Statement type as a string, for use in error messages etc.
211230
*/

‎src/pl/plpgsql/src/pl_gram.y‎

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,8 @@ static void check_raise_parameters(PLpgSQL_stmt_raise *stmt);
186186
%type <forvariable> for_variable
187187
%type <stmt> for_control
188188

189-
%type <str> any_identifier opt_block_label opt_label option_value
189+
%type <str> any_identifier opt_block_label opt_loop_label opt_label
190+
%type <str> option_value
190191

191192
%type <list> proc_sect stmt_elsifs stmt_else
192193
%type <loop_body> loop_body
@@ -535,7 +536,7 @@ decl_statement : decl_varname decl_const decl_datatype decl_collate decl_notnull
535536
$4->itemno, $1.name);
536537
}
537538
| decl_varname opt_scrollable K_CURSOR
538-
{ plpgsql_ns_push($1.name); }
539+
{ plpgsql_ns_push($1.name, PLPGSQL_LABEL_OTHER); }
539540
decl_cursor_args decl_is_for decl_cursor_query
540541
{
541542
PLpgSQL_var *new;
@@ -1218,7 +1219,7 @@ opt_case_else :
12181219
}
12191220
;
12201221

1221-
stmt_loop : opt_block_label K_LOOP loop_body
1222+
stmt_loop : opt_loop_label K_LOOP loop_body
12221223
{
12231224
PLpgSQL_stmt_loop *new;
12241225

@@ -1235,7 +1236,7 @@ stmt_loop : opt_block_label K_LOOP loop_body
12351236
}
12361237
;
12371238

1238-
stmt_while : opt_block_label K_WHILE expr_until_loop loop_body
1239+
stmt_while : opt_loop_label K_WHILE expr_until_loop loop_body
12391240
{
12401241
PLpgSQL_stmt_while *new;
12411242

@@ -1253,7 +1254,7 @@ stmt_while : opt_block_label K_WHILE expr_until_loop loop_body
12531254
}
12541255
;
12551256

1256-
stmt_for : opt_block_label K_FOR for_control loop_body
1257+
stmt_for : opt_loop_label K_FOR for_control loop_body
12571258
{
12581259
/* This runs after we've scanned the loop body */
12591260
if ($3->cmd_type == PLPGSQL_STMT_FORI)
@@ -1282,7 +1283,7 @@ stmt_for : opt_block_label K_FOR for_control loop_body
12821283
}
12831284

12841285
check_labels($1, $4.end_label, $4.end_label_location);
1285-
/* close namespace started in opt_block_label */
1286+
/* close namespace started in opt_loop_label */
12861287
plpgsql_ns_pop();
12871288
}
12881289
;
@@ -1603,7 +1604,7 @@ for_variable : T_DATUM
16031604
}
16041605
;
16051606

1606-
stmt_foreach_a : opt_block_label K_FOREACH for_variable foreach_slice K_IN K_ARRAY expr_until_loop loop_body
1607+
stmt_foreach_a : opt_loop_label K_FOREACH for_variable foreach_slice K_IN K_ARRAY expr_until_loop loop_body
16071608
{
16081609
PLpgSQL_stmt_foreach_a *new;
16091610

@@ -1666,6 +1667,42 @@ stmt_exit : exit_type opt_label opt_exitcond
16661667
new->label = $2;
16671668
new->cond = $3;
16681669

1670+
if ($2)
1671+
{
1672+
/* We have a label, so verify it exists */
1673+
PLpgSQL_nsitem *label;
1674+
1675+
label = plpgsql_ns_lookup_label(plpgsql_ns_top(), $2);
1676+
if (label == NULL)
1677+
ereport(ERROR,
1678+
(errcode(ERRCODE_SYNTAX_ERROR),
1679+
errmsg("label \"%s\" does not exist",
1680+
$2),
1681+
parser_errposition(@2)));
1682+
/* CONTINUE only allows loop labels */
1683+
if (label->itemno != PLPGSQL_LABEL_LOOP && !$1)
1684+
ereport(ERROR,
1685+
(errcode(ERRCODE_SYNTAX_ERROR),
1686+
errmsg("block label \"%s\" cannot be used in CONTINUE",
1687+
$2),
1688+
parser_errposition(@2)));
1689+
}
1690+
else
1691+
{
1692+
/*
1693+
* No label, so make sure there is some loop (an
1694+
* unlabelled EXIT does not match a block, so this
1695+
* is the same test for both EXIT and CONTINUE)
1696+
*/
1697+
if (plpgsql_ns_find_nearest_loop(plpgsql_ns_top()) == NULL)
1698+
ereport(ERROR,
1699+
(errcode(ERRCODE_SYNTAX_ERROR),
1700+
/* translator: %s is EXIT or CONTINUE */
1701+
errmsg("%s cannot be used outside a loop",
1702+
plpgsql_stmt_typename((PLpgSQL_stmt *) new)),
1703+
parser_errposition(@1)));
1704+
}
1705+
16691706
$$ = (PLpgSQL_stmt *)new;
16701707
}
16711708
;
@@ -2290,12 +2327,24 @@ expr_until_loop :
22902327

22912328
opt_block_label :
22922329
{
2293-
plpgsql_ns_push(NULL);
2330+
plpgsql_ns_push(NULL, PLPGSQL_LABEL_BLOCK);
2331+
$$ = NULL;
2332+
}
2333+
| LESS_LESS any_identifier GREATER_GREATER
2334+
{
2335+
plpgsql_ns_push($2, PLPGSQL_LABEL_BLOCK);
2336+
$$ = $2;
2337+
}
2338+
;
2339+
2340+
opt_loop_label :
2341+
{
2342+
plpgsql_ns_push(NULL, PLPGSQL_LABEL_LOOP);
22942343
$$ = NULL;
22952344
}
22962345
| LESS_LESS any_identifier GREATER_GREATER
22972346
{
2298-
plpgsql_ns_push($2);
2347+
plpgsql_ns_push($2, PLPGSQL_LABEL_LOOP);
22992348
$$ = $2;
23002349
}
23012350
;
@@ -2306,8 +2355,7 @@ opt_label :
23062355
}
23072356
| any_identifier
23082357
{
2309-
if (plpgsql_ns_lookup_label(plpgsql_ns_top(), $1) == NULL)
2310-
yyerror("label does not exist");
2358+
/* label validity will be checked by outer production */
23112359
$$ = $1;
23122360
}
23132361
;

‎src/pl/plpgsql/src/plpgsql.h‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,17 @@ enum
4646
PLPGSQL_NSTYPE_REC
4747
};
4848

49+
/* ----------
50+
* A PLPGSQL_NSTYPE_LABEL stack entry must be one of these types
51+
* ----------
52+
*/
53+
enum PLpgSQL_label_types
54+
{
55+
PLPGSQL_LABEL_BLOCK, /* DECLARE/BEGIN block */
56+
PLPGSQL_LABEL_LOOP, /* looping construct */
57+
PLPGSQL_LABEL_OTHER /* anything else */
58+
};
59+
4960
/* ----------
5061
* Datum array node types
5162
* ----------
@@ -331,6 +342,8 @@ typedef struct PLpgSQL_nsitem
331342
{ /* Item in the compilers namespace tree */
332343
int itemtype;
333344
int itemno;
345+
/* For labels, itemno is a value of enum PLpgSQL_label_types. */
346+
/* For other itemtypes, itemno is the associated PLpgSQL_datum's dno. */
334347
struct PLpgSQL_nsitem *prev;
335348
char name[FLEXIBLE_ARRAY_MEMBER]; /* nul-terminated string */
336349
} PLpgSQL_nsitem;
@@ -997,7 +1010,8 @@ extern void exec_get_datum_type_info(PLpgSQL_execstate *estate,
9971010
* ----------
9981011
*/
9991012
extern void plpgsql_ns_init(void);
1000-
extern void plpgsql_ns_push(const char *label);
1013+
extern void plpgsql_ns_push(const char *label,
1014+
enum PLpgSQL_label_types label_type);
10011015
extern void plpgsql_ns_pop(void);
10021016
extern PLpgSQL_nsitem *plpgsql_ns_top(void);
10031017
extern void plpgsql_ns_additem(int itemtype, int itemno, const char *name);
@@ -1006,6 +1020,7 @@ extern PLpgSQL_nsitem *plpgsql_ns_lookup(PLpgSQL_nsitem *ns_cur, bool localmode,
10061020
const char *name3, int *names_used);
10071021
extern PLpgSQL_nsitem *plpgsql_ns_lookup_label(PLpgSQL_nsitem *ns_cur,
10081022
const char *name);
1023+
extern PLpgSQL_nsitem *plpgsql_ns_find_nearest_loop(PLpgSQL_nsitem *ns_cur);
10091024

10101025
/* ----------
10111026
* Other functions in pl_funcs.c

0 commit comments

Comments
 (0)