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 a8972342af495c1a8aa7032edbf1076c463f533c..786ec633106faea56dafd9de0e2057a4b8aaaa5f 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.146 2006/11/28 12:54:41 petere Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.146.2.1 2008/10/25 17:19:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -920,7 +920,8 @@ coerce_to_bigint(ParseState *pstate, Node *node,
  * typeids is a nonempty list of type OIDs.  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.
  */
 Oid
 select_common_type(List *typeids, const char *context)
@@ -951,6 +952,8 @@ select_common_type(List *typeids, 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 7c72ae9e22a7ba4ccca28719e1e10bbae3a83667..595cdfddb8b5afb1d0134bcedd1b73a22b83bb1b 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.198 2006/10/04 00:29:55 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.198.2.1 2008/10/25 17:19:26 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -22,6 +22,7 @@
 #include "nodes/makefuncs.h"
 #include "nodes/plannodes.h"
 #include "optimizer/clauses.h"
+#include "optimizer/var.h"
 #include "parser/analyze.h"
 #include "parser/gramparse.h"
 #include "parser/parse_coerce.h"
@@ -842,25 +843,41 @@ 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))))
        {
                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).
                 */
-               scalar_type = select_common_type(typeids, "IN");
+               scalar_type = select_common_type(typeids, 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)
                {
                        /*