Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit a654af2

Browse files
committed
Preliminary refactoring of plpgsql expression construction.
This short and boring patch simply moves the responsibility for initializing PLpgSQL_expr.target_param into plpgsql parsing, rather than doing it at first execution of the expr as before. This doesn't save anything in terms of runtime, since the work was trivial and done only once per expr anyway. But it makes the info available during parsing, which will be useful for the next step. Likewise set PLpgSQL_expr.func during parsing. According to the comments, this was once impossible; but it's certainly possible since we invented the plpgsql_curr_compile variable. Again, this saves little runtime, but it seems far cleaner conceptually. While at it, I reordered stuff in struct PLpgSQL_expr to make it clearer which fields are filled when, and merged some duplicative code in pl_gram.y. Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Reviewed-by: Pavel Borisov <pashkin.elfe@gmail.com> Discussion: https://postgr.es/m/CACxu=vJaKFNsYxooSnW1wEgsAO5u_v1XYBacfVJ14wgJV_PYeg@mail.gmail.com
1 parent 6a7283d commit a654af2

File tree

3 files changed

+62
-61
lines changed

3 files changed

+62
-61
lines changed

src/pl/plpgsql/src/pl_exec.c

-27
Original file line numberDiff line numberDiff line change
@@ -4174,12 +4174,6 @@ exec_prepare_plan(PLpgSQL_execstate *estate,
41744174
SPIPlanPtr plan;
41754175
SPIPrepareOptions options;
41764176

4177-
/*
4178-
* The grammar can't conveniently set expr->func while building the parse
4179-
* tree, so make sure it's set before parser hooks need it.
4180-
*/
4181-
expr->func = estate->func;
4182-
41834177
/*
41844178
* Generate and save the plan
41854179
*/
@@ -5016,21 +5010,7 @@ exec_assign_expr(PLpgSQL_execstate *estate, PLpgSQL_datum *target,
50165010
* If first time through, create a plan for this expression.
50175011
*/
50185012
if (expr->plan == NULL)
5019-
{
5020-
/*
5021-
* Mark the expression as being an assignment source, if target is a
5022-
* simple variable. (This is a bit messy, but it seems cleaner than
5023-
* modifying the API of exec_prepare_plan for the purpose. We need to
5024-
* stash the target dno into the expr anyway, so that it will be
5025-
* available if we have to replan.)
5026-
*/
5027-
if (target->dtype == PLPGSQL_DTYPE_VAR)
5028-
expr->target_param = target->dno;
5029-
else
5030-
expr->target_param = -1; /* should be that already */
5031-
50325013
exec_prepare_plan(estate, expr, 0);
5033-
}
50345014

50355015
value = exec_eval_expr(estate, expr, &isnull, &valtype, &valtypmod);
50365016
exec_assign_value(estate, target, value, isnull, valtype, valtypmod);
@@ -6282,13 +6262,6 @@ setup_param_list(PLpgSQL_execstate *estate, PLpgSQL_expr *expr)
62826262
* that they are interrupting an active use of parameters.
62836263
*/
62846264
paramLI->parserSetupArg = expr;
6285-
6286-
/*
6287-
* Also make sure this is set before parser hooks need it. There is
6288-
* no need to save and restore, since the value is always correct once
6289-
* set. (Should be set already, but let's be sure.)
6290-
*/
6291-
expr->func = estate->func;
62926265
}
62936266
else
62946267
{

src/pl/plpgsql/src/pl_gram.y

+44-21
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ static bool tok_is_keyword(int token, union YYSTYPE *lval,
6161
static void word_is_not_variable(PLword *word, int location, yyscan_t yyscanner);
6262
static void cword_is_not_variable(PLcword *cword, int location, yyscan_t yyscanner);
6363
static void current_token_is_not_variable(int tok, YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_t yyscanner);
64+
static PLpgSQL_expr *make_plpgsql_expr(const char *query,
65+
RawParseMode parsemode);
66+
static void mark_expr_as_assignment_source(PLpgSQL_expr *expr,
67+
PLpgSQL_datum *target);
6468
static PLpgSQL_expr *read_sql_construct(int until,
6569
int until2,
6670
int until3,
@@ -536,6 +540,10 @@ decl_statement : decl_varname decl_const decl_datatype decl_collate decl_notnull
536540
errmsg("variable \"%s\" must have a default value, since it's declared NOT NULL",
537541
var->refname),
538542
parser_errposition(@5)));
543+
544+
if (var->default_val != NULL)
545+
mark_expr_as_assignment_source(var->default_val,
546+
(PLpgSQL_datum *) var);
539547
}
540548
| decl_varname K_ALIAS K_FOR decl_aliasitem ';'
541549
{
@@ -996,6 +1004,7 @@ stmt_assign : T_DATUM
9961004
false, true,
9971005
NULL, NULL,
9981006
&yylval, &yylloc, yyscanner);
1007+
mark_expr_as_assignment_source(new->expr, $1.datum);
9991008

10001009
$$ = (PLpgSQL_stmt *) new;
10011010
}
@@ -2651,6 +2660,38 @@ current_token_is_not_variable(int tok, YYSTYPE *yylvalp, YYLTYPE *yyllocp, yysca
26512660
yyerror(yyllocp, NULL, yyscanner, "syntax error");
26522661
}
26532662

2663+
/* Convenience routine to construct a PLpgSQL_expr struct */
2664+
static PLpgSQL_expr *
2665+
make_plpgsql_expr(const char *query,
2666+
RawParseMode parsemode)
2667+
{
2668+
PLpgSQL_expr *expr = palloc0(sizeof(PLpgSQL_expr));
2669+
2670+
expr->query = pstrdup(query);
2671+
expr->parseMode = parsemode;
2672+
expr->func = plpgsql_curr_compile;
2673+
expr->ns = plpgsql_ns_top();
2674+
/* might get changed later during parsing: */
2675+
expr->target_param = -1;
2676+
/* other fields are left as zeroes until first execution */
2677+
return expr;
2678+
}
2679+
2680+
/* Mark a PLpgSQL_expr as being the source of an assignment to target */
2681+
static void
2682+
mark_expr_as_assignment_source(PLpgSQL_expr *expr, PLpgSQL_datum *target)
2683+
{
2684+
/*
2685+
* Mark the expression as being an assignment source, if target is a
2686+
* simple variable. We don't currently support optimized assignments to
2687+
* other DTYPEs, so no need to mark in other cases.
2688+
*/
2689+
if (target->dtype == PLPGSQL_DTYPE_VAR)
2690+
expr->target_param = target->dno;
2691+
else
2692+
expr->target_param = -1; /* should be that already */
2693+
}
2694+
26542695
/* Convenience routine to read an expression with one possible terminator */
26552696
static PLpgSQL_expr *
26562697
read_sql_expression(int until, const char *expected, YYSTYPE *yylvalp, YYLTYPE *yyllocp, yyscan_t yyscanner)
@@ -2794,13 +2835,7 @@ read_sql_construct(int until,
27942835
*/
27952836
plpgsql_append_source_text(&ds, startlocation, endlocation, yyscanner);
27962837

2797-
expr = palloc0(sizeof(PLpgSQL_expr));
2798-
expr->query = pstrdup(ds.data);
2799-
expr->parseMode = parsemode;
2800-
expr->plan = NULL;
2801-
expr->paramnos = NULL;
2802-
expr->target_param = -1;
2803-
expr->ns = plpgsql_ns_top();
2838+
expr = make_plpgsql_expr(ds.data, parsemode);
28042839
pfree(ds.data);
28052840

28062841
if (valid_sql)
@@ -3122,13 +3157,7 @@ make_execsql_stmt(int firsttoken, int location, PLword *word, YYSTYPE *yylvalp,
31223157
while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
31233158
ds.data[--ds.len] = '\0';
31243159

3125-
expr = palloc0(sizeof(PLpgSQL_expr));
3126-
expr->query = pstrdup(ds.data);
3127-
expr->parseMode = RAW_PARSE_DEFAULT;
3128-
expr->plan = NULL;
3129-
expr->paramnos = NULL;
3130-
expr->target_param = -1;
3131-
expr->ns = plpgsql_ns_top();
3160+
expr = make_plpgsql_expr(ds.data, RAW_PARSE_DEFAULT);
31323161
pfree(ds.data);
31333162

31343163
check_sql_expr(expr->query, expr->parseMode, location, yyscanner);
@@ -4006,13 +4035,7 @@ read_cursor_args(PLpgSQL_var *cursor, int until, YYSTYPE *yylvalp, YYLTYPE *yyll
40064035
appendStringInfoString(&ds, ", ");
40074036
}
40084037

4009-
expr = palloc0(sizeof(PLpgSQL_expr));
4010-
expr->query = pstrdup(ds.data);
4011-
expr->parseMode = RAW_PARSE_PLPGSQL_EXPR;
4012-
expr->plan = NULL;
4013-
expr->paramnos = NULL;
4014-
expr->target_param = -1;
4015-
expr->ns = plpgsql_ns_top();
4038+
expr = make_plpgsql_expr(ds.data, RAW_PARSE_PLPGSQL_EXPR);
40164039
pfree(ds.data);
40174040

40184041
/* Next we'd better find the until token */

src/pl/plpgsql/src/plpgsql.h

+18-13
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,22 @@ typedef struct PLpgSQL_expr
219219
{
220220
char *query; /* query string, verbatim from function body */
221221
RawParseMode parseMode; /* raw_parser() mode to use */
222-
SPIPlanPtr plan; /* plan, or NULL if not made yet */
223-
Bitmapset *paramnos; /* all dnos referenced by this query */
222+
struct PLpgSQL_function *func; /* function containing this expr */
223+
struct PLpgSQL_nsitem *ns; /* namespace chain visible to this expr */
224224

225-
/* function containing this expr (not set until we first parse query) */
226-
struct PLpgSQL_function *func;
225+
/*
226+
* These fields are used to help optimize assignments to expanded-datum
227+
* variables. If this expression is the source of an assignment to a
228+
* simple variable, target_param holds that variable's dno (else it's -1).
229+
*/
230+
int target_param; /* dno of assign target, or -1 if none */
227231

228-
/* namespace chain visible to this expr */
229-
struct PLpgSQL_nsitem *ns;
232+
/*
233+
* Fields above are set during plpgsql parsing. Remaining fields are left
234+
* as zeroes/NULLs until we first parse/plan the query.
235+
*/
236+
SPIPlanPtr plan; /* plan, or NULL if not made yet */
237+
Bitmapset *paramnos; /* all dnos referenced by this query */
230238

231239
/* fields for "simple expression" fast-path execution: */
232240
Expr *expr_simple_expr; /* NULL means not a simple expr */
@@ -235,14 +243,11 @@ typedef struct PLpgSQL_expr
235243
bool expr_simple_mutable; /* true if simple expr is mutable */
236244

237245
/*
238-
* These fields are used to optimize assignments to expanded-datum
239-
* variables. If this expression is the source of an assignment to a
240-
* simple variable, target_param holds that variable's dno; else it's -1.
241-
* If we match a Param within expr_simple_expr to such a variable, that
242-
* Param's address is stored in expr_rw_param; then expression code
243-
* generation will allow the value for that Param to be passed read/write.
246+
* If we match a Param within expr_simple_expr to the variable identified
247+
* by target_param, that Param's address is stored in expr_rw_param; then
248+
* expression code generation will allow the value for that Param to be
249+
* passed as a read/write expanded-object pointer.
244250
*/
245-
int target_param; /* dno of assign target, or -1 if none */
246251
Param *expr_rw_param; /* read/write Param within expr, if any */
247252

248253
/*

0 commit comments

Comments
 (0)