Skip to content

Commit 26610df

Browse files
kovdb75kovdb75
authored
Merge pull request #238 from postgrespro/PGPRO-5113
PGPRO-5113 Added 'tuple map' for prevent addition extra columns values into partitions
2 parents 978e73c + c16f746 commit 26610df

5 files changed

+227
-12
lines changed

‎expected/pathman_rebuild_updates.out

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,45 @@ UPDATE test_updates.test SET val = -1 WHERE val = 95 RETURNING *, tableoid::RE
155155
-1 | 105 | test_updates.test_13
156156
(1 row)
157157

158+
/* basic check for 'ALTER TABLE ... ADD COLUMN'; PGPRO-5113 */
159+
create table test_updates.test_5113(val int4 not null);
160+
insert into test_updates.test_5113 values (1);
161+
select create_range_partitions('test_updates.test_5113', 'val', 1, 10);
162+
create_range_partitions
163+
-------------------------
164+
1
165+
(1 row)
166+
167+
update test_updates.test_5113 set val = 11 where val = 1;
168+
alter table test_updates.test_5113 add column x varchar;
169+
/* no error here: */
170+
select * from test_updates.test_5113 where val = 11;
171+
val | x
172+
-----+---
173+
11 |
174+
(1 row)
175+
176+
drop table test_updates.test_5113 cascade;
177+
NOTICE: drop cascades to 3 other objects
178+
create table test_updates.test_5113(val int4 not null);
179+
insert into test_updates.test_5113 values (1);
180+
select create_range_partitions('test_updates.test_5113', 'val', 1, 10);
181+
create_range_partitions
182+
-------------------------
183+
1
184+
(1 row)
185+
186+
update test_updates.test_5113 set val = 11 where val = 1;
187+
alter table test_updates.test_5113 add column x int8;
188+
/* no extra data in column 'x' here: */
189+
select * from test_updates.test_5113 where val = 11;
190+
val | x
191+
-----+---
192+
11 |
193+
(1 row)
194+
195+
drop table test_updates.test_5113 cascade;
196+
NOTICE: drop cascades to 3 other objects
158197
DROP SCHEMA test_updates CASCADE;
159198
NOTICE: drop cascades to 15 other objects
160199
DROP EXTENSION pg_pathman;

‎expected/pathman_rebuild_updates_1.out

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,45 @@ UPDATE test_updates.test SET val = -1 WHERE val = 95 RETURNING *, tableoid::RE
155155
-1 | 105 | test_updates.test_13
156156
(1 row)
157157

158+
/* basic check for 'ALTER TABLE ... ADD COLUMN'; PGPRO-5113 */
159+
create table test_updates.test_5113(val int4 not null);
160+
insert into test_updates.test_5113 values (1);
161+
select create_range_partitions('test_updates.test_5113', 'val', 1, 10);
162+
create_range_partitions
163+
-------------------------
164+
1
165+
(1 row)
166+
167+
update test_updates.test_5113 set val = 11 where val = 1;
168+
alter table test_updates.test_5113 add column x varchar;
169+
/* no error here: */
170+
select * from test_updates.test_5113 where val = 11;
171+
val | x
172+
-----+---
173+
11 |
174+
(1 row)
175+
176+
drop table test_updates.test_5113 cascade;
177+
NOTICE: drop cascades to 3 other objects
178+
create table test_updates.test_5113(val int4 not null);
179+
insert into test_updates.test_5113 values (1);
180+
select create_range_partitions('test_updates.test_5113', 'val', 1, 10);
181+
create_range_partitions
182+
-------------------------
183+
1
184+
(1 row)
185+
186+
update test_updates.test_5113 set val = 11 where val = 1;
187+
alter table test_updates.test_5113 add column x int8;
188+
/* no extra data in column 'x' here: */
189+
select * from test_updates.test_5113 where val = 11;
190+
val | x
191+
-----+---
192+
11 |
193+
(1 row)
194+
195+
drop table test_updates.test_5113 cascade;
196+
NOTICE: drop cascades to 3 other objects
158197
DROP SCHEMA test_updates CASCADE;
159198
NOTICE: drop cascades to 15 other objects
160199
DROP EXTENSION pg_pathman;

‎sql/pathman_rebuild_updates.sql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,25 @@ UPDATE test_updates.test SET val = 95 WHERE val = 115 RETURNING *, tableoid::RE
7979
UPDATE test_updates.test SET val = -1 WHERE val = 95 RETURNING *, tableoid::REGCLASS;
8080

8181

82+
/* basic check for 'ALTER TABLE ... ADD COLUMN'; PGPRO-5113 */
83+
create table test_updates.test_5113(val int4 not null);
84+
insert into test_updates.test_5113 values (1);
85+
select create_range_partitions('test_updates.test_5113', 'val', 1, 10);
86+
update test_updates.test_5113 set val = 11 where val = 1;
87+
alter table test_updates.test_5113 add column x varchar;
88+
/* no error here: */
89+
select * from test_updates.test_5113 where val = 11;
90+
drop table test_updates.test_5113 cascade;
91+
92+
create table test_updates.test_5113(val int4 not null);
93+
insert into test_updates.test_5113 values (1);
94+
select create_range_partitions('test_updates.test_5113', 'val', 1, 10);
95+
update test_updates.test_5113 set val = 11 where val = 1;
96+
alter table test_updates.test_5113 add column x int8;
97+
/* no extra data in column 'x' here: */
98+
select * from test_updates.test_5113 where val = 11;
99+
drop table test_updates.test_5113 cascade;
100+
82101

83102
DROP SCHEMA test_updates CASCADE;
84103
DROP EXTENSION pg_pathman;

‎src/include/partition_filter.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ typedef struct
4848
Oid partid; /* partition's relid */
4949
ResultRelInfo *result_rel_info; /* cached ResultRelInfo */
5050
TupleConversionMap *tuple_map; /* tuple mapping (parent => child) */
51+
TupleConversionMap *tuple_map_child; /* tuple mapping (child => child), for exclude 'ctid' */
5152

5253
PartRelationInfo *prel; /* this child might be a parent... */
5354
ExprState *prel_expr_state; /* and have its own part. expression */
@@ -173,6 +174,10 @@ PartRelationInfo * refresh_result_parts_storage(ResultPartsStorage *parts_storag
173174

174175
TupleConversionMap * build_part_tuple_map(Relation parent_rel, Relation child_rel);
175176

177+
TupleConversionMap * build_part_tuple_map_child(Relation child_rel);
178+
179+
void destroy_tuple_map(TupleConversionMap *tuple_map);
180+
176181
List * pfilter_build_tlist(Plan *subplan);
177182

178183

‎src/partition_filter.c

Lines changed: 125 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,13 +239,9 @@ fini_result_parts_storage(ResultPartsStorage *parts_storage)
239239
}
240240

241241
/* Free conversion-related stuff */
242-
if (rri_holder->tuple_map)
243-
{
244-
FreeTupleDesc(rri_holder->tuple_map->indesc);
245-
FreeTupleDesc(rri_holder->tuple_map->outdesc);
242+
destroy_tuple_map(rri_holder->tuple_map);
246243

247-
free_conversion_map(rri_holder->tuple_map);
248-
}
244+
destroy_tuple_map(rri_holder->tuple_map_child);
249245

250246
/* Don't forget to close 'prel'! */
251247
if (rri_holder->prel)
@@ -381,6 +377,13 @@ scan_result_parts_storage(ResultPartsStorage *parts_storage, Oid partid)
381377
*/
382378
rri_holder->tuple_map = build_part_tuple_map(base_rel, child_rel);
383379

380+
/*
381+
* Field for child->child tuple transformation map. We need to
382+
* convert tuples because child TupleDesc might have extra
383+
* columns ('ctid' etc.) and need remove them.
384+
*/
385+
rri_holder->tuple_map_child = NULL;
386+
384387
/* Default values */
385388
rri_holder->prel = NULL;
386389
rri_holder->prel_expr_state = NULL;
@@ -468,6 +471,73 @@ build_part_tuple_map(Relation base_rel, Relation child_rel)
468471
return tuple_map;
469472
}
470473

474+
/*
475+
* Build tuple conversion map (e.g. partition tuple has extra column(s)).
476+
* We create a special tuple map (tuple_map_child), which, when applied to the
477+
* tuple of partition, translates the tuple attributes into the tuple
478+
* attributes of the same partition, discarding service attributes like "ctid"
479+
* (i.e. working like junkFilter).
480+
*/
481+
TupleConversionMap *
482+
build_part_tuple_map_child(Relation child_rel)
483+
{
484+
TupleConversionMap *tuple_map;
485+
TupleDesc child_tupdesc1;
486+
TupleDesc child_tupdesc2;
487+
int n;
488+
#if PG_VERSION_NUM >= 130000
489+
AttrMap *attrMap;
490+
#else
491+
AttrNumber *attrMap;
492+
#endif
493+
494+
child_tupdesc1 = CreateTupleDescCopy(RelationGetDescr(child_rel));
495+
child_tupdesc1->tdtypeid = InvalidOid;
496+
497+
child_tupdesc2 = CreateTupleDescCopy(RelationGetDescr(child_rel));
498+
child_tupdesc2->tdtypeid = InvalidOid;
499+
500+
/* Generate tuple transformation map */
501+
#if PG_VERSION_NUM >= 130000
502+
attrMap = build_attrmap_by_name(child_tupdesc1, child_tupdesc2);
503+
#else
504+
attrMap = convert_tuples_by_name_map(child_tupdesc1, child_tupdesc2,
505+
ERR_PART_DESC_CONVERT);
506+
#endif
507+
508+
/* Prepare the map structure */
509+
tuple_map = (TupleConversionMap *) palloc(sizeof(TupleConversionMap));
510+
tuple_map->indesc = child_tupdesc1;
511+
tuple_map->outdesc = child_tupdesc2;
512+
tuple_map->attrMap = attrMap;
513+
514+
/* preallocate workspace for Datum arrays */
515+
n = child_tupdesc1->natts;
516+
tuple_map->outvalues = (Datum *) palloc(n * sizeof(Datum));
517+
tuple_map->outisnull = (bool *) palloc(n * sizeof(bool));
518+
519+
n = child_tupdesc1->natts + 1; /* +1 for NULL */
520+
tuple_map->invalues = (Datum *) palloc(n * sizeof(Datum));
521+
tuple_map->inisnull = (bool *) palloc(n * sizeof(bool));
522+
523+
tuple_map->invalues[0] = (Datum) 0; /* set up the NULL entry */
524+
tuple_map->inisnull[0] = true;
525+
526+
return tuple_map;
527+
}
528+
529+
/* Destroy tuple conversion map */
530+
void
531+
destroy_tuple_map(TupleConversionMap *tuple_map)
532+
{
533+
if (tuple_map)
534+
{
535+
FreeTupleDesc(tuple_map->indesc);
536+
FreeTupleDesc(tuple_map->outdesc);
537+
538+
free_conversion_map(tuple_map);
539+
}
540+
}
471541

472542
/*
473543
* -----------------------------------
@@ -812,6 +882,7 @@ partition_filter_exec(CustomScanState *node)
812882
MemoryContext old_mcxt;
813883
ResultRelInfoHolder *rri_holder;
814884
ResultRelInfo *rri;
885+
JunkFilter *junkfilter = NULL;
815886
#if PG_VERSION_NUM >= 140000
816887
PartitionRouterState *pr_state = linitial(node->custom_ps);
817888

@@ -827,6 +898,8 @@ partition_filter_exec(CustomScanState *node)
827898
*/
828899
reint_partition_filter_state(state, pr_state->current_rri);
829900
}
901+
#else
902+
junkfilter = estate->es_result_relation_info->ri_junkFilter;
830903
#endif
831904

832905
/* Switch to per-tuple context */
@@ -844,18 +917,58 @@ partition_filter_exec(CustomScanState *node)
844917
/* Magic: replace parent's ResultRelInfo with ours */
845918
estate->es_result_relation_info = rri;
846919

920+
/*
921+
* Besides 'transform map' we should process two cases:
922+
* 1) CMD_UPDATE, row moved to other partition, junkfilter == NULL
923+
* (filled in router_set_slot() for SELECT + INSERT);
924+
* we should clear attribute 'ctid' (do not insert it into database);
925+
* 2) CMD_INSERT/CMD_UPDATE operations for partitions with deleted column(s),
926+
* junkfilter == NULL.
927+
*/
847928
/* If there's a transform map, rebuild the tuple */
848-
if (rri_holder->tuple_map)
929+
if (rri_holder->tuple_map ||
930+
(!junkfilter &&
931+
(state->command_type == CMD_INSERT || state->command_type == CMD_UPDATE) &&
932+
(slot->tts_tupleDescriptor->natts > rri->ri_RelationDesc->rd_att->natts /* extra fields */
933+
#if PG_VERSION_NUM < 120000
934+
/*
935+
* If we have a regular physical tuple 'slot->tts_tuple' and
936+
* it's locally palloc'd => we will use this tuple in
937+
* ExecMaterializeSlot() instead of materialize the slot, so
938+
* need to check number of attributes for this tuple:
939+
*/
940+
|| (slot->tts_tuple && slot->tts_shouldFree &&
941+
HeapTupleHeaderGetNatts(slot->tts_tuple->t_data) >
942+
rri->ri_RelationDesc->rd_att->natts /* extra fields */)
943+
#endif
944+
)))
849945
{
850-
Relation child_rel = rri->ri_RelationDesc;
851-
852-
/* xxx why old code decided to materialize it? */
853946
#if PG_VERSION_NUM < 120000
854947
HeapTuple htup_old,
855948
htup_new;
949+
#endif
950+
Relation child_rel = rri->ri_RelationDesc;
951+
TupleConversionMap *tuple_map;
856952

953+
if (rri_holder->tuple_map)
954+
tuple_map = rri_holder->tuple_map;
955+
else
956+
{
957+
if (!rri_holder->tuple_map_child)
958+
{ /*
959+
* Generate child->child tuple transformation map. We need to
960+
* convert tuples because child TupleDesc has extra
961+
* columns ('ctid' etc.) and need remove them.
962+
*/
963+
rri_holder->tuple_map_child = build_part_tuple_map_child(child_rel);
964+
}
965+
tuple_map = rri_holder->tuple_map_child;
966+
}
967+
968+
/* xxx why old code decided to materialize it? */
969+
#if PG_VERSION_NUM < 120000
857970
htup_old = ExecMaterializeSlot(slot);
858-
htup_new = do_convert_tuple(htup_old, rri_holder->tuple_map);
971+
htup_new = do_convert_tuple(htup_old, tuple_map);
859972
ExecClearTuple(slot);
860973
#endif
861974

@@ -872,7 +985,7 @@ partition_filter_exec(CustomScanState *node)
872985
/* TODO: why should we *always* set a new slot descriptor? */
873986
ExecSetSlotDescriptor(state->tup_convert_slot, RelationGetDescr(child_rel));
874987
#if PG_VERSION_NUM >= 120000
875-
slot = execute_attr_map_slot(rri_holder->tuple_map->attrMap, slot, state->tup_convert_slot);
988+
slot = execute_attr_map_slot(tuple_map->attrMap, slot, state->tup_convert_slot);
876989
#else
877990
slot = ExecStoreTuple(htup_new, state->tup_convert_slot, InvalidBuffer, true);
878991
#endif

0 commit comments

Comments
 (0)