Skip to content

Commit 0b2e77c

Browse files
david-rowleydavid-rowley
committed
Fix incorrect partition pruning logic for boolean partitioned tables
The partition pruning logic assumed that "b IS NOT true" was exactly the same as "b IS FALSE". This is not the case when considering NULL values. Fix this so we correctly include any partition which could hold NULL values for the NOT case. Additionally, this fixes a bug in the partition pruning code which handles partitioned tables partitioned like ((NOT boolcol)). This is a seemingly unlikely schema design, and it was untested and also broken. Here we add tests for the ((NOT boolcol)) case and insert some actual data into those tables and verify we do get the correct rows back when running queries. I've also adjusted the existing boolpart tests to include some data and verify we get the correct results too. Both the bugs being fixed here could lead to incorrect query results with fewer rows being returned than expected. No additional rows could have been returned accidentally. In passing, remove needless ternary expression. It's more simple just to pass !is_not_clause to makeBoolConst(). It makes sense to do this so the code is consistent with the bug fix in the "else if" condition just below. David Kimura did submit a patch to fix the first of the issues here, but that's not what's being committed here. Reported-by: David Kimura Reviewed-by: Richard Guo, David Kimura Discussion: https://postgr.es/m/CAHnPFjQ5qxs6J_p+g8=ww7GQvfn71_JE+Tygj0S7RdRci1uwPw@mail.gmail.com Backpatch-through: 11, all supported versions
1 parent 953ff99 commit 0b2e77c

File tree

3 files changed

+288
-31
lines changed

3 files changed

+288
-31
lines changed

‎src/backend/partitioning/partprune.c

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,8 @@ static PruneStepResult *perform_pruning_combine_step(PartitionPruneContext *cont
197197
static PartClauseMatchStatus match_boolean_partition_clause(Oid partopfamily,
198198
Expr *clause,
199199
Expr *partkey,
200-
Expr **outconst);
200+
Expr **outconst,
201+
bool *noteq);
201202
static void partkey_datum_from_expr(PartitionPruneContext *context,
202203
Expr *expr, int stateidx,
203204
Datum *value, bool *isnull);
@@ -1715,12 +1716,13 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
17151716
Oid partopfamily = part_scheme->partopfamily[partkeyidx],
17161717
partcoll = part_scheme->partcollation[partkeyidx];
17171718
Expr *expr;
1719+
bool noteq;
17181720

17191721
/*
17201722
* Recognize specially shaped clauses that match a Boolean partition key.
17211723
*/
17221724
boolmatchstatus = match_boolean_partition_clause(partopfamily, clause,
1723-
partkey, &expr);
1725+
partkey, &expr, &noteq);
17241726

17251727
if (boolmatchstatus == PARTCLAUSE_MATCH_CLAUSE)
17261728
{
@@ -1730,7 +1732,7 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
17301732
partclause->keyno = partkeyidx;
17311733
/* Do pruning with the Boolean equality operator. */
17321734
partclause->opno = BooleanEqualOperator;
1733-
partclause->op_is_ne = false;
1735+
partclause->op_is_ne = noteq;
17341736
partclause->expr = expr;
17351737
/* We know that expr is of Boolean type. */
17361738
partclause->cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
@@ -3489,20 +3491,22 @@ perform_pruning_combine_step(PartitionPruneContext *context,
34893491
* match_boolean_partition_clause
34903492
*
34913493
* If we're able to match the clause to the partition key as specially-shaped
3492-
* boolean clause, set *outconst to a Const containing a true or false value
3493-
* and return PARTCLAUSE_MATCH_CLAUSE. Returns PARTCLAUSE_UNSUPPORTED if the
3494-
* clause is not a boolean clause or if the boolean clause is unsuitable for
3495-
* partition pruning. Returns PARTCLAUSE_NOMATCH if it's a bool quals but
3496-
* just does not match this partition key. *outconst is set to NULL in the
3497-
* latter two cases.
3494+
* boolean clause, set *outconst to a Const containing a true or false value,
3495+
* set *noteq according to if the clause was in the "not" form, i.e. "is not
3496+
* true" or "is not false", and return PARTCLAUSE_MATCH_CLAUSE. Returns
3497+
* PARTCLAUSE_UNSUPPORTED if the clause is not a boolean clause or if the
3498+
* boolean clause is unsuitable for partition pruning. Returns
3499+
* PARTCLAUSE_NOMATCH if it's a bool quals but just does not match this
3500+
* partition key. *outconst is set to NULL in the latter two cases.
34983501
*/
34993502
static PartClauseMatchStatus
35003503
match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
3501-
Expr **outconst)
3504+
Expr **outconst, bool *noteq)
35023505
{
35033506
Expr *leftop;
35043507

35053508
*outconst = NULL;
3509+
*noteq = false;
35063510

35073511
if (!IsBooleanOpfamily(partopfamily))
35083512
return PARTCLAUSE_UNSUPPORTED;
@@ -3521,11 +3525,25 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
35213525
leftop = ((RelabelType *) leftop)->arg;
35223526

35233527
if (equal(leftop, partkey))
3524-
*outconst = (btest->booltesttype == IS_TRUE ||
3525-
btest->booltesttype == IS_NOT_FALSE)
3526-
? (Expr *) makeBoolConst(true, false)
3527-
: (Expr *) makeBoolConst(false, false);
3528-
3528+
{
3529+
switch (btest->booltesttype)
3530+
{
3531+
case IS_NOT_TRUE:
3532+
*noteq = true;
3533+
/* fall through */
3534+
case IS_TRUE:
3535+
*outconst = (Expr *) makeBoolConst(true, false);
3536+
break;
3537+
case IS_NOT_FALSE:
3538+
*noteq = true;
3539+
/* fall through */
3540+
case IS_FALSE:
3541+
*outconst = (Expr *) makeBoolConst(false, false);
3542+
break;
3543+
default:
3544+
return PARTCLAUSE_UNSUPPORTED;
3545+
}
3546+
}
35293547
if (*outconst)
35303548
return PARTCLAUSE_MATCH_CLAUSE;
35313549
}
@@ -3540,11 +3558,9 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
35403558

35413559
/* Compare to the partition key, and make up a clause ... */
35423560
if (equal(leftop, partkey))
3543-
*outconst = is_not_clause ?
3544-
(Expr *) makeBoolConst(false, false) :
3545-
(Expr *) makeBoolConst(true, false);
3561+
*outconst = (Expr *) makeBoolConst(!is_not_clause, false);
35463562
else if (equal(negate_clause((Node *) leftop), partkey))
3547-
*outconst = (Expr *) makeBoolConst(false, false);
3563+
*outconst = (Expr *) makeBoolConst(is_not_clause, false);
35483564

35493565
if (*outconst)
35503566
return PARTCLAUSE_MATCH_CLAUSE;

‎src/test/regress/expected/partition_prune.out

Lines changed: 216 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,7 @@ create table boolpart (a bool) partition by list (a);
10181018
create table boolpart_default partition of boolpart default;
10191019
create table boolpart_t partition of boolpart for values in ('true');
10201020
create table boolpart_f partition of boolpart for values in ('false');
1021+
insert into boolpart values (true), (false), (null);
10211022
explain (costs off) select * from boolpart where a in (true, false);
10221023
QUERY PLAN
10231024
------------------------------------------------
@@ -1050,20 +1051,25 @@ explain (costs off) select * from boolpart where a is true or a is not true;
10501051
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
10511052
-> Seq Scan on boolpart_t
10521053
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1053-
(5 rows)
1054+
-> Seq Scan on boolpart_default
1055+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1056+
(7 rows)
10541057

10551058
explain (costs off) select * from boolpart where a is not true;
1056-
QUERY PLAN
1057-
---------------------------
1058-
Seq Scan on boolpart_f
1059-
Filter: (a IS NOT TRUE)
1060-
(2 rows)
1059+
QUERY PLAN
1060+
------------------------------------
1061+
Append
1062+
-> Seq Scan on boolpart_f
1063+
Filter: (a IS NOT TRUE)
1064+
-> Seq Scan on boolpart_default
1065+
Filter: (a IS NOT TRUE)
1066+
(5 rows)
10611067

10621068
explain (costs off) select * from boolpart where a is not true and a is not false;
1063-
QUERY PLAN
1064-
--------------------------
1065-
Result
1066-
One-Time Filter: false
1069+
QUERY PLAN
1070+
--------------------------------------------------
1071+
Seq Scan on boolpart_default
1072+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
10671073
(2 rows)
10681074

10691075
explain (costs off) select * from boolpart where a is unknown;
@@ -1090,6 +1096,205 @@ explain (costs off) select * from boolpart where a is not unknown;
10901096
Filter: (a IS NOT UNKNOWN)
10911097
(7 rows)
10921098

1099+
select * from boolpart where a in (true, false);
1100+
a
1101+
---
1102+
f
1103+
t
1104+
(2 rows)
1105+
1106+
select * from boolpart where a = false;
1107+
a
1108+
---
1109+
f
1110+
(1 row)
1111+
1112+
select * from boolpart where not a = false;
1113+
a
1114+
---
1115+
t
1116+
(1 row)
1117+
1118+
select * from boolpart where a is true or a is not true;
1119+
a
1120+
---
1121+
f
1122+
t
1123+
1124+
(3 rows)
1125+
1126+
select * from boolpart where a is not true;
1127+
a
1128+
---
1129+
f
1130+
1131+
(2 rows)
1132+
1133+
select * from boolpart where a is not true and a is not false;
1134+
a
1135+
---
1136+
1137+
(1 row)
1138+
1139+
select * from boolpart where a is unknown;
1140+
a
1141+
---
1142+
1143+
(1 row)
1144+
1145+
select * from boolpart where a is not unknown;
1146+
a
1147+
---
1148+
f
1149+
t
1150+
(2 rows)
1151+
1152+
-- inverse boolean partitioning - a seemingly unlikely design, but we've got
1153+
-- code for it, so we'd better test it.
1154+
create table iboolpart (a bool) partition by list ((not a));
1155+
create table iboolpart_default partition of iboolpart default;
1156+
create table iboolpart_f partition of iboolpart for values in ('true');
1157+
create table iboolpart_t partition of iboolpart for values in ('false');
1158+
insert into iboolpart values (true), (false), (null);
1159+
explain (costs off) select * from iboolpart where a in (true, false);
1160+
QUERY PLAN
1161+
------------------------------------------------
1162+
Append
1163+
-> Seq Scan on iboolpart_t
1164+
Filter: (a = ANY ('{t,f}'::boolean[]))
1165+
-> Seq Scan on iboolpart_f
1166+
Filter: (a = ANY ('{t,f}'::boolean[]))
1167+
-> Seq Scan on iboolpart_default
1168+
Filter: (a = ANY ('{t,f}'::boolean[]))
1169+
(7 rows)
1170+
1171+
explain (costs off) select * from iboolpart where a = false;
1172+
QUERY PLAN
1173+
-------------------------
1174+
Seq Scan on iboolpart_f
1175+
Filter: (NOT a)
1176+
(2 rows)
1177+
1178+
explain (costs off) select * from iboolpart where not a = false;
1179+
QUERY PLAN
1180+
-------------------------
1181+
Seq Scan on iboolpart_t
1182+
Filter: a
1183+
(2 rows)
1184+
1185+
explain (costs off) select * from iboolpart where a is true or a is not true;
1186+
QUERY PLAN
1187+
--------------------------------------------------
1188+
Append
1189+
-> Seq Scan on iboolpart_t
1190+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1191+
-> Seq Scan on iboolpart_f
1192+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1193+
-> Seq Scan on iboolpart_default
1194+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1195+
(7 rows)
1196+
1197+
explain (costs off) select * from iboolpart where a is not true;
1198+
QUERY PLAN
1199+
-------------------------------------
1200+
Append
1201+
-> Seq Scan on iboolpart_t
1202+
Filter: (a IS NOT TRUE)
1203+
-> Seq Scan on iboolpart_f
1204+
Filter: (a IS NOT TRUE)
1205+
-> Seq Scan on iboolpart_default
1206+
Filter: (a IS NOT TRUE)
1207+
(7 rows)
1208+
1209+
explain (costs off) select * from iboolpart where a is not true and a is not false;
1210+
QUERY PLAN
1211+
--------------------------------------------------------
1212+
Append
1213+
-> Seq Scan on iboolpart_t
1214+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1215+
-> Seq Scan on iboolpart_f
1216+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1217+
-> Seq Scan on iboolpart_default
1218+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1219+
(7 rows)
1220+
1221+
explain (costs off) select * from iboolpart where a is unknown;
1222+
QUERY PLAN
1223+
-------------------------------------
1224+
Append
1225+
-> Seq Scan on iboolpart_t
1226+
Filter: (a IS UNKNOWN)
1227+
-> Seq Scan on iboolpart_f
1228+
Filter: (a IS UNKNOWN)
1229+
-> Seq Scan on iboolpart_default
1230+
Filter: (a IS UNKNOWN)
1231+
(7 rows)
1232+
1233+
explain (costs off) select * from iboolpart where a is not unknown;
1234+
QUERY PLAN
1235+
-------------------------------------
1236+
Append
1237+
-> Seq Scan on iboolpart_t
1238+
Filter: (a IS NOT UNKNOWN)
1239+
-> Seq Scan on iboolpart_f
1240+
Filter: (a IS NOT UNKNOWN)
1241+
-> Seq Scan on iboolpart_default
1242+
Filter: (a IS NOT UNKNOWN)
1243+
(7 rows)
1244+
1245+
select * from iboolpart where a in (true, false);
1246+
a
1247+
---
1248+
t
1249+
f
1250+
(2 rows)
1251+
1252+
select * from iboolpart where a = false;
1253+
a
1254+
---
1255+
f
1256+
(1 row)
1257+
1258+
select * from iboolpart where not a = false;
1259+
a
1260+
---
1261+
t
1262+
(1 row)
1263+
1264+
select * from iboolpart where a is true or a is not true;
1265+
a
1266+
---
1267+
t
1268+
f
1269+
1270+
(3 rows)
1271+
1272+
select * from iboolpart where a is not true;
1273+
a
1274+
---
1275+
f
1276+
1277+
(2 rows)
1278+
1279+
select * from iboolpart where a is not true and a is not false;
1280+
a
1281+
---
1282+
1283+
(1 row)
1284+
1285+
select * from iboolpart where a is unknown;
1286+
a
1287+
---
1288+
1289+
(1 row)
1290+
1291+
select * from iboolpart where a is not unknown;
1292+
a
1293+
---
1294+
t
1295+
f
1296+
(2 rows)
1297+
10931298
create table boolrangep (a bool, b bool, c int) partition by range (a,b,c);
10941299
create table boolrangep_tf partition of boolrangep for values from ('true', 'false', 0) to ('true', 'false', 100);
10951300
create table boolrangep_ft partition of boolrangep for values from ('false', 'true', 0) to ('false', 'true', 100);
@@ -1489,7 +1694,7 @@ explain (costs off) select * from rparted_by_int2 where a > 100000000000000;
14891694
Filter: (a > '100000000000000'::bigint)
14901695
(2 rows)
14911696

1492-
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2;
1697+
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, iboolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2;
14931698
--
14941699
-- Test Partition pruning for HASH partitioning
14951700
--

0 commit comments

Comments
 (0)