From fe578cbd4b4262496d44cb5cd110eae3c4aff48c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 8 Aug 2017 19:18:12 -0400 Subject: [PATCH] Fix datumSerialize infrastructure to not crash on non-varlena data. Commit 1efc7e538 did a poor job of emulating existing logic for touching Datums that might be expanded-object pointers. It didn't check for typlen being -1 first, which meant it could crash on fixed-length pass-by-ref values, and probably on cstring values as well. It also didn't use DatumGetPointer before VARATT_IS_EXTERNAL_EXPANDED, which while currently harmless is not according to documentation nor prevailing style. I also think the lack of any explanation as to why datumSerialize makes these particular nonobvious choices is pretty awful, so fix that. Per report from Jarred Ward. Back-patch to 9.6 where this code came in. Discussion: https://postgr.es/m/6F61E6D2-2F5E-4794-9479-A429BE1CEA4B@simple.com --- src/backend/utils/adt/datum.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/backend/utils/adt/datum.c b/src/backend/utils/adt/datum.c index c6c296b960..d02e3c8110 100644 --- a/src/backend/utils/adt/datum.c +++ b/src/backend/utils/adt/datum.c @@ -264,11 +264,11 @@ datumEstimateSpace(Datum value, bool isnull, bool typByVal, int typLen) /* no need to use add_size, can't overflow */ if (typByVal) sz += sizeof(Datum); - else if (VARATT_IS_EXTERNAL_EXPANDED(value)) + else if (typLen == -1 && + VARATT_IS_EXTERNAL_EXPANDED(DatumGetPointer(value))) { - ExpandedObjectHeader *eoh = DatumGetEOHP(value); - - sz += EOH_get_flat_size(eoh); + /* Expanded objects need to be flattened, see comment below */ + sz += EOH_get_flat_size(DatumGetEOHP(value)); } else sz += datumGetSize(value, typByVal, typLen); @@ -282,6 +282,13 @@ datumEstimateSpace(Datum value, bool isnull, bool typByVal, int typLen) * * Serialize a possibly-NULL datum into caller-provided storage. * + * Note: "expanded" objects are flattened so as to produce a self-contained + * representation, but other sorts of toast pointers are transferred as-is. + * This is because the intended use of this function is to pass the value + * to another process within the same database server. The other process + * could not access an "expanded" object within this process's memory, but + * we assume it can dereference the same TOAST pointers this one can. + * * The format is as follows: first, we write a 4-byte header word, which * is either the length of a pass-by-reference datum, -1 for a * pass-by-value datum, or -2 for a NULL. If the value is NULL, nothing @@ -306,7 +313,8 @@ datumSerialize(Datum value, bool isnull, bool typByVal, int typLen, header = -2; else if (typByVal) header = -1; - else if (VARATT_IS_EXTERNAL_EXPANDED(value)) + else if (typLen == -1 && + VARATT_IS_EXTERNAL_EXPANDED(DatumGetPointer(value))) { eoh = DatumGetEOHP(value); header = EOH_get_flat_size(eoh);