Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits

From: Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
Cc: Justin Pryzby <pryzby(at)telsasoft(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits
Date: 2020-07-14 17:51:37
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01.07.2020 12:38, Daniel Gustafsson wrote:
> This patch incurs a compiler warning, which probably is quite simple to fix:
> heapam.c: In function ‘heap_multi_insert’:
> heapam.c:2349:4: error: ‘recptr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> visibilitymap_set(relation, BufferGetBlockNumber(buffer), buffer,
> ^
> heapam.c:2136:14: note: ‘recptr’ was declared here
> XLogRecPtr recptr;
> ^
> Please fix and submit a new version, I'm marking the entry Waiting on Author in
> the meantime.
> cheers ./daniel
This patch looks very useful to me, so I want to pick it up.

The patch that fixes the compiler warning is in the attachment. Though,
I'm not
entirely satisfied with this fix. Also, the patch contains some FIXME
I'll test it more and send fixes this week.

Questions from the first review pass:

1) Do we need XLH_INSERT_ALL_VISIBLE_SET ? IIUC, in the patch it is always

2) What does this comment mean? Does XXX refers to the lsn comparison?
Since it
is definitely necessary to update the VM.

+             * XXX: This seems entirely unnecessary?
+             *
+             * FIXME: Theoretically we should only do this after we've
+             * modified the heap - but it's safe to do it here I think,
+             * because this means that the page previously was empty.
+             */
+            if (lsn > PageGetLSN(vmpage))
+                visibilitymap_set(reln, blkno, InvalidBuffer, lsn,
+                                  InvalidTransactionId, vmbits);

Anastasia Lubennikova
Postgres Professional:
The Russian Postgres Company

Attachment Content-Type Size
0005-copy-freeze-should-actually-freeze-right.patch text/x-patch 12.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Dave Cramer 2020-07-14 18:08:53 Re: Binary support for pgoutput plugin
Previous Message Alvaro Herrera 2020-07-14 17:20:25 Re: recovering from "found xmin ... from before relfrozenxid ..."