Skip to content

Commit cc2905e

Browse files
anarazelanarazel
committed
Use slots more widely in tuple mapping code and make naming more consistent.
It's inefficient to use a single slot for mapping between tuple descriptors for multiple tuples, as previously done when using ConvertPartitionTupleSlot(), as that means the slot's tuple descriptors change for every tuple. Previously we also, via ConvertPartitionTupleSlot(), built new tuples after the mapping even in cases where we, immediately afterwards, access individual columns again. Refactor the code so one slot, on demand, is used for each partition. That avoids having to change the descriptor (and allows to use the more efficient "fixed" tuple slots). Then use slot->slot mapping, to avoid unnecessarily forming a tuple. As the naming between the tuple and slot mapping functions wasn't consistent, rename them to execute_attr_map_{tuple,slot}. It's likely that we'll also rename convert_tuples_by_* to denote that these functions "only" build a map, but that's left for later. Author: Amit Khandekar and Amit Langote, editorialized by me Reviewed-By: Amit Langote, Amit Khandekar, Andres Freund Discussion: https://postgr.es/m/CAJ3gD9fR0wRNeAE8VqffNTyONS_UfFPRpqxhnD9Q42vZB+Jvpg@mail.gmail.com https://postgr.es/m/e4f9d743-cd4b-efb0-7574-da21d86a7f36%40lab.ntt.co.jp Backpatch: -
1 parent 625b38e commit cc2905e

File tree

11 files changed

+272
-191
lines changed

11 files changed

+272
-191
lines changed

‎src/backend/access/common/tupconvert.c

Lines changed: 131 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,8 @@
44
* Tuple conversion support.
55
*
66
* These functions provide conversion between rowtypes that are logically
7-
* equivalent but might have columns in a different order or different sets
8-
* of dropped columns. There is some overlap of functionality with the
9-
* executor's "junkfilter" routines, but these functions work on bare
10-
* HeapTuples rather than TupleTableSlots.
7+
* equivalent but might have columns in a different order or different sets of
8+
* dropped columns.
119
*
1210
* Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group
1311
* Portions Copyright (c) 1994, Regents of the University of California
@@ -22,6 +20,7 @@
2220

2321
#include "access/htup_details.h"
2422
#include "access/tupconvert.h"
23+
#include "executor/tuptable.h"
2524
#include "utils/builtins.h"
2625

2726

@@ -31,7 +30,7 @@
3130
* The setup routine checks whether the given source and destination tuple
3231
* descriptors are logically compatible. If not, it throws an error.
3332
* If so, it returns NULL if they are physically compatible (ie, no conversion
34-
* is needed), else a TupleConversionMap that can be used by do_convert_tuple
33+
* is needed), else a TupleConversionMap that can be used by execute_attr_map_tuple
3534
* to perform the conversion.
3635
*
3736
* The TupleConversionMap, if needed, is palloc'd in the caller's memory
@@ -214,55 +213,13 @@ convert_tuples_by_name(TupleDesc indesc,
214213
TupleConversionMap *map;
215214
AttrNumber *attrMap;
216215
int n = outdesc->natts;
217-
int i;
218-
bool same;
219216

220217
/* Verify compatibility and prepare attribute-number map */
221-
attrMap = convert_tuples_by_name_map(indesc, outdesc, msg);
222-
223-
/*
224-
* Check to see if the map is one-to-one, in which case we need not do a
225-
* tuple conversion. We must also insist that both tupdescs either
226-
* specify or don't specify an OID column, else we need a conversion to
227-
* add/remove space for that. (For some callers, presence or absence of
228-
* an OID column perhaps would not really matter, but let's be safe.)
229-
*/
230-
if (indesc->natts == outdesc->natts &&
231-
indesc->tdhasoid == outdesc->tdhasoid)
232-
{
233-
same = true;
234-
for (i = 0; i < n; i++)
235-
{
236-
Form_pg_attribute inatt;
237-
Form_pg_attribute outatt;
238-
239-
if (attrMap[i] == (i + 1))
240-
continue;
241-
242-
/*
243-
* If it's a dropped column and the corresponding input column is
244-
* also dropped, we needn't convert. However, attlen and attalign
245-
* must agree.
246-
*/
247-
inatt = TupleDescAttr(indesc, i);
248-
outatt = TupleDescAttr(outdesc, i);
249-
if (attrMap[i] == 0 &&
250-
inatt->attisdropped &&
251-
inatt->attlen == outatt->attlen &&
252-
inatt->attalign == outatt->attalign)
253-
continue;
254-
255-
same = false;
256-
break;
257-
}
258-
}
259-
else
260-
same = false;
218+
attrMap = convert_tuples_by_name_map_if_req(indesc, outdesc, msg);
261219

262-
if (same)
220+
if (attrMap == NULL)
263221
{
264-
/* Runtime conversion is not needed */
265-
pfree(attrMap);
222+
/* runtime conversion is not needed */
266223
return NULL;
267224
}
268225

@@ -367,11 +324,78 @@ convert_tuples_by_name_map(TupleDesc indesc,
367324
return attrMap;
368325
}
369326

327+
/*
328+
* Returns mapping created by convert_tuples_by_name_map, or NULL if no
329+
* conversion not required. This is a convenience routine for
330+
* convert_tuples_by_name() and other functions.
331+
*/
332+
AttrNumber *
333+
convert_tuples_by_name_map_if_req(TupleDesc indesc,
334+
TupleDesc outdesc,
335+
const char *msg)
336+
{
337+
AttrNumber *attrMap;
338+
int n = outdesc->natts;
339+
int i;
340+
bool same;
341+
342+
/* Verify compatibility and prepare attribute-number map */
343+
attrMap = convert_tuples_by_name_map(indesc, outdesc, msg);
344+
345+
/*
346+
* Check to see if the map is one-to-one, in which case we need not do a
347+
* tuple conversion. We must also insist that both tupdescs either
348+
* specify or don't specify an OID column, else we need a conversion to
349+
* add/remove space for that. (For some callers, presence or absence of
350+
* an OID column perhaps would not really matter, but let's be safe.)
351+
*/
352+
if (indesc->natts == outdesc->natts &&
353+
indesc->tdhasoid == outdesc->tdhasoid)
354+
{
355+
same = true;
356+
for (i = 0; i < n; i++)
357+
{
358+
Form_pg_attribute inatt;
359+
Form_pg_attribute outatt;
360+
361+
if (attrMap[i] == (i + 1))
362+
continue;
363+
364+
/*
365+
* If it's a dropped column and the corresponding input column is
366+
* also dropped, we needn't convert. However, attlen and attalign
367+
* must agree.
368+
*/
369+
inatt = TupleDescAttr(indesc, i);
370+
outatt = TupleDescAttr(outdesc, i);
371+
if (attrMap[i] == 0 &&
372+
inatt->attisdropped &&
373+
inatt->attlen == outatt->attlen &&
374+
inatt->attalign == outatt->attalign)
375+
continue;
376+
377+
same = false;
378+
break;
379+
}
380+
}
381+
else
382+
same = false;
383+
384+
if (same)
385+
{
386+
/* Runtime conversion is not needed */
387+
pfree(attrMap);
388+
return NULL;
389+
}
390+
else
391+
return attrMap;
392+
}
393+
370394
/*
371395
* Perform conversion of a tuple according to the map.
372396
*/
373397
HeapTuple
374-
do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
398+
execute_attr_map_tuple(HeapTuple tuple, TupleConversionMap *map)
375399
{
376400
AttrNumber *attrMap = map->attrMap;
377401
Datum *invalues = map->invalues;
@@ -405,6 +429,62 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map)
405429
return heap_form_tuple(map->outdesc, outvalues, outisnull);
406430
}
407431

432+
/*
433+
* Perform conversion of a tuple slot according to the map.
434+
*/
435+
TupleTableSlot *
436+
execute_attr_map_slot(AttrNumber *attrMap,
437+
TupleTableSlot *in_slot,
438+
TupleTableSlot *out_slot)
439+
{
440+
Datum *invalues;
441+
bool *inisnull;
442+
Datum *outvalues;
443+
bool *outisnull;
444+
int outnatts;
445+
int i;
446+
447+
/* Sanity checks */
448+
Assert(in_slot->tts_tupleDescriptor != NULL &&
449+
out_slot->tts_tupleDescriptor != NULL);
450+
Assert(in_slot->tts_values != NULL && out_slot->tts_values != NULL);
451+
452+
outnatts = out_slot->tts_tupleDescriptor->natts;
453+
454+
/* Extract all the values of the in slot. */
455+
slot_getallattrs(in_slot);
456+
457+
/* Before doing the mapping, clear any old contents from the out slot */
458+
ExecClearTuple(out_slot);
459+
460+
invalues = in_slot->tts_values;
461+
inisnull = in_slot->tts_isnull;
462+
outvalues = out_slot->tts_values;
463+
outisnull = out_slot->tts_isnull;
464+
465+
/* Transpose into proper fields of the out slot. */
466+
for (i = 0; i < outnatts; i++)
467+
{
468+
int j = attrMap[i] - 1;
469+
470+
/* attrMap[i] == 0 means it's a NULL datum. */
471+
if (j == -1)
472+
{
473+
outvalues[i] = (Datum) 0;
474+
outisnull[i] = true;
475+
}
476+
else
477+
{
478+
outvalues[i] = invalues[j];
479+
outisnull[i] = inisnull[j];
480+
}
481+
}
482+
483+
ExecStoreVirtualTuple(out_slot);
484+
485+
return out_slot;
486+
}
487+
408488
/*
409489
* Free a TupleConversionMap structure.
410490
*/

‎src/backend/commands/analyze.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1452,7 +1452,7 @@ acquire_inherited_sample_rows(Relation onerel, int elevel,
14521452
{
14531453
HeapTuple newtup;
14541454

1455-
newtup = do_convert_tuple(rows[numrows + j], map);
1455+
newtup = execute_attr_map_tuple(rows[numrows + j], map);
14561456
heap_freetuple(rows[numrows + j]);
14571457
rows[numrows + j] = newtup;
14581458
}

‎src/backend/commands/copy.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "commands/trigger.h"
3232
#include "executor/execPartition.h"
3333
#include "executor/executor.h"
34+
#include "executor/tuptable.h"
3435
#include "foreign/fdwapi.h"
3536
#include "libpq/libpq.h"
3637
#include "libpq/pqformat.h"
@@ -2691,6 +2692,7 @@ CopyFrom(CopyState cstate)
26912692
if (proute)
26922693
{
26932694
int leaf_part_index;
2695+
TupleConversionMap *map;
26942696

26952697
/*
26962698
* Away we go ... If we end up not finding a partition after all,
@@ -2862,14 +2864,27 @@ CopyFrom(CopyState cstate)
28622864

28632865
/*
28642866
* We might need to convert from the parent rowtype to the
2865-
* partition rowtype. Don't free the already stored tuple as it
2866-
* may still be required for a multi-insert batch.
2867+
* partition rowtype.
28672868
*/
2868-
tuple = ConvertPartitionTupleSlot(proute->parent_child_tupconv_maps[leaf_part_index],
2869-
tuple,
2870-
proute->partition_tuple_slot,
2871-
&slot,
2872-
false);
2869+
map = proute->parent_child_tupconv_maps[leaf_part_index];
2870+
if (map != NULL)
2871+
{
2872+
TupleTableSlot *new_slot;
2873+
MemoryContext oldcontext;
2874+
2875+
Assert(proute->partition_tuple_slots != NULL &&
2876+
proute->partition_tuple_slots[leaf_part_index] != NULL);
2877+
new_slot = proute->partition_tuple_slots[leaf_part_index];
2878+
slot = execute_attr_map_slot(map->attrMap, slot, new_slot);
2879+
2880+
/*
2881+
* Get the tuple in the per-tuple context, so that it will be
2882+
* freed after each batch insert.
2883+
*/
2884+
oldcontext = MemoryContextSwitchTo(GetPerTupleMemoryContext(estate));
2885+
tuple = ExecCopySlotTuple(slot);
2886+
MemoryContextSwitchTo(oldcontext);
2887+
}
28732888

28742889
tuple->t_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc);
28752890
}

‎src/backend/commands/trigger.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5786,7 +5786,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
57865786

57875787
if (map != NULL)
57885788
{
5789-
HeapTuple converted = do_convert_tuple(oldtup, map);
5789+
HeapTuple converted = execute_attr_map_tuple(oldtup, map);
57905790

57915791
tuplestore_puttuple(old_tuplestore, converted);
57925792
pfree(converted);
@@ -5806,7 +5806,7 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo,
58065806
tuplestore_puttuple(new_tuplestore, original_insert_tuple);
58075807
else if (map != NULL)
58085808
{
5809-
HeapTuple converted = do_convert_tuple(newtup, map);
5809+
HeapTuple converted = execute_attr_map_tuple(newtup, map);
58105810

58115811
tuplestore_puttuple(new_tuplestore, converted);
58125812
pfree(converted);

‎src/backend/executor/execExprInterp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3285,7 +3285,7 @@ ExecEvalConvertRowtype(ExprState *state, ExprEvalStep *op, ExprContext *econtext
32853285
if (op->d.convert_rowtype.map != NULL)
32863286
{
32873287
/* Full conversion with attribute rearrangement needed */
3288-
result = do_convert_tuple(&tmptup, op->d.convert_rowtype.map);
3288+
result = execute_attr_map_tuple(&tmptup, op->d.convert_rowtype.map);
32893289
/* Result already has appropriate composite-datum header fields */
32903290
*op->resvalue = HeapTupleGetDatum(result);
32913291
}

0 commit comments

Comments
 (0)