From e12740fee3fb915bf01f7fe67efe4c5cd68e6078 Mon Sep 17 00:00:00 2001 From: Asim R P Date: Wed, 18 Dec 2019 13:41:25 +0530 Subject: [PATCH] Revert "When serializing a tuple for Motion, don't decompress compressed datums." This reverts commit c34e76e4093ce124163caec46bd3b901154f4f85. Thank you Ekta for finding this simple repro that demonstrates the problem with this patch and Jesse for initial analysis: CREATE TABLE foo(a text, b text); INSERT INTO foo SELECT repeat('123456789', 100000)::text as a, repeat('123456789', 10)::text as b; SELECT * FROM foo; The motion receiver has no idea whether a datum it received is compressed or not, because the varlena header is stripped off before sending the data. Heikki and I discussed two options to fix this: 1. Include varlena header when sending. This incurs at the most 8-byte overhead per variable length datum in a heap tuple. 2. Always send tuples as MemTuples. This is more desirable because it simplifies code, but also comes with performance cost. Let's evaluate the two options based on performance and then commit the best one. --- src/backend/cdb/motion/tupser.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/backend/cdb/motion/tupser.c b/src/backend/cdb/motion/tupser.c index 5771052551..eaafd5d8b4 100644 --- a/src/backend/cdb/motion/tupser.c +++ b/src/backend/cdb/motion/tupser.c @@ -16,7 +16,6 @@ #include "postgres.h" #include "access/htup.h" -#include "access/tuptoaster.h" #include "catalog/pg_type.h" #include "cdb/cdbmotion.h" #include "cdb/cdbsrlz.h" @@ -656,17 +655,9 @@ SerializeTuple(TupleTableSlot *slot, SerTupInfo *pSerInfo, struct directTranspor * avoid memory leakage: we want to force the detoast * allocation(s) to happen in our reset-able serialization * context. - * - * Only detoast, don't decompress. It saves network bandwidth - * to let the receiver decompress it. Furthermore, it's possible - * that the receiver doesn't need to decompress it at all, which - * can be a very big win. */ oldCtxt = MemoryContextSwitchTo(s_tupSerMemCtxt); - if (VARATT_IS_EXTERNAL(origattr)) - attr = PointerGetDatum(heap_tuple_fetch_attr((struct varlena *) DatumGetPointer(origattr))); - else - attr = origattr; + attr = PointerGetDatum(PG_DETOAST_DATUM_PACKED(origattr)); MemoryContextSwitchTo(oldCtxt); sz = VARSIZE_ANY_EXHDR(attr); -- GitLab