]> 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:31:51 +0000 (15:31 -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 c270b735fecaac40daff3cc8e7bd37ffc45ec453..8902b5b8e6a748f3e56269550750d0916da23b1d 100644 (file)
@@ -2595,7 +2595,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;
@@ -2614,7 +2625,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 5d4c0462d5ad3f0c667720fe596acebb769805f7..72316425d22e0d64fa8571dffea29fbcce78331e 100644 (file)
@@ -4906,23 +4906,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);
                                                }