Skip to content

Commit 4b1fcb4

Browse files
committed
Fix partition pruning to treat stable comparison operators properly.
Cross-type comparison operators in a btree or hash opclass might be only stable not immutable (this is true of timestamp vs. timestamptz for example). partprune.c ignored this possibility and would perform plan-time pruning with them anyway, possibly leading to wrong answers if the environment changed between planning and execution. To fix, teach gen_partprune_steps() to do things differently when creating plan-time pruning steps vs. run-time pruning steps. analyze_partkey_exprs() also needs an extra check, which is rather annoying but now is not the time to restructure things enough to avoid that. While at it, simplify the logic for the plan-time case a little by insisting that the comparison value be a Const and nothing else. This relies on the assumption that eval_const_expressions will have reduced any immutable expression to a Const; which is not quite 100% true, but certainly any case that comes up often enough to be interesting should have simplification logic there. Also improve a bunch of inadequate/obsolete/wrong comments. Per discussion of a report from Alan Jackson (though this fixes only one aspect of that problem). Back-patch to v11 where this code came in. David Rowley, with some further hacking by me Discussion: https://postgr.es/m/FAD28A83-AC73-489E-A058-2681FA31D648@tvsquared.com
1 parent 489e431 commit 4b1fcb4

File tree

3 files changed

+278
-50
lines changed

3 files changed

+278
-50
lines changed

src/backend/partitioning/partprune.c

Lines changed: 153 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
#include "access/nbtree.h"
4040
#include "catalog/pg_operator.h"
4141
#include "catalog/pg_opfamily.h"
42+
#include "catalog/pg_proc.h"
4243
#include "catalog/pg_type.h"
4344
#include "executor/executor.h"
4445
#include "miscadmin.h"
@@ -93,8 +94,12 @@ typedef enum PartClauseMatchStatus
9394
*/
9495
typedef struct GeneratePruningStepsContext
9596
{
97+
/* Input data: */
98+
bool forplanner; /* true when generating steps to be used
99+
* during query planning */
100+
/* Working state and result data: */
96101
int next_step_id;
97-
List *steps;
102+
List *steps; /* output, list of PartitionPruneSteps */
98103
} GeneratePruningStepsContext;
99104

100105
/* The result of performing one PartitionPruneStep */
@@ -117,7 +122,7 @@ static List *make_partitionedrel_pruneinfo(PlannerInfo *root,
117122
List *partitioned_rels, List *prunequal,
118123
Bitmapset **matchedsubplans);
119124
static List *gen_partprune_steps(RelOptInfo *rel, List *clauses,
120-
bool *contradictory);
125+
bool forplanner, bool *contradictory);
121126
static List *gen_partprune_steps_internal(GeneratePruningStepsContext *context,
122127
RelOptInfo *rel, List *clauses,
123128
bool *contradictory);
@@ -409,8 +414,13 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
409414
targetpart->relids);
410415
}
411416

412-
/* Convert pruning qual to pruning steps. */
413-
pruning_steps = gen_partprune_steps(subpart, partprunequal,
417+
/*
418+
* Convert pruning qual to pruning steps. Since these steps will be
419+
* used in the executor, we can pass 'forplanner' as false to allow
420+
* steps to be generated that are unsafe for evaluation during
421+
* planning, e.g. evaluation of stable functions.
422+
*/
423+
pruning_steps = gen_partprune_steps(subpart, partprunequal, false,
414424
&contradictory);
415425

416426
if (contradictory)
@@ -523,22 +533,37 @@ make_partitionedrel_pruneinfo(PlannerInfo *root, RelOptInfo *parentrel,
523533
/*
524534
* gen_partprune_steps
525535
* Process 'clauses' (a rel's baserestrictinfo list of clauses) and return
526-
* a list of "partition pruning steps"
536+
* a list of "partition pruning steps".
537+
*
538+
* 'forplanner' must be true when generating steps to be evaluated during
539+
* query planning, false when generating steps to be used at run-time.
540+
*
541+
* The result generated with forplanner=false includes all clauses that
542+
* are selected with forplanner=true, because in some cases we need a
543+
* combination of clauses to prune successfully. For example, if we
544+
* are partitioning on a hash of columns A and B, and we have clauses
545+
* "WHERE A=constant AND B=nonconstant", we can't do anything at plan
546+
* time even though the first clause would be evaluable then. And we
547+
* must include the first clause when called with forplanner=false,
548+
* or we'll fail to prune at run-time either. This does mean that when
549+
* called with forplanner=false, we may return steps that don't actually
550+
* need to be executed at runtime; it's left to analyze_partkey_exprs()
551+
* to (re)discover that.
527552
*
528553
* If the clauses in the input list are contradictory or there is a
529-
* pseudo-constant "false", *contradictory is set to true upon return.
554+
* pseudo-constant "false", *contradictory is set to true upon return,
555+
* else it's set false.
530556
*/
531557
static List *
532-
gen_partprune_steps(RelOptInfo *rel, List *clauses, bool *contradictory)
558+
gen_partprune_steps(RelOptInfo *rel, List *clauses, bool forplanner,
559+
bool *contradictory)
533560
{
534561
GeneratePruningStepsContext context;
535562

563+
context.forplanner = forplanner;
536564
context.next_step_id = 0;
537565
context.steps = NIL;
538566

539-
/* The clauses list may be modified below, so better make a copy. */
540-
clauses = list_copy(clauses);
541-
542567
/*
543568
* For sub-partitioned tables there's a corner case where if the
544569
* sub-partitioned table shares any partition keys with its parent, then
@@ -563,20 +588,23 @@ gen_partprune_steps(RelOptInfo *rel, List *clauses, bool *contradictory)
563588
if (rel->relid != 1)
564589
ChangeVarNodes((Node *) partqual, 1, rel->relid, 0);
565590

566-
clauses = list_concat(clauses, partqual);
591+
/* Use list_copy to avoid modifying the passed-in List */
592+
clauses = list_concat(list_copy(clauses), partqual);
567593
}
568594

569595
/* Down into the rabbit-hole. */
570-
gen_partprune_steps_internal(&context, rel, clauses, contradictory);
596+
(void) gen_partprune_steps_internal(&context, rel, clauses, contradictory);
571597

572598
return context.steps;
573599
}
574600

575601
/*
576602
* prune_append_rel_partitions
577-
* Returns indexes into rel->part_rels of the minimum set of child
578-
* partitions which must be scanned to satisfy rel's baserestrictinfo
579-
* quals.
603+
* Process rel's baserestrictinfo and make use of quals which can be
604+
* evaluated during query planning in order to determine the minimum set
605+
* of partitions which must be scanned to satisfy these quals. Returns
606+
* the matching partitions in the form of a Bitmapset containing the
607+
* partitions' indexes in the rel's part_rels array.
580608
*
581609
* Callers must ensure that 'rel' is a partitioned table.
582610
*/
@@ -603,9 +631,12 @@ prune_append_rel_partitions(RelOptInfo *rel)
603631

604632
/*
605633
* Process clauses. If the clauses are found to be contradictory, we can
606-
* return the empty set.
634+
* return the empty set. Pass 'forplanner' as true to indicate to the
635+
* pruning code that we only want pruning steps that can be evaluated
636+
* during planning.
607637
*/
608-
pruning_steps = gen_partprune_steps(rel, clauses, &contradictory);
638+
pruning_steps = gen_partprune_steps(rel, clauses, true,
639+
&contradictory);
609640
if (contradictory)
610641
return NULL;
611642

@@ -635,6 +666,9 @@ prune_append_rel_partitions(RelOptInfo *rel)
635666
* get_matching_partitions
636667
* Determine partitions that survive partition pruning
637668
*
669+
* Note: context->planstate must be set to a valid PlanState when the
670+
* pruning_steps were generated with 'forplanner' = false.
671+
*
638672
* Returns a Bitmapset of the RelOptInfo->part_rels indexes of the surviving
639673
* partitions.
640674
*/
@@ -755,9 +789,6 @@ get_matching_partitions(PartitionPruneContext *context, List *pruning_steps)
755789
* clause that contains false, we set *contradictory to true and return NIL
756790
* (that is, no pruning steps). Caller should consider all partitions as
757791
* pruned in that case. Otherwise, *contradictory is set to false.
758-
*
759-
* Note: the 'clauses' List may be modified inside this function. Callers may
760-
* like to make a copy of it before passing them to this function.
761792
*/
762793
static List *
763794
gen_partprune_steps_internal(GeneratePruningStepsContext *context,
@@ -1572,8 +1603,7 @@ match_clause_to_partition_key(RelOptInfo *rel,
15721603
Expr *expr;
15731604

15741605
/*
1575-
* Recognize specially shaped clauses that match with the Boolean
1576-
* partition key.
1606+
* Recognize specially shaped clauses that match a Boolean partition key.
15771607
*/
15781608
if (match_boolean_partition_clause(partopfamily, clause, partkey, &expr))
15791609
{
@@ -1648,16 +1678,47 @@ match_clause_to_partition_key(RelOptInfo *rel,
16481678
* Matched with this key. Now check various properties of the clause
16491679
* to see if it's sane to use it for pruning. In most of these cases,
16501680
* we can return UNSUPPORTED because the same failure would occur no
1651-
* matter which partkey it's matched to.
1681+
* matter which partkey it's matched to. (In particular, now that
1682+
* we've successfully matched one side of the opclause to a partkey,
1683+
* there is no chance that matching the other side to another partkey
1684+
* will produce a usable result, since that'd mean there are Vars on
1685+
* both sides.)
16521686
*/
1687+
if (context->forplanner)
1688+
{
1689+
/*
1690+
* When pruning in the planner, we only support pruning using
1691+
* comparisons to constants. Immutable subexpressions will have
1692+
* been folded to constants already, and we cannot prune on the
1693+
* basis of anything that's not immutable.
1694+
*/
1695+
if (!IsA(expr, Const))
1696+
return PARTCLAUSE_UNSUPPORTED;
16531697

1654-
/*
1655-
* We can't prune using an expression with Vars. (Report failure as
1656-
* UNSUPPORTED, not NOMATCH: as in the no-commutator case above, we
1657-
* now know there are Vars on both sides, so it's no good.)
1658-
*/
1659-
if (contain_var_clause((Node *) expr))
1660-
return PARTCLAUSE_UNSUPPORTED;
1698+
/*
1699+
* Also, the comparison operator itself must be immutable.
1700+
*/
1701+
if (op_volatile(opno) != PROVOLATILE_IMMUTABLE)
1702+
return PARTCLAUSE_UNSUPPORTED;
1703+
}
1704+
else
1705+
{
1706+
/*
1707+
* Otherwise, non-consts are allowed, but we can't prune using an
1708+
* expression that contains Vars.
1709+
*/
1710+
if (contain_var_clause((Node *) expr))
1711+
return PARTCLAUSE_UNSUPPORTED;
1712+
1713+
/*
1714+
* And we must reject anything containing a volatile function.
1715+
* Stable functions are OK though. (We need not check this for
1716+
* the comparison operator itself: anything that belongs to a
1717+
* partitioning operator family must be at least stable.)
1718+
*/
1719+
if (contain_volatile_functions((Node *) expr))
1720+
return PARTCLAUSE_UNSUPPORTED;
1721+
}
16611722

16621723
/*
16631724
* Only allow strict operators. This will guarantee nulls are
@@ -1666,10 +1727,6 @@ match_clause_to_partition_key(RelOptInfo *rel,
16661727
if (!op_strict(opno))
16671728
return PARTCLAUSE_UNSUPPORTED;
16681729

1669-
/* We can't use any volatile expressions to prune partitions. */
1670-
if (contain_volatile_functions((Node *) expr))
1671-
return PARTCLAUSE_UNSUPPORTED;
1672-
16731730
/*
16741731
* See if the operator is relevant to the partitioning opfamily.
16751732
*
@@ -1798,20 +1855,51 @@ match_clause_to_partition_key(RelOptInfo *rel,
17981855
if (IsA(leftop, RelabelType))
17991856
leftop = ((RelabelType *) leftop)->arg;
18001857

1801-
/* Check it matches this partition key */
1858+
/* check if the LHS matches this partition key */
18021859
if (!equal(leftop, partkey) ||
18031860
!PartCollMatchesExprColl(partcoll, saop->inputcollid))
18041861
return PARTCLAUSE_NOMATCH;
18051862

18061863
/*
18071864
* Matched with this key. Check various properties of the clause to
1808-
* see if it can sanely be used for partition pruning (this is mostly
1809-
* the same as for a plain OpExpr).
1865+
* see if it can sanely be used for partition pruning (this is
1866+
* identical to the logic for a plain OpExpr).
18101867
*/
1868+
if (context->forplanner)
1869+
{
1870+
/*
1871+
* When pruning in the planner, we only support pruning using
1872+
* comparisons to constants. Immutable subexpressions will have
1873+
* been folded to constants already, and we cannot prune on the
1874+
* basis of anything that's not immutable.
1875+
*/
1876+
if (!IsA(rightop, Const))
1877+
return PARTCLAUSE_UNSUPPORTED;
18111878

1812-
/* We can't prune using an expression with Vars. */
1813-
if (contain_var_clause((Node *) rightop))
1814-
return PARTCLAUSE_UNSUPPORTED;
1879+
/*
1880+
* Also, the comparison operator itself must be immutable.
1881+
*/
1882+
if (op_volatile(saop_op) != PROVOLATILE_IMMUTABLE)
1883+
return PARTCLAUSE_UNSUPPORTED;
1884+
}
1885+
else
1886+
{
1887+
/*
1888+
* Otherwise, non-consts are allowed, but we can't prune using an
1889+
* expression that contains Vars.
1890+
*/
1891+
if (contain_var_clause((Node *) rightop))
1892+
return PARTCLAUSE_UNSUPPORTED;
1893+
1894+
/*
1895+
* And we must reject anything containing a volatile function.
1896+
* Stable functions are OK though. (We need not check this for
1897+
* the comparison operator itself: anything that belongs to a
1898+
* partitioning operator family must be at least stable.)
1899+
*/
1900+
if (contain_volatile_functions((Node *) rightop))
1901+
return PARTCLAUSE_UNSUPPORTED;
1902+
}
18151903

18161904
/*
18171905
* Only allow strict operators. This will guarantee nulls are
@@ -1820,10 +1908,6 @@ match_clause_to_partition_key(RelOptInfo *rel,
18201908
if (!op_strict(saop_op))
18211909
return PARTCLAUSE_UNSUPPORTED;
18221910

1823-
/* We can't use any volatile expressions to prune partitions. */
1824-
if (contain_volatile_functions((Node *) rightop))
1825-
return PARTCLAUSE_UNSUPPORTED;
1826-
18271911
/*
18281912
* In case of NOT IN (..), we get a '<>', which we handle if list
18291913
* partitioning is in use and we're able to confirm that it's negator
@@ -2948,8 +3032,14 @@ analyze_partkey_exprs(PartitionedRelPruneInfo *pinfo, List *steps,
29483032

29493033
/*
29503034
* Steps require run-time pruning if they contain EXEC_PARAM Params.
2951-
* Otherwise, if their expressions aren't simple Consts, they require
2952-
* startup-time pruning.
3035+
* Otherwise, if their expressions aren't simple Consts or they involve
3036+
* non-immutable comparison operators, they require startup-time pruning.
3037+
* (Otherwise, the pruning would have been done at plan time.)
3038+
*
3039+
* Notice that what we actually check for mutability is the comparison
3040+
* functions, not the original operators. This relies on the support
3041+
* functions of the btree or hash opfamily being marked consistently with
3042+
* the operators.
29533043
*/
29543044
pinfo->nexprs = list_length(steps) * partnatts;
29553045
pinfo->hasexecparam = (bool *) palloc0(sizeof(bool) * pinfo->nexprs);
@@ -2961,15 +3051,18 @@ analyze_partkey_exprs(PartitionedRelPruneInfo *pinfo, List *steps,
29613051
{
29623052
PartitionPruneStepOp *step = (PartitionPruneStepOp *) lfirst(lc);
29633053
ListCell *lc2;
3054+
ListCell *lc3;
29643055
int keyno;
29653056

29663057
if (!IsA(step, PartitionPruneStepOp))
29673058
continue;
29683059

29693060
keyno = 0;
2970-
foreach(lc2, step->exprs)
3061+
Assert(list_length(step->exprs) == list_length(step->cmpfns));
3062+
forboth(lc2, step->exprs, lc3, step->cmpfns)
29713063
{
29723064
Expr *expr = lfirst(lc2);
3065+
Oid fnoid = lfirst_oid(lc3);
29733066

29743067
if (!IsA(expr, Const))
29753068
{
@@ -2992,6 +3085,12 @@ analyze_partkey_exprs(PartitionedRelPruneInfo *pinfo, List *steps,
29923085

29933086
doruntimeprune = true;
29943087
}
3088+
else if (func_volatile(fnoid) != PROVOLATILE_IMMUTABLE)
3089+
{
3090+
/* No exec params here, but must do initial pruning */
3091+
pinfo->do_initial_prune = true;
3092+
doruntimeprune = true;
3093+
}
29953094
keyno++;
29963095
}
29973096
}
@@ -3346,6 +3445,12 @@ partkey_datum_from_expr(PartitionPruneContext *context,
33463445
}
33473446
else
33483447
{
3448+
/*
3449+
* We should never see a non-Const in a step unless we're running in
3450+
* the executor.
3451+
*/
3452+
Assert(context->planstate != NULL);
3453+
33493454
/*
33503455
* When called from the executor we'll have a valid planstate so we
33513456
* may be able to evaluate an expression which could not be folded to
@@ -3354,9 +3459,7 @@ partkey_datum_from_expr(PartitionPruneContext *context,
33543459
* must be careful here to evaluate expressions containing PARAM_EXEC
33553460
* Params only when told it's OK.
33563461
*/
3357-
if (context->planstate &&
3358-
(context->evalexecparams ||
3359-
!context->exprhasexecparam[stateidx]))
3462+
if (context->evalexecparams || !context->exprhasexecparam[stateidx])
33603463
{
33613464
ExprState *exprstate;
33623465
ExprContext *ectx;

0 commit comments

Comments
 (0)