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

Commit b6e3a3a

Browse files
alvherreAmit Langote
andcommitted
Better handle pseudotypes as partition keys
We fail to handle polymorphic types properly when they are used as partition keys: we were unnecessarily adding a RelabelType node on top, which confuses code examining the nodes. In particular, this makes predtest.c-based partition pruning not to work, and ruleutils.c to emit expressions that are uglier than needed. Fix it by not adding RelabelType when not needed. In master/11 the new pruning code is separate so it doesn't suffer from this problem, since we already fixed it (in essentially the same way) in e5dcbb8, which also added a few tests; back-patch those tests to pg10 also. But since UPDATE/DELETE still uses predtest.c in pg11, this change improves partitioning for those cases too. Add tests for this. The ruleutils.c behavior change is relevant in pg11/master too. Co-authored-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Discussion: https://postgr.es/m/54745d13-7ed4-54ac-97d8-ea1eec95ae25@lab.ntt.co.jp
1 parent bcbd940 commit b6e3a3a

File tree

4 files changed

+41
-31
lines changed

4 files changed

+41
-31
lines changed

src/backend/partitioning/partbounds.c

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,8 +1144,12 @@ get_partition_bound_num_indexes(PartitionBoundInfo bound)
11441144
/*
11451145
* get_partition_operator
11461146
*
1147-
* Return oid of the operator of given strategy for a given partition key
1148-
* column.
1147+
* Return oid of the operator of the given strategy for the given partition
1148+
* key column. It is assumed that the partitioning key is of the same type as
1149+
* the chosen partitioning opclass, or at least binary-compatible. In the
1150+
* latter case, *need_relabel is set to true if the opclass is not of a
1151+
* polymorphic type (indicating a RelabelType node needed on top), otherwise
1152+
* false.
11491153
*/
11501154
static Oid
11511155
get_partition_operator(PartitionKey key, int col, StrategyNumber strategy,
@@ -1154,40 +1158,26 @@ get_partition_operator(PartitionKey key, int col, StrategyNumber strategy,
11541158
Oid operoid;
11551159

11561160
/*
1157-
* First check if there exists an operator of the given strategy, with
1158-
* this column's type as both its lefttype and righttype, in the
1159-
* partitioning operator family specified for the column.
1161+
* Get the operator in the partitioning opfamily using the opclass'
1162+
* declared input type as both left- and righttype.
11601163
*/
11611164
operoid = get_opfamily_member(key->partopfamily[col],
1162-
key->parttypid[col],
1163-
key->parttypid[col],
1165+
key->partopcintype[col],
1166+
key->partopcintype[col],
11641167
strategy);
1168+
if (!OidIsValid(operoid))
1169+
elog(ERROR, "missing operator %d(%u,%u) in partition opfamily %u",
1170+
strategy, key->partopcintype[col], key->partopcintype[col],
1171+
key->partopfamily[col]);
11651172

11661173
/*
1167-
* If one doesn't exist, we must resort to using an operator in the same
1168-
* operator family but with the operator class declared input type. It is
1169-
* OK to do so, because the column's type is known to be binary-coercible
1170-
* with the operator class input type (otherwise, the operator class in
1171-
* question would not have been accepted as the partitioning operator
1172-
* class). We must however inform the caller to wrap the non-Const
1173-
* expression with a RelabelType node to denote the implicit coercion. It
1174-
* ensures that the resulting expression structurally matches similarly
1175-
* processed expressions within the optimizer.
1174+
* If the partition key column is not of the same type as the operator
1175+
* class and not polymorphic, tell caller to wrap the non-Const expression
1176+
* in a RelabelType. This matches what parse_coerce.c does.
11761177
*/
1177-
if (!OidIsValid(operoid))
1178-
{
1179-
operoid = get_opfamily_member(key->partopfamily[col],
1180-
key->partopcintype[col],
1181-
key->partopcintype[col],
1182-
strategy);
1183-
if (!OidIsValid(operoid))
1184-
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
1185-
strategy, key->partopcintype[col], key->partopcintype[col],
1186-
key->partopfamily[col]);
1187-
*need_relabel = true;
1188-
}
1189-
else
1190-
*need_relabel = false;
1178+
*need_relabel = (key->parttypid[col] != key->partopcintype[col] &&
1179+
key->partopcintype[col] != RECORDOID &&
1180+
!IsPolymorphicType(key->partopcintype[col]));
11911181

11921182
return operoid;
11931183
}

src/test/regress/expected/create_table.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -882,7 +882,7 @@ CREATE TABLE arrlp12 PARTITION OF arrlp FOR VALUES IN ('{1}', '{2}');
882882
--------+-----------+-----------+----------+---------+----------+--------------+-------------
883883
a | integer[] | | | | extended | |
884884
Partition of: arrlp FOR VALUES IN ('{1}', '{2}')
885-
Partition constraint: ((a IS NOT NULL) AND (((a)::anyarray OPERATOR(pg_catalog.=) '{1}'::integer[]) OR ((a)::anyarray OPERATOR(pg_catalog.=) '{2}'::integer[])))
885+
Partition constraint: ((a IS NOT NULL) AND ((a = '{1}'::integer[]) OR (a = '{2}'::integer[])))
886886

887887
DROP TABLE arrlp;
888888
-- partition on boolean column

src/test/regress/expected/partition_prune.out

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2817,6 +2817,24 @@ explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
28172817
Filter: ((a = '{4,5}'::integer[]) OR (a = '{1}'::integer[]))
28182818
(5 rows)
28192819

2820+
explain (costs off) update pp_arrpart set a = a where a = '{1}';
2821+
QUERY PLAN
2822+
----------------------------------------
2823+
Update on pp_arrpart
2824+
Update on pp_arrpart1
2825+
-> Seq Scan on pp_arrpart1
2826+
Filter: (a = '{1}'::integer[])
2827+
(4 rows)
2828+
2829+
explain (costs off) delete from pp_arrpart where a = '{1}';
2830+
QUERY PLAN
2831+
----------------------------------------
2832+
Delete on pp_arrpart
2833+
Delete on pp_arrpart1
2834+
-> Seq Scan on pp_arrpart1
2835+
Filter: (a = '{1}'::integer[])
2836+
(4 rows)
2837+
28202838
drop table pp_arrpart;
28212839
-- array type hash partition key
28222840
create table pph_arrpart (a int[]) partition by hash (a);

src/test/regress/sql/partition_prune.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,8 @@ create table pp_arrpart2 partition of pp_arrpart for values in ('{2, 3}', '{4, 5
721721
explain (costs off) select * from pp_arrpart where a = '{1}';
722722
explain (costs off) select * from pp_arrpart where a = '{1, 2}';
723723
explain (costs off) select * from pp_arrpart where a in ('{4, 5}', '{1}');
724+
explain (costs off) update pp_arrpart set a = a where a = '{1}';
725+
explain (costs off) delete from pp_arrpart where a = '{1}';
724726
drop table pp_arrpart;
725727

726728
-- array type hash partition key

0 commit comments

Comments
 (0)