From 0901dbab338f3161b50c4c60ef669ed393c9e308 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 29 Apr 2014 13:12:33 -0400 Subject: [PATCH] Improve planner to drop constant-NULL inputs of AND/OR where it's legal. In general we can't discard constant-NULL inputs, since they could change the result of the AND/OR to be NULL. But at top level of WHERE, we do not need to distinguish a NULL result from a FALSE result, so it's okay to treat NULL as FALSE and then simplify AND/OR accordingly. This is a very ancient oversight, but in 9.2 and later it can lead to failure to optimize queries that previous releases did optimize, as a result of more aggressive parameter substitution rules making it possible to reduce more subexpressions to NULL constants. This is the root cause of bug #10171 from Arnold Scheffler. We could alternatively have fixed that by teaching orclauses.c to ignore constant-NULL OR arms, but it seems better to get rid of them globally. I resisted the temptation to back-patch this change into all active branches, but it seems appropriate to back-patch as far as 9.2 so that there will not be performance regressions of the kind shown in this bug. --- src/backend/optimizer/prep/prepqual.c | 79 ++++++++++++++++++---- src/test/regress/expected/create_index.out | 11 +++ src/test/regress/sql/create_index.sql | 7 ++ 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/src/backend/optimizer/prep/prepqual.c b/src/backend/optimizer/prep/prepqual.c index 3760fdc645..e7a6c80131 100644 --- a/src/backend/optimizer/prep/prepqual.c +++ b/src/backend/optimizer/prep/prepqual.c @@ -293,7 +293,7 @@ canonicalize_qual(Expr *qual) /* * Pull up redundant subclauses in OR-of-AND trees. We do this only * within the top-level AND/OR structure; there's no point in looking - * deeper. + * deeper. Also remove any NULL constants in the top-level structure. */ newqual = find_duplicate_ors(qual); @@ -393,6 +393,13 @@ pull_ors(List *orlist) * OR clauses to which the inverse OR distributive law might apply. * Only the top-level AND/OR structure is searched. * + * While at it, we remove any NULL constants within the top-level AND/OR + * structure, eg "x OR NULL::boolean" is reduced to "x". In general that + * would change the result, so eval_const_expressions can't do it; but at + * top level of WHERE, we don't need to distinguish between FALSE and NULL + * results, so it's valid to treat NULL::boolean the same as FALSE and then + * simplify AND/OR accordingly. + * * Returns the modified qualification. AND/OR flatness is preserved. */ static Expr * @@ -405,12 +412,30 @@ find_duplicate_ors(Expr *qual) /* Recurse */ foreach(temp, ((BoolExpr *) qual)->args) - orlist = lappend(orlist, find_duplicate_ors(lfirst(temp))); + { + Expr *arg = (Expr *) lfirst(temp); - /* - * Don't need pull_ors() since this routine will never introduce an OR - * where there wasn't one before. - */ + arg = find_duplicate_ors(arg); + + /* Get rid of any constant inputs */ + if (arg && IsA(arg, Const)) + { + Const *carg = (Const *) arg; + + /* Drop constant FALSE or NULL */ + if (carg->constisnull || !DatumGetBool(carg->constvalue)) + continue; + /* constant TRUE, so OR reduces to TRUE */ + return arg; + } + + orlist = lappend(orlist, arg); + } + + /* Flatten any ORs pulled up to just below here */ + orlist = pull_ors(orlist); + + /* Now we can look for duplicate ORs */ return process_duplicate_ors(orlist); } else if (and_clause((Node *) qual)) @@ -420,10 +445,38 @@ find_duplicate_ors(Expr *qual) /* Recurse */ foreach(temp, ((BoolExpr *) qual)->args) - andlist = lappend(andlist, find_duplicate_ors(lfirst(temp))); + { + Expr *arg = (Expr *) lfirst(temp); + + arg = find_duplicate_ors(arg); + + /* Get rid of any constant inputs */ + if (arg && IsA(arg, Const)) + { + Const *carg = (Const *) arg; + + /* Drop constant TRUE */ + if (!carg->constisnull && DatumGetBool(carg->constvalue)) + continue; + /* constant FALSE or NULL, so AND reduces to FALSE */ + return (Expr *) makeBoolConst(false, false); + } + + andlist = lappend(andlist, arg); + } + /* Flatten any ANDs introduced just below here */ andlist = pull_ands(andlist); - /* The AND list can't get shorter, so result is always an AND */ + + /* AND of no inputs reduces to TRUE */ + if (andlist == NIL) + return (Expr *) makeBoolConst(true, false); + + /* Single-expression AND just reduces to that expression */ + if (list_length(andlist) == 1) + return (Expr *) linitial(andlist); + + /* Else we still need an AND node */ return make_andclause(andlist); } else @@ -447,11 +500,13 @@ process_duplicate_ors(List *orlist) List *neworlist; ListCell *temp; + /* OR of no inputs reduces to FALSE */ if (orlist == NIL) - return NULL; /* probably can't happen */ - if (list_length(orlist) == 1) /* single-expression OR (can this - * happen?) */ - return linitial(orlist); + return (Expr *) makeBoolConst(false, false); + + /* Single-expression OR just reduces to that expression */ + if (list_length(orlist) == 1) + return (Expr *) linitial(orlist); /* * Choose the shortest AND clause as the reference list --- obviously, any diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index da3bdfbe07..650197a784 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2741,3 +2741,14 @@ ORDER BY thousand; 1 | 1001 (2 rows) +-- +-- Check elimination of constant-NULL subexpressions +-- +explain (costs off) + select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null)); + QUERY PLAN +------------------------------------------------------ + Index Scan using tenk1_thous_tenthous on tenk1 + Index Cond: ((thousand = 1) AND (tenthous = 1001)) +(2 rows) + diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql index 6835c884c6..c7644c0fb2 100644 --- a/src/test/regress/sql/create_index.sql +++ b/src/test/regress/sql/create_index.sql @@ -915,3 +915,10 @@ ORDER BY thousand; SELECT thousand, tenthous FROM tenk1 WHERE thousand < 2 AND tenthous IN (1001,3000) ORDER BY thousand; + +-- +-- Check elimination of constant-NULL subexpressions +-- + +explain (costs off) + select * from tenk1 where (thousand, tenthous) in ((1,1001), (null,null)); -- 2.40.0