From 34a17fd6df4f531527b976df043aed6fb5b6add0 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 16:39:17 +0200
Subject: [PATCH 2/2] review reworks

---
 src/backend/access/heap/tuptoaster.c | 47 +++++++++++++++-------------
 src/common/pg_lzcompress.c           | 19 +++++------
 src/include/common/pg_lzcompress.h   |  2 +-
 3 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index 1eb6ccaca2..809a52196c 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -62,7 +62,7 @@ typedef struct toast_compress_header
 #define TOAST_COMPRESS_HDRSZ		((int32) sizeof(toast_compress_header))
 #define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
 #define TOAST_COMPRESS_SIZE(ptr)	((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)
-#define TOAST_COMPRESS_DATA(ptr) \
+#define TOAST_COMPRESS_RAWDATA(ptr) \
 	(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
 #define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
 	(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -254,7 +254,7 @@ heap_tuple_untoast_attr(struct varlena *attr)
  *		Public entry point to get back part of a toasted value
  *		from compression or external storage.
  *
- * Note if slicelength is negative, return suffix of the value.
+ * Note: when slicelength is negative, return suffix of the value.
  * ----------
  */
 struct varlena *
@@ -277,17 +277,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
 			return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
 
 		/*
-		 * If only need the prefix slice, fetch enough datums to decompress.
-		 * Otherwise, fetch all datums.
+		 * When a prefix of the value is requested, fetch enough slices to
+		 * decompress. Otherwise, fetch all slices.
 		 */
 		if (slicelength > 0 && sliceoffset >= 0)
 		{
 			int32 max_size;
+
+			/*
+			 * Determine maximum amount of compressed data needed for a prefix
+			 * of a given length (after decompression).
+			 */
 			max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
 													TOAST_COMPRESS_SIZE(attr));
+
 			/*
-			 * Be sure to get enough compressed slice
-			 * and compressed marker will get set automatically
+			 * Fetch enough compressed slices, compressed marker will get set
+			 * automatically.
 			 */
 			preslice = toast_fetch_datum_slice(attr, 0, max_size);
 		}
@@ -1409,7 +1415,7 @@ toast_compress_datum(Datum value)
 	 */
 	len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
 						valsize,
-						TOAST_COMPRESS_DATA(tmp),
+						TOAST_COMPRESS_RAWDATA(tmp),
 						PGLZ_strategy_default);
 	if (len >= 0 &&
 		len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2050,7 +2056,8 @@ toast_fetch_datum(struct varlena *attr)
  *	in the toast relation
  *
  *	Note that this function supports non-compressed external datums
- *	and compressed external datum slices at the start of the object.
+ *	and compressed external datums (in which case the requrested slice
+ *  has to be a prefix, i.e. sliceoffset has to be 0).
  * ----------
  */
 static struct varlena *
@@ -2092,7 +2099,8 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
 
 	/*
 	 * It's meaningful to fetch slices at the start of a compressed datum,
-	 * since we can decompress the prefix raw data from it.
+	 * since we can decompress the prefix raw data without decompressing the
+	 * whole value.
 	 */
 	Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
 
@@ -2105,26 +2113,23 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
 		length = 0;
 	}
 
+	/*
+	 * When fetching a prefix of a compressed external datum, account for
+	 * the rawsize tracking amount of raw data (it's stored at the beginning
+	 * as an int32 value).
+	 */
 	if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)
-	{
-		/*
-		 * If we are going to fetch the compressed external datum slice
-		 * at the start of the object, also should fetch rawsize field used
-		 * to record the size of the raw data.
-		 */
 		length = length + sizeof(int32);
-	}
 
 	if (((sliceoffset + length) > attrsize) || length < 0)
 		length = attrsize - sliceoffset;
 
 	result = (struct varlena *) palloc(length + VARHDRSZ);
 
-	if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {
+	if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
 		SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
-	} else {
+	else
 		SET_VARSIZE(result, length + VARHDRSZ);
-	}
 
 	if (length == 0)
 		return result;			/* Can save a lot of work at this point! */
@@ -2307,7 +2312,7 @@ toast_decompress_datum(struct varlena *attr)
 		palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
 	SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
 
-	if (pglz_decompress(TOAST_COMPRESS_DATA(attr),
+	if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
 						TOAST_COMPRESS_SIZE(attr),
 						VARDATA(result),
 						TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
@@ -2334,7 +2339,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
 
 	result = (struct varlena *) palloc(slicelength + VARHDRSZ);
 
-	rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),
+	rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
 							  VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
 							  VARDATA(result),
 							  slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.c
index ac7d66db77..18766ade02 100644
--- a/src/common/pg_lzcompress.c
+++ b/src/common/pg_lzcompress.c
@@ -773,27 +773,28 @@ pglz_decompress(const char *source, int32 slen, char *dest,
 }
 
 
-
 /* ----------
  * pglz_max_compressed_size -
  *
- * 		Calculate the maximum size of the compressed slice corresponding to the
- * 		raw slice. Return the maximum size, or total compressed size if maximum
- * 		size is larger than total compressed size.
+ *		Calculate the maximum compressed size for a given amount of raw data.
+ *		Return the maximum size, or total compressed size if maximum size is
+ *		larger than total compressed size.
  * ----------
  */
 int32
-pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)
+pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size)
 {
-	int32 result;
+	int32 compressed_size;
 
 	/*
 	 * Use int64 to prevent overflow during calculation.
 	 */
-	result = (int32)((int64)raw_slice_size * 9 + 8) / 8;
+	compressed_size = (int32) ((int64) rawsize * 9 + 8) / 8;
 
 	/*
-	 * Note that slice compressed size will never be larger than total compressed size.
+	 * Maximum compressed size can't be larger than total compressed size.
 	 */
-	return result > total_compressed_size ? total_compressed_size: result;
+	compressed_size = Min(compressed_size, total_compressed_size);
+
+	return compressed_size;
 }
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.h
index cda3e1d364..e59fe2eaea 100644
--- a/src/include/common/pg_lzcompress.h
+++ b/src/include/common/pg_lzcompress.h
@@ -87,6 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
 						   const PGLZ_Strategy *strategy);
 extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
 							 int32 rawsize, bool check_complete);
-extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
+extern int32 pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size);
 
 #endif							/* _PG_LZCOMPRESS_H_ */
-- 
2.20.1

