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 ff1c314d23ccf09fb922325546f7dcee9df2f4b4..f939dd8b2f4bcb0861b3cf2d8915820974a4365f 100644 (file)
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -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 f750f809a64a356f35f0104c5c3e722870553286..ac905672d2523188773e815f83b7dbade5f1601e 100644 (file)
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -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)
                {
                        /*