public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dilip Kumar <[email protected]>
Subject: [PATCH 01/12] Disallow compressed data inside container types
Date: Tue, 16 Feb 2021 19:24:38 +0530
Currently, we have a general rule that Datums of container types
(rows, arrays, ranges, etc) must not contain any external TOAST
pointers. But the rule for the compressed data is not defined
and no specific rule is followed e.g. while constructing the array
we decompress the comprassed field but while contructing the row
in some cases we don't decompress the compressed data whereas in
the other cases we onlle decompress while flattening the external
toast pointers. This patch make a general rule for the compressed
data i.e. we don't allow the compressed data in the container type.
---
src/backend/access/common/heaptuple.c | 28 ++++++---------------------
src/backend/access/heap/heaptoast.c | 16 ++++++++-------
src/backend/executor/execTuples.c | 11 -----------
src/include/access/heaptoast.h | 4 ++--
4 files changed, 17 insertions(+), 42 deletions(-)
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 24a27e387d..5c3194f96d 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -983,30 +983,14 @@ heap_expand_tuple(HeapTuple sourceTuple, TupleDesc tupleDesc)
Datum
heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc)
{
- HeapTupleHeader td;
-
- /*
- * If the tuple contains any external TOAST pointers, we have to inline
- * those fields to meet the conventions for composite-type Datums.
- */
- if (HeapTupleHasExternal(tuple))
- return toast_flatten_tuple_to_datum(tuple->t_data,
- tuple->t_len,
- tupleDesc);
-
/*
- * Fast path for easy case: just make a palloc'd copy and insert the
- * correct composite-Datum header fields (since those may not be set if
- * the given tuple came from disk, rather than from heap_form_tuple).
+ * The tuple contains compressed/external TOAST pointers, so we have
+ * to inline those fields to meet the conventions for composite-type
+ * Datums.
*/
- td = (HeapTupleHeader) palloc(tuple->t_len);
- memcpy((char *) td, (char *) tuple->t_data, tuple->t_len);
-
- HeapTupleHeaderSetDatumLength(td, tuple->t_len);
- HeapTupleHeaderSetTypeId(td, tupleDesc->tdtypeid);
- HeapTupleHeaderSetTypMod(td, tupleDesc->tdtypmod);
-
- return PointerGetDatum(td);
+ return toast_flatten_tuple_to_datum(tuple->t_data,
+ tuple->t_len,
+ tupleDesc);
}
/*
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index 55bbe1d584..dd162daab9 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -549,14 +549,15 @@ toast_flatten_tuple_to_datum(HeapTupleHeader tup,
/* ----------
* toast_build_flattened_tuple -
*
- * Build a tuple containing no out-of-line toasted fields.
- * (This does not eliminate compressed or short-header datums.)
+ * Build a tuple containing no compressed/out-of-line toasted fields.
+ * (This does not eliminate short-header datums.)
*
* This is essentially just like heap_form_tuple, except that it will
- * expand any external-data pointers beforehand.
+ * expand any compressed/external-data pointers beforehand.
*
- * It's not very clear whether it would be preferable to decompress
- * in-line compressed datums while at it. For now, we don't.
+ * It is not necessary to decompress the compressed data for the
+ * correctness, but reflects an expectation that compression will be more
+ * effective if applied to the whole tuple not individual fields.
* ----------
*/
HeapTuple
@@ -589,9 +590,10 @@ toast_build_flattened_tuple(TupleDesc tupleDesc,
struct varlena *new_value;
new_value = (struct varlena *) DatumGetPointer(new_values[i]);
- if (VARATT_IS_EXTERNAL(new_value))
+ if (VARATT_IS_EXTERNAL(new_value) ||
+ VARATT_IS_COMPRESSED(new_value))
{
- new_value = detoast_external_attr(new_value);
+ new_value = detoast_attr(new_value);
new_values[i] = PointerGetDatum(new_value);
freeable_values[num_to_free++] = (Pointer) new_value;
}
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index 73c35df9c9..ca7fbed576 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -2189,13 +2189,6 @@ BuildTupleFromCStrings(AttInMetadata *attinmeta, char **values)
* memory context. Beware of code that changes context between the initial
* heap_form_tuple/etc call and calling HeapTuple(Header)GetDatum.
*
- * For performance-critical callers, it could be worthwhile to take extra
- * steps to ensure that there aren't TOAST pointers in the output of
- * heap_form_tuple to begin with. It's likely however that the costs of the
- * typcache lookup and tuple disassembly/reassembly are swamped by TOAST
- * dereference costs, so that the benefits of such extra effort would be
- * minimal.
- *
* XXX it would likely be better to create wrapper functions that produce
* a composite Datum from the field values in one step. However, there's
* enough code using the existing APIs that we couldn't get rid of this
@@ -2207,10 +2200,6 @@ HeapTupleHeaderGetDatum(HeapTupleHeader tuple)
Datum result;
TupleDesc tupDesc;
- /* No work if there are no external TOAST pointers in the tuple */
- if (!HeapTupleHeaderHasExternal(tuple))
- return PointerGetDatum(tuple);
-
/* Use the type data saved by heap_form_tuple to look up the rowtype */
tupDesc = lookup_rowtype_tupdesc(HeapTupleHeaderGetTypeId(tuple),
HeapTupleHeaderGetTypMod(tuple));
diff --git a/src/include/access/heaptoast.h b/src/include/access/heaptoast.h
index 8b29f1a986..c80af45012 100644
--- a/src/include/access/heaptoast.h
+++ b/src/include/access/heaptoast.h
@@ -128,8 +128,8 @@ extern Datum toast_flatten_tuple_to_datum(HeapTupleHeader tup,
/* ----------
* toast_build_flattened_tuple -
*
- * Build a tuple containing no out-of-line toasted fields.
- * (This does not eliminate compressed or short-header datums.)
+ * Build a tuple containing no compressed/out-of-line toasted fields.
+ * (This does not eliminate short-header datums.)
* ----------
*/
extern HeapTuple toast_build_flattened_tuple(TupleDesc tupleDesc,
--
2.17.0
--9Ek0hoCL9XbhcSqy
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0002-Built-in-compression-method.patch"
view thread (49+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 01/12] Disallow compressed data inside container types
In-Reply-To: <no-message-id-1879777@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox