Hello Pavan,
Thank you for the patch. It seems to me that while performing COPY
FREEZE, if we've copied tuples in a previously emptied page, we can
set the PageSetAllVisible(page) in heap_muli_insert only. Something
like,
bool init = (ItemPointerGetOffsetNumber(&(heaptuples[ndone]->t_self))
== FirstOffsetNumber &&
PageGetMaxOffsetNumber(page) == FirstOffsetNumber + nthispage - 1);
if (init && (options & HEAP_INSERT_FROZEN))
PageSetAllVisible(page);
Later, you can skip the pages for
CheckAndSetAllVisibleBulkInsertState() where PD_ALL_VISIBLE flag is
already set. Do you think it's correct?
On Thu, Feb 21, 2019 at 11:35 AM Pavan Deolasee
<pavan.deola...@gmail.com> wrote:
>
> Hi,
>
> Jeff Janes raised an issue [1] about PD_ALL_VISIBLE not being set correctly
> while loading data via COPY FREEZE and had also posted a draft patch.
>
> I now have what I think is a more complete patch. I took a slightly different
> approach and instead of setting PD_ALL_VISIBLE bit initially and then not
> clearing it during insertion, we now recheck the page for all-frozen,
> all-visible tuples just before switching to a new page. This allows us to
> then also mark set the visibility map bit, like we do in vacuumlazy.c
>
> Some special treatment is required to handle the last page before bulk insert
> it shutdown. We could have chosen not to do anything special for the last
> page and let it remain unfrozen, but I thought it makes sense to take that
> extra effort so that we can completely freeze the table and set all VM bits
> at the end of COPY FREEZE.
>
> Let me know what you think.
>
> Thanks,
> Pavan
>
> [1]
> https://www.postgresql.org/message-id/CAMkU%3D1w3osJJ2FneELhhNRLxfZitDgp9FPHee08NT2FQFmz_pQ%40mail.gmail.com
>
> --
> Pavan Deolasee http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com
