Subject: Re: Fix inconsistencies for v12 (pass 2)
To: Michael Paquier <michael@paquier.xyz>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>,
 Andres Freund <andres@anarazel.de>
References: <e59e4d46-7c78-afb1-6edc-df77f6d8ccc9@gmail.com>
 <20190613081007.GH1643@paquier.xyz>
From: Alexander Lakhin <exclusion@gmail.com>
Message-ID: <dec6aae8-2d63-639f-4d50-20e229fb83e3@gmail.com>
Date: Thu, 13 Jun 2019 11:28:42 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.0
MIME-Version: 1.0
In-Reply-To: <20190613081007.GH1643@paquier.xyz>
Content-Type: multipart/mixed;
 boundary="------------71EF3284435520CC5DAAFBEC"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------71EF3284435520CC5DAAFBEC
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit

Hello Michael,
13.06.2019 11:10, Michael Paquier wrote:
> On Wed, Jun 12, 2019 at 05:34:06PM +0300, Alexander Lakhin wrote:
>> I can't see another inconsistencies for v12 for now, but there are some
>> that appeared before.
>> If this work can be performed more effectively or should be
>> postponed/canceled, please let me know.
> Note sure that it is much productive to have one patch with basically 
> one-liners in each one...  Anyway..
As the proposed fixes are independent, I decided to separate them. I
will make a single patch on next iteration.
> All your suggestions are right.  I do have one doubt for the
> suggestion in execnodes.h:
> @@ -1571,7 +1571,6 @@ typedef struct TidScanState
>     int         tss_NumTids;
>     int         tss_TidPtr;
>     ItemPointerData *tss_TidList;
> -   HeapTupleData tss_htup;
> } TidScanState;
> The last trace of tss_htup has been removed as of 2e3da03, and I see
> no mention of it in the related thread.  Andres, is that intentional
> for table AMs to keep a trace of a currently-fetched tuple for a TID
> scan or something that can be removed?  The field is still
> documented, so the patch is incomplete if we finish by removing the
> field.  And my take is that we should keep it.
Yes, you're right. I've completed the patch for a possible elimination
of the field.

Best regards,
Alexander

--------------71EF3284435520CC5DAAFBEC
Content-Type: text/x-patch;
 name="tss_htup.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="tss_htup.patch"

diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 99b9fa414f..03166754c0 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1560,7 +1560,6 @@ typedef struct BitmapHeapScanState
  *		NumTids		   number of tids in this scan
  *		TidPtr		   index of currently fetched tid
  *		TidList		   evaluated item pointers (array of size NumTids)
- *		htup		   currently-fetched tuple, if any
  * ----------------
  */
 typedef struct TidScanState
@@ -1571,7 +1570,6 @@ typedef struct TidScanState
 	int			tss_NumTids;
 	int			tss_TidPtr;
 	ItemPointerData *tss_TidList;
-	HeapTupleData tss_htup;
 } TidScanState;
 
 /* ----------------

--------------71EF3284435520CC5DAAFBEC--