On Tue, May 5, 2020 at 10:25 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, May 5, 2020 at 9:27 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Mon, May 4, 2020 at 5:16 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Fri, May 1, 2020 at 8:41 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > > > > > > 5. Shouldn't we add a check in table_scan_sample_next_block and > > > table_scan_sample_next_tuple APIs as well? > > > > I am not sure that we need to do that, Because generally, we want to > > avoid getting any wrong system table tuple which we can use for taking > > some decision or decode tuple. But, I don't think that > > table_scan_sample falls under that category. > > > > Hmm, I am asking a check similar to what you have in function > table_scan_bitmap_next_block(), can't we have that one?
Yeah we can put that and there is no harm in that, but my point is the table_scan_bitmap_next_block and other functions where I have put the check are used for fetching the tuple which can be used for decoding tuple or taking some decision, but IMHO, table_scan_sample_next_tuple is only used for analyzing the table. So do we really need to do that? Am I missing something here? BTW, I > noticed a below spurious line removal in the patch we are talking > about. > > +/* > * These are updated by GetSnapshotData. We initialize them this way > * for the convenience of TransactionIdIsInProgress: even in bootstrap > * mode, we don't want it to say that BootstrapTransactionId is in progress. > @@ -2043,7 +2055,6 @@ SetupHistoricSnapshot(Snapshot > historic_snapshot, HTAB *tuplecids) > tuplecid_data = tuplecids; > } > > - Okay, I will take care. of this. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
