Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit e0693fa

Browse files
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 c7dc56b commit e0693fa

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
@@ -201,7 +201,8 @@ static PruneStepResult *perform_pruning_combine_step(PartitionPruneContext *cont
201201
static PartClauseMatchStatus match_boolean_partition_clause(Oid partopfamily,
202202
Expr *clause,
203203
Expr *partkey,
204-
Expr **outconst);
204+
Expr **outconst,
205+
bool *noteq);
205206
static void partkey_datum_from_expr(PartitionPruneContext *context,
206207
Expr *expr, int stateidx,
207208
Datum *value, bool *isnull);
@@ -1809,12 +1810,13 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
18091810
Oid partopfamily = part_scheme->partopfamily[partkeyidx],
18101811
partcoll = part_scheme->partcollation[partkeyidx];
18111812
Expr *expr;
1813+
bool noteq;
18121814

18131815
/*
18141816
* Recognize specially shaped clauses that match a Boolean partition key.
18151817
*/
18161818
boolmatchstatus = match_boolean_partition_clause(partopfamily, clause,
1817-
partkey, &expr);
1819+
partkey, &expr, &noteq);
18181820

18191821
if (boolmatchstatus == PARTCLAUSE_MATCH_CLAUSE)
18201822
{
@@ -1824,7 +1826,7 @@ match_clause_to_partition_key(GeneratePruningStepsContext *context,
18241826
partclause->keyno = partkeyidx;
18251827
/* Do pruning with the Boolean equality operator. */
18261828
partclause->opno = BooleanEqualOperator;
1827-
partclause->op_is_ne = false;
1829+
partclause->op_is_ne = noteq;
18281830
partclause->expr = expr;
18291831
/* We know that expr is of Boolean type. */
18301832
partclause->cmpfn = part_scheme->partsupfunc[partkeyidx].fn_oid;
@@ -3587,20 +3589,22 @@ perform_pruning_combine_step(PartitionPruneContext *context,
35873589
* match_boolean_partition_clause
35883590
*
35893591
* If we're able to match the clause to the partition key as specially-shaped
3590-
* boolean clause, set *outconst to a Const containing a true or false value
3591-
* and return PARTCLAUSE_MATCH_CLAUSE. Returns PARTCLAUSE_UNSUPPORTED if the
3592-
* clause is not a boolean clause or if the boolean clause is unsuitable for
3593-
* partition pruning. Returns PARTCLAUSE_NOMATCH if it's a bool quals but
3594-
* just does not match this partition key. *outconst is set to NULL in the
3595-
* latter two cases.
3592+
* boolean clause, set *outconst to a Const containing a true or false value,
3593+
* set *noteq according to if the clause was in the "not" form, i.e. "is not
3594+
* true" or "is not false", and return PARTCLAUSE_MATCH_CLAUSE. Returns
3595+
* PARTCLAUSE_UNSUPPORTED if the clause is not a boolean clause or if the
3596+
* boolean clause is unsuitable for partition pruning. Returns
3597+
* PARTCLAUSE_NOMATCH if it's a bool quals but just does not match this
3598+
* partition key. *outconst is set to NULL in the latter two cases.
35963599
*/
35973600
static PartClauseMatchStatus
35983601
match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
3599-
Expr **outconst)
3602+
Expr **outconst, bool *noteq)
36003603
{
36013604
Expr *leftop;
36023605

36033606
*outconst = NULL;
3607+
*noteq = false;
36043608

36053609
/*
36063610
* Partitioning currently can only use built-in AMs, so checking for
@@ -3623,11 +3627,25 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
36233627
leftop = ((RelabelType *) leftop)->arg;
36243628

36253629
if (equal(leftop, partkey))
3626-
*outconst = (btest->booltesttype == IS_TRUE ||
3627-
btest->booltesttype == IS_NOT_FALSE)
3628-
? (Expr *) makeBoolConst(true, false)
3629-
: (Expr *) makeBoolConst(false, false);
3630-
3630+
{
3631+
switch (btest->booltesttype)
3632+
{
3633+
case IS_NOT_TRUE:
3634+
*noteq = true;
3635+
/* fall through */
3636+
case IS_TRUE:
3637+
*outconst = (Expr *) makeBoolConst(true, false);
3638+
break;
3639+
case IS_NOT_FALSE:
3640+
*noteq = true;
3641+
/* fall through */
3642+
case IS_FALSE:
3643+
*outconst = (Expr *) makeBoolConst(false, false);
3644+
break;
3645+
default:
3646+
return PARTCLAUSE_UNSUPPORTED;
3647+
}
3648+
}
36313649
if (*outconst)
36323650
return PARTCLAUSE_MATCH_CLAUSE;
36333651
}
@@ -3642,11 +3660,9 @@ match_boolean_partition_clause(Oid partopfamily, Expr *clause, Expr *partkey,
36423660

36433661
/* Compare to the partition key, and make up a clause ... */
36443662
if (equal(leftop, partkey))
3645-
*outconst = is_not_clause ?
3646-
(Expr *) makeBoolConst(false, false) :
3647-
(Expr *) makeBoolConst(true, false);
3663+
*outconst = (Expr *) makeBoolConst(!is_not_clause, false);
36483664
else if (equal(negate_clause((Node *) leftop), partkey))
3649-
*outconst = (Expr *) makeBoolConst(false, false);
3665+
*outconst = (Expr *) makeBoolConst(is_not_clause, false);
36503666

36513667
if (*outconst)
36523668
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
@@ -1038,6 +1038,7 @@ create table boolpart (a bool) partition by list (a);
10381038
create table boolpart_default partition of boolpart default;
10391039
create table boolpart_t partition of boolpart for values in ('true');
10401040
create table boolpart_f partition of boolpart for values in ('false');
1041+
insert into boolpart values (true), (false), (null);
10411042
explain (costs off) select * from boolpart where a in (true, false);
10421043
QUERY PLAN
10431044
------------------------------------------------
@@ -1070,20 +1071,25 @@ explain (costs off) select * from boolpart where a is true or a is not true;
10701071
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
10711072
-> Seq Scan on boolpart_t boolpart_2
10721073
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1073-
(5 rows)
1074+
-> Seq Scan on boolpart_default boolpart_3
1075+
Filter: ((a IS TRUE) OR (a IS NOT TRUE))
1076+
(7 rows)
10741077

10751078
explain (costs off) select * from boolpart where a is not true;
1076-
QUERY PLAN
1077-
---------------------------------
1078-
Seq Scan on boolpart_f boolpart
1079-
Filter: (a IS NOT TRUE)
1080-
(2 rows)
1079+
QUERY PLAN
1080+
-----------------------------------------------
1081+
Append
1082+
-> Seq Scan on boolpart_f boolpart_1
1083+
Filter: (a IS NOT TRUE)
1084+
-> Seq Scan on boolpart_default boolpart_2
1085+
Filter: (a IS NOT TRUE)
1086+
(5 rows)
10811087

10821088
explain (costs off) select * from boolpart where a is not true and a is not false;
1083-
QUERY PLAN
1084-
--------------------------
1085-
Result
1086-
One-Time Filter: false
1089+
QUERY PLAN
1090+
--------------------------------------------------
1091+
Seq Scan on boolpart_default boolpart
1092+
Filter: ((a IS NOT TRUE) AND (a IS NOT FALSE))
10871093
(2 rows)
10881094

10891095
explain (costs off) select * from boolpart where a is unknown;
@@ -1110,6 +1116,205 @@ explain (costs off) select * from boolpart where a is not unknown;
11101116
Filter: (a IS NOT UNKNOWN)
11111117
(7 rows)
11121118

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

1533-
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2;
1738+
drop table lp, coll_pruning, rlp, mc3p, mc2p, boolpart, iboolpart, boolrangep, rp, coll_pruning_multi, like_op_noprune, lparted_by_int2, rparted_by_int2;
15341739
--
15351740
-- Test Partition pruning for HASH partitioning
15361741
--

0 commit comments

Comments
 (0)