From de2af6e001a3d6aeb2a10a802e73af8c7d1d3405 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 15 Jul 2017 16:57:43 -0400 Subject: [PATCH] Improve comments for execExpr.c's handling of FieldStore subexpressions. Given this code's general eagerness to use subexpressions' output variables as temporary workspace, it's not exactly clear that it is safe for FieldStore to tell a newval subexpression that it can write into the same variable that is being supplied as a potential input. Document the chain of assumptions needed for that to be safe. --- src/backend/executor/execExpr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index d1c2bbbd44..5267a011bb 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1116,6 +1116,18 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state, * field assignment can't be within a CASE either. (So * saving and restoring innermost_caseval is just * paranoia, but let's do it anyway.) + * + * Another non-obvious point is that it's safe to use the + * field's values[]/nulls[] entries as both the caseval + * source and the result address for this subexpression. + * That's okay only because (1) both FieldStore and + * ArrayRef evaluate their arg or refexpr inputs first, + * and (2) any such CaseTestExpr is directly the arg or + * refexpr input. So any read of the caseval will occur + * before there's a chance to overwrite it. Also, if + * multiple entries in the newvals/fieldnums lists target + * the same field, they'll effectively be applied + * left-to-right which is what we want. */ save_innermost_caseval = state->innermost_caseval; save_innermost_casenull = state->innermost_casenull; -- 2.40.0