Add a heuristic to transformAExprIn() to make it prefer expanding "x IN (list)"

into an OR of equality comparisons, rather than x = ANY(ARRAY[...]), when there
are Vars in the right-hand side.  This avoids a performance regression compared
to pre-8.2 releases, in cases where the OR form can be optimized into scans
of multiple indexes.  Limit the possible downside by preferring this form only
when the list isn't very long (I set the cutoff at 32 elements, which is a
bit arbitrary but in the right ballpark).  Per discussion with Jim Nasby.

In passing, also make it try the OR form if it cannot select a common type
for the array elements; we've seen a complaint or two about how the OR form
worked for such cases and ARRAY doesn't.

diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index 7c66c6380476d4597f8f43147fc5704c46b11e63..6d703ae571884d9c5cba8252e99020ed7e5fd0a0 100644 (file)
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.169 2008/10/13 16:25:19 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.170 2008/10/25 17:19:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1085,13 +1085,14 @@ parser_coercion_errposition(ParseState *pstate,
 /*
  * select_common_type()
  *             Determine the common supertype of a list of input expressions.
- *             This is used for determining the output type of CASE and UNION
- *             constructs.
+ *             This is used for determining the output type of CASE, UNION,
+ *             and similar constructs.
  *
  * 'exprs' is a *nonempty* list of expressions.  Note that earlier items
  * in the list will be preferred if there is doubt.
  * 'context' is a phrase to use in the error message if we fail to select
- * a usable type.
+ * a usable type.  Pass NULL to have the routine return InvalidOid
+ * rather than throwing an error on failure.
  * 'which_expr': if not NULL, receives a pointer to the particular input
  * expression from which the result type was taken.
  */
@@ -1166,6 +1167,8 @@ select_common_type(ParseState *pstate, List *exprs, const char *context,
                                /*
                                 * both types in different categories? then not much hope...
                                 */
+                               if (context == NULL)
+                                       return InvalidOid;
                                ereport(ERROR,
                                                (errcode(ERRCODE_DATATYPE_MISMATCH),
                                /*------

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index ddd041818a54450b8e8e86bac74783e93fede28e..ced222578db86251b5fd08cf8e1543be1d89f160 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.235 2008/10/06 17:39:26 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.236 2008/10/25 17:19:09 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -20,6 +20,7 @@
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
+#include "optimizer/var.h"
 #include "parser/analyze.h"
 #include "parser/parse_coerce.h"
 #include "parser/parse_expr.h"
@@ -974,29 +975,45 @@ transformAExprIn(ParseState *pstate, A_Expr *a)
        }
 
        /*
-        * If not forced by presence of RowExpr, try to resolve a common scalar
-        * type for all the expressions, and see if it has an array type. (But if
-        * there's only one righthand expression, we may as well just fall through
-        * and generate a simple = comparison.)
+        * We prefer a boolean tree to ScalarArrayOpExpr if any of these are true:
+        *
+        * 1. We have a RowExpr anywhere.
+        *
+        * 2. There's only one righthand expression --- best to just generate a
+        * simple = comparison.
+        *
+        * 3. There's a reasonably small number of righthand expressions and
+        * they contain any Vars.  This is a heuristic to support cases like
+        * WHERE '555-1212' IN (tab.home_phone, tab.work_phone), which can be
+        * optimized into an OR of indexscans on different indexes so long as
+        * it's left as an OR tree.  (It'd be better to leave this decision
+        * to the planner, no doubt, but the amount of code required to reformat
+        * the expression later on seems out of proportion to the benefit.)
         */
-       if (!haveRowExpr && list_length(rexprs) != 1)
+       if (!(haveRowExpr ||
+                 list_length(rexprs) == 1 ||
+                 (list_length(rexprs) <= 32 &&
+                  contain_vars_of_level((Node *) rexprs, 0))))
        {
                List       *allexprs;
                Oid                     scalar_type;
                Oid                     array_type;
 
                /*
-                * Select a common type for the array elements.  Note that since the
-                * LHS' type is first in the list, it will be preferred when there is
-                * doubt (eg, when all the RHS items are unknown literals).
+                * Try to select a common type for the array elements.  Note that
+                * since the LHS' type is first in the list, it will be preferred when
+                * there is doubt (eg, when all the RHS items are unknown literals).
                 *
                 * Note: use list_concat here not lcons, to avoid damaging rexprs.
                 */
                allexprs = list_concat(list_make1(lexpr), rexprs);
-               scalar_type = select_common_type(pstate, allexprs, "IN", NULL);
+               scalar_type = select_common_type(pstate, allexprs, NULL, NULL);
 
                /* Do we have an array type to use? */
-               array_type = get_array_type(scalar_type);
+               if (OidIsValid(scalar_type))
+                       array_type = get_array_type(scalar_type);
+               else
+                       array_type = InvalidOid;
                if (array_type != InvalidOid)
                {
                        /*