diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-08-19 14:07:49 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-08-19 14:07:49 -0400 |
commit | 814a57065ec97e44de58db61a16957ddd09938e8 (patch) | |
tree | 9d2ceeca7660d552fd141202c4f3a9a0cc74d2dd /src/backend/optimizer/path/equivclass.c | |
parent | aecefffc3f8041c883ab4fb035cf0d5519b5a7ed (diff) |
Suppress unnecessary RelabelType nodes in yet more cases.
Commit a477bfc1d fixed eval_const_expressions() to ensure that it
didn't generate unnecessary RelabelType nodes, but I failed to notice
that some other places in the planner had the same issue. Really
noplace in the planner should be using plain makeRelabelType(), for
fear of generating expressions that should be equal() to semantically
equivalent trees, but aren't.
An example is that because canonicalize_ec_expression() failed
to be careful about this, we could end up with an equivalence class
containing both a plain Const, and a Const-with-RelabelType
representing exactly the same value. So far as I can tell this led to
no visible misbehavior, but we did waste a bunch of cycles generating
and evaluating "Const = Const-with-RelabelType" to prove such entries
are redundant.
Hence, move the support function added by a477bfc1d to where it can
be more generally useful, and use it in the places where planner code
previously used makeRelabelType.
Back-patch to v12, like the previous patch. While I have no concrete
evidence of any real misbehavior here, it's certainly possible that
I overlooked a case where equivalent expressions that aren't equal()
could cause a user-visible problem. In any case carrying extra
RelabelType nodes through planning to execution isn't very desirable.
Discussion: https://postgr.es/m/1311836.1597781384@sss.pgh.pa.us
Diffstat (limited to 'src/backend/optimizer/path/equivclass.c')
-rw-r--r-- | src/backend/optimizer/path/equivclass.c | 43 |
1 files changed, 17 insertions, 26 deletions
diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 9264d819109..85a0105e0e4 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -485,10 +485,6 @@ process_equivalence(PlannerInfo *root, * work to not label the collation at all in EC members, but this is risky * since some parts of the system expect exprCollation() to deliver the * right answer for a sort key.) - * - * Note this code assumes that the expression has already been through - * eval_const_expressions, so there are no CollateExprs and no redundant - * RelabelTypes. */ Expr * canonicalize_ec_expression(Expr *expr, Oid req_type, Oid req_collation) @@ -509,29 +505,24 @@ canonicalize_ec_expression(Expr *expr, Oid req_type, Oid req_collation) exprCollation((Node *) expr) != req_collation) { /* - * Strip any existing RelabelType, then add a new one if needed. This - * is to preserve the invariant of no redundant RelabelTypes. - * - * If we have to change the exposed type of the stripped expression, - * set typmod to -1 (since the new type may not have the same typmod - * interpretation). If we only have to change collation, preserve the - * exposed typmod. + * If we have to change the type of the expression, set typmod to -1, + * since the new type may not have the same typmod interpretation. + * When we only have to change collation, preserve the exposed typmod. + */ + int32 req_typmod; + + if (expr_type != req_type) + req_typmod = -1; + else + req_typmod = exprTypmod((Node *) expr); + + /* + * Use applyRelabelType so that we preserve const-flatness. This is + * important since eval_const_expressions has already been applied. */ - while (expr && IsA(expr, RelabelType)) - expr = (Expr *) ((RelabelType *) expr)->arg; - - if (exprType((Node *) expr) != req_type) - expr = (Expr *) makeRelabelType(expr, - req_type, - -1, - req_collation, - COERCE_IMPLICIT_CAST); - else if (exprCollation((Node *) expr) != req_collation) - expr = (Expr *) makeRelabelType(expr, - req_type, - exprTypmod((Node *) expr), - req_collation, - COERCE_IMPLICIT_CAST); + expr = (Expr *) applyRelabelType((Node *) expr, + req_type, req_typmod, req_collation, + COERCE_IMPLICIT_CAST, -1, false); } return expr; |