From 4489b8e89557c92d932dd944db07c99eea8c088c Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Thu, 4 Mar 2021 16:33:10 +0530 Subject: [PATCH 3/8] Disallow compressed data inside container types 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 compressed field but while constructing the row in some cases we don't decompress the compressed data whereas in the other cases we only 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. Dilip Kumar based on idea from Robert Haas --- src/backend/access/common/heaptuple.c | 9 +-- src/backend/access/heap/heaptoast.c | 4 +- src/backend/executor/execExprInterp.c | 6 +- src/backend/executor/execTuples.c | 4 -- src/backend/utils/adt/expandedrecord.c | 76 +++++++++----------------- src/backend/utils/adt/jsonfuncs.c | 3 +- src/include/funcapi.h | 4 +- src/pl/plpgsql/src/pl_exec.c | 3 +- 8 files changed, 38 insertions(+), 71 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index c36c283253..eb9f016dfa 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -984,15 +984,12 @@ Datum heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc) { /* - * If the tuple contains any external TOAST pointers, we have to inline - * those fields to meet the conventions for composite-type Datums. + * We have to inline any external/compressed data to meet the conventions + * for composite-type Datums. */ - if (HeapTupleHasExternal(tuple)) - return toast_flatten_tuple_to_datum(tuple->t_data, + return toast_flatten_tuple_to_datum(tuple->t_data, tuple->t_len, tupleDesc); - else - return heap_copy_tuple_as_raw_datum(tuple, tupleDesc); } /* ---------------- diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c index 55bbe1d584..b09462348b 100644 --- a/src/backend/access/heap/heaptoast.c +++ b/src/backend/access/heap/heaptoast.c @@ -589,9 +589,9 @@ 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/execExprInterp.c b/src/backend/executor/execExprInterp.c index c3754acca4..71e6f41fee 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -2845,8 +2845,7 @@ ExecEvalRow(ExprState *state, ExprEvalStep *op) { Form_pg_attribute attr = TupleDescAttr(op->d.row.tupdesc, i); - if (op->d.row.elemnulls[i] || attr->attlen != -1 || - !VARATT_IS_EXTERNAL(DatumGetPointer(op->d.row.elemvalues[i]))) + if (op->d.row.elemnulls[i] || attr->attlen != -1) continue; op->d.row.elemvalues[i] = @@ -3103,8 +3102,7 @@ ExecEvalFieldStoreForm(ExprState *state, ExprEvalStep *op, ExprContext *econtext { Form_pg_attribute attr = TupleDescAttr(*op->d.fieldstore.argdesc, i); - if (op->d.fieldstore.nulls[i] || attr->attlen != -1 || - !VARATT_IS_EXTERNAL(DatumGetPointer(op->d.fieldstore.values[i]))) + if (op->d.fieldstore.nulls[i] || attr->attlen != -1) continue; op->d.fieldstore.values[i] = PointerGetDatum( PG_DETOAST_DATUM_PACKED(op->d.fieldstore.values[i])); diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 73c35df9c9..f11546468e 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -2207,10 +2207,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/backend/utils/adt/expandedrecord.c b/src/backend/utils/adt/expandedrecord.c index e19491ecf7..3cbc256671 100644 --- a/src/backend/utils/adt/expandedrecord.c +++ b/src/backend/utils/adt/expandedrecord.c @@ -673,14 +673,6 @@ ER_get_flat_size(ExpandedObjectHeader *eohptr) erh->er_typmod = tupdesc->tdtypmod; } - /* - * If we have a valid flattened value without out-of-line fields, we can - * just use it as-is. - */ - if (erh->flags & ER_FLAG_FVALUE_VALID && - !(erh->flags & ER_FLAG_HAVE_EXTERNAL)) - return erh->fvalue->t_len; - /* If we have a cached size value, believe that */ if (erh->flat_size) return erh->flat_size; @@ -693,38 +685,36 @@ ER_get_flat_size(ExpandedObjectHeader *eohptr) tupdesc = erh->er_tupdesc; /* - * Composite datums mustn't contain any out-of-line values. + * Composite datums mustn't contain any out-of-line/compressed values. */ - if (erh->flags & ER_FLAG_HAVE_EXTERNAL) + for (i = 0; i < erh->nfields; i++) { - for (i = 0; i < erh->nfields; i++) - { - Form_pg_attribute attr = TupleDescAttr(tupdesc, i); + Form_pg_attribute attr = TupleDescAttr(tupdesc, i); - if (!erh->dnulls[i] && - !attr->attbyval && attr->attlen == -1 && - VARATT_IS_EXTERNAL(DatumGetPointer(erh->dvalues[i]))) - { - /* - * expanded_record_set_field_internal can do the actual work - * of detoasting. It needn't recheck domain constraints. - */ - expanded_record_set_field_internal(erh, i + 1, - erh->dvalues[i], false, - true, - false); - } + if (!erh->dnulls[i] && + !attr->attbyval && attr->attlen == -1 && + (VARATT_IS_EXTERNAL(DatumGetPointer(erh->dvalues[i])) || + VARATT_IS_COMPRESSED(DatumGetPointer(erh->dvalues[i])))) + { + /* + * expanded_record_set_field_internal can do the actual work + * of detoasting. It needn't recheck domain constraints. + */ + expanded_record_set_field_internal(erh, i + 1, + erh->dvalues[i], false, + true, + false); } - - /* - * We have now removed all external field values, so we can clear the - * flag about them. This won't cause ER_flatten_into() to mistakenly - * take the fast path, since expanded_record_set_field() will have - * cleared ER_FLAG_FVALUE_VALID. - */ - erh->flags &= ~ER_FLAG_HAVE_EXTERNAL; } + /* + * We have now removed all external field values, so we can clear the + * flag about them. This won't cause ER_flatten_into() to mistakenly + * take the fast path, since expanded_record_set_field() will have + * cleared ER_FLAG_FVALUE_VALID. + */ + erh->flags &= ~ER_FLAG_HAVE_EXTERNAL; + /* Test if we currently have any null values */ hasnull = false; for (i = 0; i < erh->nfields; i++) @@ -770,19 +760,6 @@ ER_flatten_into(ExpandedObjectHeader *eohptr, Assert(erh->er_magic == ER_MAGIC); - /* Easy if we have a valid flattened value without out-of-line fields */ - if (erh->flags & ER_FLAG_FVALUE_VALID && - !(erh->flags & ER_FLAG_HAVE_EXTERNAL)) - { - Assert(allocated_size == erh->fvalue->t_len); - memcpy(tuphdr, erh->fvalue->t_data, allocated_size); - /* The original flattened value might not have datum header fields */ - HeapTupleHeaderSetDatumLength(tuphdr, allocated_size); - HeapTupleHeaderSetTypeId(tuphdr, erh->er_typeid); - HeapTupleHeaderSetTypMod(tuphdr, erh->er_typmod); - return; - } - /* Else allocation should match previous get_flat_size result */ Assert(allocated_size == erh->flat_size); @@ -1155,11 +1132,12 @@ expanded_record_set_field_internal(ExpandedRecordHeader *erh, int fnumber, if (expand_external) { if (attr->attlen == -1 && - VARATT_IS_EXTERNAL(DatumGetPointer(newValue))) + (VARATT_IS_EXTERNAL(DatumGetPointer(newValue)) || + VARATT_IS_COMPRESSED(DatumGetPointer(newValue)))) { /* Detoasting should be done in short-lived context. */ oldcxt = MemoryContextSwitchTo(get_short_term_cxt(erh)); - newValue = PointerGetDatum(detoast_external_attr((struct varlena *) DatumGetPointer(newValue))); + newValue = PointerGetDatum(detoast_attr((struct varlena *) DatumGetPointer(newValue))); MemoryContextSwitchTo(oldcxt); } else diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index c3d464f42b..821aa8fbdb 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -3388,8 +3388,7 @@ populate_record(TupleDesc tupdesc, &field, &nulls[i]); - if (!nulls[i] && att->attlen == -1 && - VARATT_IS_EXTERNAL(DatumGetPointer(values[i]))) + if (!nulls[i] && att->attlen == -1) values[i] = PointerGetDatum(PG_DETOAST_DATUM_PACKED(values[i])); } diff --git a/src/include/funcapi.h b/src/include/funcapi.h index 8ba7ae211f..c869012873 100644 --- a/src/include/funcapi.h +++ b/src/include/funcapi.h @@ -208,10 +208,10 @@ extern TupleDesc build_function_result_tupdesc_t(HeapTuple procTuple); * Macro declarations/inline functions: * HeapTupleHeaderGetRawDatum(HeapTupleHeader tuple) - same as * HeapTupleHeaderGetDatum but the input tuple should not contain - * external varlena + * external/compressed varlena * HeapTupleGetDatum(HeapTuple tuple) - convert a HeapTuple to a Datum. * HeapTupleGetRawDatum(HeapTuple tuple) - same as HeapTupleGetDatum - * but the input tuple should not contain external varlena + * but the input tuple should not contain external/compressed varlena * * Obsolete routines and macros: * TupleDesc RelationNameGetTupleDesc(const char *relname) - Use to get a diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index fd073767bc..0519253cbe 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -7300,8 +7300,7 @@ make_tuple_from_row(PLpgSQL_execstate *estate, &dvalues[i], &nulls[i]); if (fieldtypeid != TupleDescAttr(tupdesc, i)->atttypid) return NULL; - if (!nulls[i] && TupleDescAttr(tupdesc, i)->attlen == -1 && - VARATT_IS_EXTERNAL(DatumGetPointer(dvalues[i]))) + if (!nulls[i] && TupleDescAttr(tupdesc, i)->attlen == -1) dvalues[i] = PointerGetDatum(PG_DETOAST_DATUM_PACKED(dvalues[i])); /* XXX should we insist on typmod match, too? */ } -- 2.17.0