Michel Pelletier <pelletier.mic...@gmail.com> writes:

> On Mon, Nov 18, 2024 at 7:42 PM Andy Fan <zhihuifan1...@163.com> wrote:
>
>  Andy Fan <zhihuifan1...@163.com> writes:
>
>  >
>  > make check-world passed after applying this patch.
>
>  v2 changes the places of Assert, which is missed in v1 by mistakes.
>
> I'm not an expert in this end of the code but it looks correct to me,
> my only comment would be maybe add a new function
> detoast_external_expanded_attr that is called from both
> detoast_external_attr and detoast_attr so the EOHP stuff stays 
> hidden behind a function.

Thanks for the double check. Your suggestion looks good to me. v3 is
attached for this.

Just that I'm not sure about the function name. To be
consistent with other functions in this area, e.g. toast_fetch_datum,
totast_decompressed_datum, it looks like this function would be
toast_EOH/expaned_attr. Currently I'm using toast_EOH_attr. I'm open for
other names.

I find detoast_attr_slice has the similar issues as detoast_attr, so
included in v3 as well. 

-- 
Best Regards
Andy Fan

>From c92d96ea219dceeb510927395bdc49787521c536 Mon Sep 17 00:00:00 2001
From: Andy Fan <zhihuifan1...@163.com>
Date: Tue, 29 Oct 2024 14:05:05 +0800
Subject: [PATCH v3 1/1] Using more specific code when detoasting an expanded
 datum.

In the detoast_attr/detoast_attr_slice function,
VARATT_IS_EXTERNAL_ONDISK and VARATT_IS_EXTERNAL_INDIRECT are checked
first, and then VARATT_IS_EXTERNAL_EXPANDED, however when it's true,
detoast_external_attr is called which checks the two cases again.
The attached patch uses a more specific code to handle this.
---
 src/backend/access/common/detoast.c | 48 ++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index d92e683384..249a4544de 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -28,6 +28,7 @@ static struct varlena *toast_fetch_datum_slice(struct varlena *attr,
 											   int32 slicelength);
 static struct varlena *toast_decompress_datum(struct varlena *attr);
 static struct varlena *toast_decompress_datum_slice(struct varlena *attr, int32 slicelength);
+static struct varlena *toast_EOH_datum(struct varlena *attr);
 
 /* ----------
  * detoast_external_attr -
@@ -79,16 +80,7 @@ detoast_external_attr(struct varlena *attr)
 	}
 	else if (VARATT_IS_EXTERNAL_EXPANDED(attr))
 	{
-		/*
-		 * This is an expanded-object pointer --- get flat format
-		 */
-		ExpandedObjectHeader *eoh;
-		Size		resultsize;
-
-		eoh = DatumGetEOHP(PointerGetDatum(attr));
-		resultsize = EOH_get_flat_size(eoh);
-		result = (struct varlena *) palloc(resultsize);
-		EOH_flatten_into(eoh, result, resultsize);
+		result = toast_EOH_datum(attr);
 	}
 	else
 	{
@@ -158,12 +150,7 @@ detoast_attr(struct varlena *attr)
 	}
 	else if (VARATT_IS_EXTERNAL_EXPANDED(attr))
 	{
-		/*
-		 * This is an expanded-object pointer --- get flat format
-		 */
-		attr = detoast_external_attr(attr);
-		/* flatteners are not allowed to produce compressed/short output */
-		Assert(!VARATT_IS_EXTENDED(attr));
+		attr = toast_EOH_datum(attr);
 	}
 	else if (VARATT_IS_COMPRESSED(attr))
 	{
@@ -279,7 +266,7 @@ detoast_attr_slice(struct varlena *attr,
 	else if (VARATT_IS_EXTERNAL_EXPANDED(attr))
 	{
 		/* pass it off to detoast_external_attr to flatten */
-		preslice = detoast_external_attr(attr);
+		preslice = toast_EOH_datum(attr);
 	}
 	else
 		preslice = attr;
@@ -491,7 +478,6 @@ toast_decompress_datum(struct varlena *attr)
 	}
 }
 
-
 /* ----------
  * toast_decompress_datum_slice -
  *
@@ -534,6 +520,32 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
 	}
 }
 
+
+/* ----------
+ * toast_EOH_datum -
+ *
+ *	Reconstruct a non-extended varlena form from a EOP version of a
+ *  varlena datum.
+ */
+static struct varlena *
+toast_EOH_datum(struct varlena *attr)
+{
+	/*
+	 * This is an expanded-object pointer --- get flat format
+	 */
+	ExpandedObjectHeader *eoh;
+	Size		resultsize;
+	struct varlena *result;
+
+	Assert(VARATT_IS_EXTERNAL_EXPANDED(attr));
+	eoh = DatumGetEOHP(PointerGetDatum(attr));
+	resultsize = EOH_get_flat_size(eoh);
+	result = (struct varlena *) palloc(resultsize);
+	EOH_flatten_into(eoh, (void *) result, resultsize);
+	Assert(!VARATT_IS_EXTENDED(result));
+	return result;
+}
+
 /* ----------
  * toast_raw_datum_size -
  *
-- 
2.45.1

Reply via email to