]> granicus.if.org Git - postgresql/commitdiff
Fix assorted partition pruning bugs
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 9 May 2018 13:51:23 +0000 (10:51 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 9 May 2018 14:27:04 +0000 (11:27 -0300)
match_clause_to_partition_key failed to consider COERCION_PATH_ARRAYCOERCE
cases in scalar-op-array expressions, so it was possible to crash the
server easily.  To handle this case properly (ie. prune partitions) we
would need to run a bit of executor code during planning.  Maybe it can
be improved, but for now let's just not crash.  Add a test case that
used to trigger the crash.
Author: Michaël Paquier

match_clause_to_partition_key failed to indicate that operators that
don't have a commutator in a btree opclass are unsupported.  It is
possible for this to cause a crash later if such an operator is used in
a scalar-op-array expression.  Add a test case that used to the crash.
Author: Amit Langote

One caller of gen_partprune_steps_internal in
match_clause_to_partition_key was too optimistic about the former never
returning an empty step list.  Rid it of its innocence.  (Having fixed
the bug above, I no longer know how to exploit this, so no test case for
it, but it remained a bug.)  Revise code flow a little bit, for
succintness.
Author: Álvaro Herrera

Reported-by: Marina Polyakova
Reviewed-by: Michaël Paquier
Reviewed-by: Amit Langote
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/ff8f9bfa485ff961d6bb43e54120485b@postgrespro.ru

src/backend/partitioning/partprune.c
src/test/regress/expected/partition_prune.out
src/test/regress/sql/partition_prune.sql

index 69879bf3a45a5521d32e66ca839dd5c75cef6749..58ec2a684d8e5665fe313b8a2a05c11bf10ec8c4 100644 (file)
@@ -573,8 +573,9 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps)
  * For BoolExpr clauses, we recursively generate steps for each argument, and
  * return a PartitionPruneStepCombine of their results.
  *
- * The generated steps are added to the context's steps list.  Each step is
- * assigned a step identifier, unique even across recursive calls.
+ * The return value is a list of the steps generated, which are also added to
+ * the context's steps list.  Each step is assigned a step identifier, unique
+ * even across recursive calls.
  *
  * If we find clauses that are mutually contradictory, or a pseudoconstant
  * clause that contains false, we set *contradictory to true and return NIL
@@ -1601,6 +1602,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
                List       *elem_exprs,
                                   *elem_clauses;
                ListCell   *lc1;
+               bool            contradictory;
 
                if (IsA(leftop, RelabelType))
                        leftop = ((RelabelType *) leftop)->arg;
@@ -1619,7 +1621,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
                 * Only allow strict operators.  This will guarantee nulls are
                 * filtered.
                 */
-               if (!op_strict(saop->opno))
+               if (!op_strict(saop_op))
                        return PARTCLAUSE_UNSUPPORTED;
 
                /* Useless if the array has any volatile functions. */
@@ -1652,6 +1654,8 @@ match_clause_to_partition_key(RelOptInfo *rel,
                                if (strategy != BTEqualStrategyNumber)
                                        return PARTCLAUSE_UNSUPPORTED;
                        }
+                       else
+                               return PARTCLAUSE_UNSUPPORTED; /* no useful negator */
                }
 
                /*
@@ -1692,7 +1696,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
                                elem_exprs = lappend(elem_exprs, elem_expr);
                        }
                }
-               else
+               else if (IsA(rightop, ArrayExpr))
                {
                        ArrayExpr  *arrexpr = castNode(ArrayExpr, rightop);
 
@@ -1706,6 +1710,11 @@ match_clause_to_partition_key(RelOptInfo *rel,
 
                        elem_exprs = arrexpr->elements;
                }
+               else
+               {
+                       /* Give up on any other clause types. */
+                       return PARTCLAUSE_UNSUPPORTED;
+               }
 
                /*
                 * Now generate a list of clauses, one for each array element, of the
@@ -1724,36 +1733,21 @@ match_clause_to_partition_key(RelOptInfo *rel,
                }
 
                /*
-                * Build a combine step as if for an OR clause or add the clauses to
-                * the end of the list that's being processed currently.
+                * If we have an ANY clause and multiple elements, first turn the list
+                * of clauses into an OR expression.
                 */
                if (saop->useOr && list_length(elem_clauses) > 1)
-               {
-                       Expr       *orexpr;
-                       bool            contradictory;
-
-                       orexpr = makeBoolExpr(OR_EXPR, elem_clauses, -1);
-                       *clause_steps =
-                               gen_partprune_steps_internal(context, rel, list_make1(orexpr),
-                                                                                        &contradictory);
-                       if (contradictory)
-                               return PARTCLAUSE_MATCH_CONTRADICT;
-
-                       Assert(list_length(*clause_steps) == 1);
-                       return PARTCLAUSE_MATCH_STEPS;
-               }
-               else
-               {
-                       bool            contradictory;
-
-                       *clause_steps =
-                               gen_partprune_steps_internal(context, rel, elem_clauses,
-                                                                                        &contradictory);
-                       if (contradictory)
-                               return PARTCLAUSE_MATCH_CONTRADICT;
-                       Assert(list_length(*clause_steps) >= 1);
-                       return PARTCLAUSE_MATCH_STEPS;
-               }
+                       elem_clauses = list_make1(makeBoolExpr(OR_EXPR, elem_clauses, -1));
+
+               /* Finally, generate steps */
+               *clause_steps =
+                       gen_partprune_steps_internal(context, rel, elem_clauses,
+                                                                                &contradictory);
+               if (contradictory)
+                       return PARTCLAUSE_MATCH_CONTRADICT;
+               else if (*clause_steps == NIL)
+                       return PARTCLAUSE_UNSUPPORTED;  /* step generation failed */
+               return PARTCLAUSE_MATCH_STEPS;
        }
        else if (IsA(clause, NullTest))
        {
index e0cc5f3393a8db5bc0d1e023117e9a9fed1cc50f..cf331e79c1988d4fbb00e44cd7a8ec69af54aefd 100644 (file)
@@ -1073,6 +1073,72 @@ explain (costs off) select * from boolpart where a is not unknown;
          Filter: (a IS NOT UNKNOWN)
 (7 rows)
 
+-- test scalar-to-array operators
+create table coercepart (a varchar) partition by list (a);
+create table coercepart_ab partition of coercepart for values in ('ab');
+create table coercepart_bc partition of coercepart for values in ('bc');
+create table coercepart_cd partition of coercepart for values in ('cd');
+explain (costs off) select * from coercepart where a in ('ab', to_char(125, '999'));
+                                                          QUERY PLAN                                                          
+------------------------------------------------------------------------------------------------------------------------------
+ Append
+   ->  Seq Scan on coercepart_ab
+         Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+   ->  Seq Scan on coercepart_bc
+         Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+   ->  Seq Scan on coercepart_cd
+         Filter: ((a)::text = ANY ((ARRAY['ab'::character varying, (to_char(125, '999'::text))::character varying])::text[]))
+(7 rows)
+
+explain (costs off) select * from coercepart where a ~ any ('{ab}');
+                     QUERY PLAN                     
+----------------------------------------------------
+ Append
+   ->  Seq Scan on coercepart_ab
+         Filter: ((a)::text ~ ANY ('{ab}'::text[]))
+   ->  Seq Scan on coercepart_bc
+         Filter: ((a)::text ~ ANY ('{ab}'::text[]))
+   ->  Seq Scan on coercepart_cd
+         Filter: ((a)::text ~ ANY ('{ab}'::text[]))
+(7 rows)
+
+explain (costs off) select * from coercepart where a !~ all ('{ab}');
+                     QUERY PLAN                      
+-----------------------------------------------------
+ Append
+   ->  Seq Scan on coercepart_ab
+         Filter: ((a)::text !~ ALL ('{ab}'::text[]))
+   ->  Seq Scan on coercepart_bc
+         Filter: ((a)::text !~ ALL ('{ab}'::text[]))
+   ->  Seq Scan on coercepart_cd
+         Filter: ((a)::text !~ ALL ('{ab}'::text[]))
+(7 rows)
+
+explain (costs off) select * from coercepart where a ~ any ('{ab,bc}');
+                      QUERY PLAN                       
+-------------------------------------------------------
+ Append
+   ->  Seq Scan on coercepart_ab
+         Filter: ((a)::text ~ ANY ('{ab,bc}'::text[]))
+   ->  Seq Scan on coercepart_bc
+         Filter: ((a)::text ~ ANY ('{ab,bc}'::text[]))
+   ->  Seq Scan on coercepart_cd
+         Filter: ((a)::text ~ ANY ('{ab,bc}'::text[]))
+(7 rows)
+
+explain (costs off) select * from coercepart where a !~ all ('{ab,bc}');
+                       QUERY PLAN                       
+--------------------------------------------------------
+ Append
+   ->  Seq Scan on coercepart_ab
+         Filter: ((a)::text !~ ALL ('{ab,bc}'::text[]))
+   ->  Seq Scan on coercepart_bc
+         Filter: ((a)::text !~ ALL ('{ab,bc}'::text[]))
+   ->  Seq Scan on coercepart_cd
+         Filter: ((a)::text !~ ALL ('{ab,bc}'::text[]))
+(7 rows)
+
+drop table coercepart;
 --
 -- some more cases
 --
index 6b7f57ab41380941e9c449b7e8361f3172de0d59..1464f4dcd96585dbf1d5aa7d6f4e4817325ae440 100644 (file)
@@ -152,6 +152,20 @@ explain (costs off) select * from boolpart where a is not true and a is not fals
 explain (costs off) select * from boolpart where a is unknown;
 explain (costs off) select * from boolpart where a is not unknown;
 
+-- test scalar-to-array operators
+create table coercepart (a varchar) partition by list (a);
+create table coercepart_ab partition of coercepart for values in ('ab');
+create table coercepart_bc partition of coercepart for values in ('bc');
+create table coercepart_cd partition of coercepart for values in ('cd');
+
+explain (costs off) select * from coercepart where a in ('ab', to_char(125, '999'));
+explain (costs off) select * from coercepart where a ~ any ('{ab}');
+explain (costs off) select * from coercepart where a !~ all ('{ab}');
+explain (costs off) select * from coercepart where a ~ any ('{ab,bc}');
+explain (costs off) select * from coercepart where a !~ all ('{ab,bc}');
+
+drop table coercepart;
+
 --
 -- some more cases
 --