Date: Thu, 5 Mar 2020 11:18:59 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>,
	Paul Jungwirth <pj@illuminatedcomputing.com>
Subject: Re: useless RangeIOData->typiofunc
Message-ID: <20200305141859.GA6736@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="jRHKVT23PllUwdXP"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <8261.1583361293@sss.pgh.pa.us>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk


--jRHKVT23PllUwdXP
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On 2020-Mar-04, Tom Lane wrote:

> Hm, I'm not sure that really lessens the cognitive load any, but
> if you do commit this please fix the dangling reference you left
> in the nearby comment:
> 
>  {
>     TypeCacheEntry *typcache;   /* range type's typcache entry */
> -   Oid         typiofunc;      /* element type's I/O function */
>     Oid         typioparam;     /* element type's I/O parameter */
>     FmgrInfo    proc;           /* lookup result for typiofunc */
>                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  } RangeIOData;

Thanks -- ISTM it makes more sense to put the FmgrInfo before the
typioparam too:

typedef struct RangeIOData
{
    TypeCacheEntry *typcache;    /* range type's typcache entry */
    FmgrInfo    proc;            /* element type's I/O function */
    Oid         typioparam;      /* element type's I/O parameter */
} RangeIOData;

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--jRHKVT23PllUwdXP
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="range-typiofunc-cleanup-2.patch"

diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c
index 490bc2ae81..c6bb21219b 100644
--- a/src/backend/utils/adt/rangetypes.c
+++ b/src/backend/utils/adt/rangetypes.c
@@ -49,9 +49,8 @@
 typedef struct RangeIOData
 {
 	TypeCacheEntry *typcache;	/* range type's typcache entry */
-	Oid			typiofunc;		/* element type's I/O function */
+	FmgrInfo	proc;			/* element type's I/O function */
 	Oid			typioparam;		/* element type's I/O parameter */
-	FmgrInfo	proc;			/* lookup result for typiofunc */
 } RangeIOData;
 
 
@@ -309,6 +308,7 @@ get_range_io_data(FunctionCallInfo fcinfo, Oid rngtypid, IOFuncSelector func)
 		bool		typbyval;
 		char		typalign;
 		char		typdelim;
+		Oid			typiofunc;
 
 		cache = (RangeIOData *) MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
 												   sizeof(RangeIOData));
@@ -324,9 +324,9 @@ get_range_io_data(FunctionCallInfo fcinfo, Oid rngtypid, IOFuncSelector func)
 						 &typalign,
 						 &typdelim,
 						 &cache->typioparam,
-						 &cache->typiofunc);
+						 &typiofunc);
 
-		if (!OidIsValid(cache->typiofunc))
+		if (!OidIsValid(typiofunc))
 		{
 			/* this could only happen for receive or send */
 			if (func == IOFunc_receive)
@@ -340,7 +340,7 @@ get_range_io_data(FunctionCallInfo fcinfo, Oid rngtypid, IOFuncSelector func)
 						 errmsg("no binary output function available for type %s",
 								format_type_be(cache->typcache->rngelemtype->type_id))));
 		}
-		fmgr_info_cxt(cache->typiofunc, &cache->proc,
+		fmgr_info_cxt(typiofunc, &cache->proc,
 					  fcinfo->flinfo->fn_mcxt);
 
 		fcinfo->flinfo->fn_extra = (void *) cache;

--jRHKVT23PllUwdXP--