]> granicus.if.org Git - postgresql/commitdiff
Fix up handling of simple-form CASE with constant test expression.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 19 Dec 2010 20:30:44 +0000 (15:30 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 19 Dec 2010 20:30:44 +0000 (15:30 -0500)
eval_const_expressions() can replace CaseTestExprs with constants when
the surrounding CASE's test expression is a constant.  This confuses
ruleutils.c's heuristic for deparsing simple-form CASEs, leading to
Assert failures or "unexpected CASE WHEN clause" errors.  I had put in
a hack solution for that years ago (see commit
514ce7a331c5bea8e55b106d624e55732a002295 of 2006-10-01), but bug #5794
from Peter Speck shows that that solution failed to cover all cases.

Fortunately, there's a much better way, which came to me upon reflecting
that Peter's "CASE TRUE WHEN" seemed pretty redundant: we can "simplify"
the simple-form CASE to the general form of CASE, by simply omitting the
constant test expression from the rebuilt CASE construct.  This is
intuitively valid because there is no need for the executor to evaluate
the test expression at runtime; it will never be referenced, because any
CaseTestExprs that would have referenced it are now replaced by constants.
This won't save a whole lot of cycles, since evaluating a Const is pretty
cheap, but a cycle saved is a cycle earned.  In any case it beats kluging
ruleutils.c still further.  So this patch improves const-simplification
and reverts the previous change in ruleutils.c.

Back-patch to all supported branches.  The bug exists in 8.1 too, but it's
out of warranty.

src/backend/optimizer/util/clauses.c
src/backend/utils/adt/ruleutils.c

index fa84164b068f9ede9edc42806e024fb083ff18d8..ccf913b35cf34c7f0b516af90efbe86c3be4d1d4 100644 (file)
@@ -2583,7 +2583,18 @@ eval_const_expressions_mutator(Node *node,
                 * placeholder nodes, so that we have the opportunity to reduce
                 * constant test conditions.  For example this allows
                 *              CASE 0 WHEN 0 THEN 1 ELSE 1/0 END
-                * to reduce to 1 rather than drawing a divide-by-0 error.
+                * to reduce to 1 rather than drawing a divide-by-0 error.  Note
+                * that when the test expression is constant, we don't have to
+                * include it in the resulting CASE; for example
+                *              CASE 0 WHEN x THEN y ELSE z END
+                * is transformed by the parser to
+                *              CASE 0 WHEN CaseTestExpr = x THEN y ELSE z END
+                * which we can simplify to
+                *              CASE WHEN 0 = x THEN y ELSE z END
+                * It is not necessary for the executor to evaluate the "arg"
+                * expression when executing the CASE, since any contained
+                * CaseTestExprs that might have referred to it will have been
+                * replaced by the constant.
                 *----------
                 */
                CaseExpr   *caseexpr = (CaseExpr *) node;
@@ -2602,7 +2613,10 @@ eval_const_expressions_mutator(Node *node,
                /* Set up for contained CaseTestExpr nodes */
                save_case_val = context->case_val;
                if (newarg && IsA(newarg, Const))
+               {
                        context->case_val = newarg;
+                       newarg = NULL;          /* not needed anymore, see comment above */
+               }
                else
                        context->case_val = NULL;
 
index dc17685563bad4f1141b44becbf05f7b204c4b34..21aa6718c8439ede2741b264f726613e2ccf61d0 100644 (file)
@@ -5146,23 +5146,19 @@ get_rule_expr(Node *node, deparse_context *context,
                                                 * boolexpr WHEN TRUE THEN ...", then the optimizer's
                                                 * simplify_boolean_equality() may have reduced this
                                                 * to just "CaseTestExpr" or "NOT CaseTestExpr", for
-                                                * which we have to show "TRUE" or "FALSE".  Also,
-                                                * depending on context the original CaseTestExpr
-                                                * might have been reduced to a Const (but we won't
-                                                * see "WHEN Const").  We have also to consider the
-                                                * possibility that an implicit coercion was inserted
-                                                * between the CaseTestExpr and the operator.
+                                                * which we have to show "TRUE" or "FALSE".  We have
+                                                * also to consider the possibility that an implicit
+                                                * coercion was inserted between the CaseTestExpr and
+                                                * the operator.
                                                 */
                                                if (IsA(w, OpExpr))
                                                {
                                                        List       *args = ((OpExpr *) w)->args;
-                                                       Node       *lhs;
                                                        Node       *rhs;
 
                                                        Assert(list_length(args) == 2);
-                                                       lhs = strip_implicit_coercions(linitial(args));
-                                                       Assert(IsA(lhs, CaseTestExpr) ||
-                                                                  IsA(lhs, Const));
+                                                       Assert(IsA(strip_implicit_coercions(linitial(args)),
+                                                                          CaseTestExpr));
                                                        rhs = (Node *) lsecond(args);
                                                        get_rule_expr(rhs, context, false);
                                                }