8000 Fix incorrect partition pruning logic for boolean partitioned tables · postgres/postgres@1c19e28 · GitHub
[go: up one dir, main page]
Skip to content

Commit 1c19e28

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 9517e6e commit 1c19e28

File tree

3 files changed

+289
-30
lines changed

3 files changed

+289
-30
lines changed

‎src/backend/partitioning/partprune.c

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ static PruneStepResult *perform_pruning_combine_step(PartitionPruneContext *cont
198198
static PartClauseMatchStatus match_boolean_partition_clause(Oid partopfamily,
199199
Expr *clause,
200200
Expr *partkey,
201-
Expr **outconst);
201+
Expr **outconst,
202+
bool *noteq);
202203
static void partkey_datum_from_expr(PartitionPruneContext *context,
203204
Expr *expr, int stateidx,
204205
Datum *value, bool *isnull);
@@ -1695,12 +1696,13 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
16951696
Oid partopfamily = part_scheme->partopfamily[partkeyidx],
16961697
partcoll = part_scheme->partcollation[partkeyidx];
16971698
Expr *expr;
1699+
bool noteq;
16981700

16991701
/*
17001702
* Recognize specially shaped clauses that match a Boolean partition key.
17011703
*/
17021704
boolmatchstatus = match_boolean_partition_clause(partopfamily, clause,
1703-
partkey, &expr);
1705+
partkey, &expr, &noteq);
17041706

17051707
if (boolmatchstatus == PARTCLAUSE_MATCH_CLAUSE)
17061708
{
@@ -1710,7 +1712,7 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
17101712
partclause->keyno = partkeyidx;
17111713
/* Do pruning with the Boolean equality operator. */
17121714
partclause->opno = BooleanEqualOperator;
1713-
partclause->op_is_ne = false;
1715+
partclause->op_is_ne = noteq;
17141716
partclause->expr = expr;
17151717
/* We know that expr is of Boolean type. */
17161718
partclause->cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
@@ -3468,20 +3470,22 @@ perform_pruning_combine_step(PartitionPruneContext *context,
34683470
* match_boolean_partition_clause
34693471
*
34703472
* If we're able to match the clause to the partition key as specially-shaped
3471-
* boolean clause, set *outconst to a Const containing a true or false value
3472-
* and return PARTCLAUSE_MATCH_CLAUSE. Returns PARTCLAUSE_UNSUPPORTED if the
3473-
* clause is not a boolean clause or if the boolean clause is unsuitable for
3474-
* partition pruning. Returns PARTCLAUSE_NOMATCH if it's a bool quals but
3475-
* just does not match this partition key. *outconst is set to NULL in the
3476-
* latter two cases.
3473+
* boolean clause, set *outconst to a Const containing a true or false value,
3474+
* set *noteq according to if the clause was in the "not" form, i.e. "is not
3475+
* true" or "is not false", and return PARTCLAUSE_MATCH_CLAUSE. Returns
3476+
* PARTCLAUSE_UNSUPPORTED if the clause is not a boolean clause or if the
3477+
* boolean clause is unsuitable for partition pruning. Returns
3478+
* PARTCLAUSE_NOMATCH if it's a bool quals but just does not match this
3479+
* partition key. *outconst is set to NULL in the latter two cases.
34773480
*/
34783481
static PartClauseMatchStatus
34793482
match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
3480-
Expr **outconst)
3483+
Expr **outconst, bool *noteq)
34813484
{
34823485
Expr *leftop;
34833486

34843487
*outconst = NULL;
3488+
*noteq = false;
34853489

34863490
if (!IsBooleanOpfamily(partopfamily))
34873491
return PARTCLAUSE_UNSUPPORTED;
@@ -3500,11 +3504,25 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
35003504
leftop = ((RelabelType *) leftop)->arg;
35013505

35023506
if (equal(leftop, partkey))
3503-
*outconst = (btest->booltesttype == IS_TRUE ||
3504-
btest->booltesttype == IS_NOT_FALSE)
3505-
? (Expr *) makeBoolConst(true, false)
3506-
: (Expr *) makeBoolConst(false, false);
3507-
3507+
{
3508+
switch (btest->booltesttype)
3509+
{
3510+
case IS_NOT_TRUE:
3511+
*noteq = true;
3512+
/* fall through */
3513+
case IS_TRUE:
3514+
*outconst = (Expr *) makeBoolConst(true, false);
3515+
break;
3516+
case IS_NOT_FALSE:
3517+
*noteq = true;
3518+
/* fall through */
3519+
case IS_FALSE:
3520+
*outconst = (Expr *) makeBoolConst(false, false);
3521+
break;
3522+
default:
3523+
return PARTCLAUSE_UNSUPPORTED;
3524+
}
3525+
}
35083526
if (*outconst)
35093527
return PARTCLAUSE_MATCH_CLAUSE;
35103528
}
@@ -3519,11 +3537,9 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
35193537

35203538
/* Compare to the partition key, and make up a clause ... */
35213539
if (equal(leftop, partkey))
3522-
*outconst = is_not_clause ?
3523-
(Expr *) makeBoolConst(false, false) :
3524-
(Expr *) makeBoolConst(true, false);
3540+
*outconst = (Expr *) makeBoolConst(!is_not_clause, false);
35253541
else if (equal(negate_clause((Node *) leftop), partkey))
3526-
*outconst = (Expr *) makeBoolConst(false, false);
3542+
*outconst = (Expr *) makeBoolConst(is_not_clause, false);
35273543

35283544
if (*outconst)
35293545
return PARTCLAUSE_MATCH_CLAUSE;

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

Lines changed: 217 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,7 @@ create table boolpart (a bool) partition by list (a);
10431043
create table boolpart_default partition of boolpart default;
10441044
create table boolpart_t partition of boolpart for values in ('true');
10451045
create table boolpart_f partition of boolpart for values in ('false');
1046+
insert into boolpart values (true), (false), (null);
10461047
explain (costs off) select * from boolpart where a in (true, false);
10471048
QUERY PLAN
10481049
------------------------------------------------
@@ -1077,22 +1078,27 @@ explain (costs off) select * from boolpart where a is true or a is not true;
10771078
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
10781079
-> Seq Scan on boolpart_t
10791080
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1080-
(5 rows)
1081+
-> Seq Scan on boolpart_default
1082+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1083+
(7 rows)
10811084

10821085
explain (costs off) select * from boolpart where a is not true;
1083-
QUERY PLAN
1084-
---------------------------------
1086+
QUERY PLAN
1087+
------------------------------------
10851088
Append
10861089
-> Seq Scan on boolpart_f
10871090
Filter: (a IS NOT TRUE)
1088-
(3 rows)
1091+
-> Seq Scan on boolpart_default
1092+
Filter: (a IS NOT TRUE)
1093+
(5 rows)
10891094

10901095
explain (costs off) select * from boolpart where a is not true and a is not false;
1091-
QUERY PLAN
1092-
--------------------------
1093-
Result
1094-
One-Time Filter: false
1095-
(2 rows)
1096+
QUERY PLAN
1097+
--------------------------------------------------------
1098+
Append
1099+
-> Seq Scan on boolpart_default
1100+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1101+
(3 rows)
10961102

10971103
explain (costs off) select * from boolpart where a is unknown;
10981104
QUERY PLAN
@@ -1118,6 +1124,207 @@ explain (costs off) select * from boolpart where a is not unknown;
11181124
Filter: (a IS NOT UNKNOWN)
11191125
(7 rows)
11201126

1127+
select * from boolpart where a in (true, false);
1128+
a
1129+
---
1130+
f
1131+
t
1132+
(2 rows)
1133+
1134+
select * from boolpart where a = false;
1135+
a
1136+
---
1137+
f
1138+
(1 row)
1139+
1140+
select * from boolpart where not a = false;
1141+
a
1142+
---
1143+
t
1144+
(1 row)
1145+
1146+
select * from boolpart where a is true or a is not true;
1147+
a
1148+
---
1149+
f
1150+
t
1151+
1152+
(3 rows)
1153+
1154+
select * from boolpart where a is not true;
1155+
a
1156+
---
1157+
f
1158+
1159+
(2 rows)
1160+
1161+
select * from boolpart where a is not true and a is not false;
1162+
a
1163+
---
1164+
1165+
(1 row)
1166+
1167+
select * from boolpart where a is unknown;
1168+
a
1169+
---
1170+
1171+
(1 row)
1172+
1173+
select * from boolpart where a is not unknown;
1174+
a
1175+
---
1176+
f
1177+
t
1178+
(2 rows)
1179+
1180+
-- inverse boolean partitioning - a seemingly unlikely design, but we've got
1181+
-- code for it, so we'd better test it.
1182+
create table iboolpart (a bool) partition by list ((not a));
1183+
create table iboolpart_default partition of iboolpart default;
1184+
create table iboolpart_f partition of iboolpart for values in ('true');
1185+
create table iboolpart_t partition of iboolpart for values in ('false');
1186+
insert into iboolpart values (true), (false), (null);
1187+
explain (costs off) select * from iboolpart where a in (true, false);
1188+
QUERY PLAN
1189+
------------------------------------------------
1190+
Append
1191+
-> Seq Scan on iboolpart_t
1192+
Filter: (a = ANY ('{t,f}'::boolean[]))
1193+
-> Seq Scan on iboolpart_f
1194+
Filter: (a = ANY ('{t,f}'::boolean[]))
1195+
-> Seq Scan on iboolpart_default
1196+
Filter: (a = ANY ('{t,f}'::boolean[]))
1197+
(7 rows)
1198+
1199+
explain (costs off) select * from iboolpart where a = false;
1200+
QUERY PLAN
1201+
-------------------------------
1202+
Append
1203+
-> Seq Scan on iboolpart_f
1204+
Filter: (NOT a)
1205+
(3 rows)
1206+
1207+
explain (costs off) select * from iboolpart where not a = false;
1208+
QUERY PLAN
1209+
-------------------------------
1210+
Append
1211+
-> Seq Scan on iboolpart_t
1212+
Filter: a
1213+
(3 rows)
1214+
1215+
explain (costs off) select * from iboolpart where a is true or a is not true;
1216+
QUERY PLAN
1217+
--------------------------------------------------
1218+
Append
1219+
-> Seq Scan on iboolpart_t
1220+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1221+
-> Seq Scan on iboolpart_f
1222+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1223+
-> Seq Scan on iboolpart_default
1224+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1225+
(7 rows)
1226+
1227+
explain (costs off) select * from iboolpart where a is not true;
1228+
QUERY PLAN
1229+
-------------------------------------
1230+
Append
1231+
-> Seq Scan on iboolpart_t
1232+
Filter: (a IS NOT TRUE)
1233+
-> Seq Scan on iboolpart_f
1234+
Filter: (a IS NOT TRUE)
1235+
-> Seq Scan on iboolpart_default
1236+
Filter: (a IS NOT TRUE)
1237+
(7 rows)
1238+
1239+
explain (costs off) select * from iboolpart where a is not true and a is not false;
1240+
QUERY PLAN
1241+
--------------------------------------------------------
1242+
Append
1243+
-> Seq Scan on iboolpart_t
1244+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1245+
-> Seq Scan on iboolpart_f
1246+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1247+
-> Seq Scan on iboolpart_default
1248+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
1249+
(7 rows)
1250+
1251+
explain (costs off) select * from iboolpart where a is unknown;
1252+
QUERY PLAN
1253+
-------------------------------------
1254+
Append
1255+
-> Seq Scan on iboolpart_t
1256+
Filter: (a IS UNKNOWN)
1257+
-> Seq Scan on iboolpart_f
1258+
Filter: (a IS UNKNOWN)
1259+
-> Seq Scan on iboolpart_default
1260+
Filter: (a IS UNKNOWN)
1261+
(7 rows)
1262+
1263+
explain (costs off) select * from iboolpart where a is not unknown;
1264+
QUERY PLAN
1265+
-------------------------------------
1266+
Append
1267+
-> Seq Scan on iboolpart_t
1268+
Filter: (a IS NOT UNKNOWN)
1269+
-> Seq Scan on iboolpart_f
1270+
Filter: (a IS NOT UNKNOWN)
1271+
-> Seq Scan on iboolpart_default
1272+
Filter: (a IS NOT UNKNOWN)
1273+
(7 rows)
1274+
1275+
select * from iboolpart where a in (true, false);
1276+
a
1277+
---
1278+
t
1279+
f
1280+
(2 rows)
1281+
1282+
select * from iboolpart where a = false;
1283+
a
1284+
---
1285+
f
1286+
(1 row)
1287+
1288+
select * from iboolpart where not a = false;
1289+
a
1290+
---
1291+
t
1292+
(1 row)
1293+
1294+
select * from iboolpart where a is true or a is not true;
1295+
a
1296+
---
1297+
t
1298+
f
1299+
1300+
(3 rows)
1301+
1302+
select * from iboolpart where a is not true;
1303+
a
1304+
---
1305+
f
1306+
1307+
(2 rows)
1308+
1309+
select * from iboolpart where a is not true and a is not false;
1310+
a
1311+
---
1312+
1313+
(1 row)
1314+
1315+
select * from iboolpart where a is unknown;
1316+
a
1317+
---
1318+
1319+
(1 row)
1320+
1321+
select * from iboolpart where a is not unknown;
1322+
a
1323+
---
1324+
t
1325+
f
1326+
(2 rows)
1327+
11211328
create table boolrangep (a bool, b bool, c int) partition by range (a,b,c);
11221329
create table boolrangep_tf partition of boolrangep for values from ('true', 'false', 0) to ('true', 'false', 100);
11231330
create table boolrangep_ft partition of boolrangep for values from ('false', 'true', 0) to ('false', 'true', 100);
@@ -1524,7 +1731,7 @@ explain (costs off) select * from rparted_by_int2 where a > 100000000000000;
15241731
Filter: (a > '100000000000000'::bigint)
15251732
(3 rows)
15261733

1527-
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2;
1734+
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, iboolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2;
15281735
--
15291736
-- Test Partition pruning for HASH partitioning
15301737
--

0 commit comments

Comments
 (0)
0