Suspicious place in heap_prepare_freeze_tuple()
Hi!
Playing around freezing tuple I found suspicious piece of code:
heap_prepare_freeze_tuple():
...
frz->t_infomask = tuple->t_infomask;
...
frz->t_infomask &= ~HEAP_XMAX_BITS;
frz->xmax = newxmax;
if (flags & FRM_MARK_COMMITTED)
frz->t_infomask &= HEAP_XMAX_COMMITTED;
Seems, in last line it should be a bitwise OR instead of AND. Now this line
cleans all bits in t_infomask which later will be copied directly in tuple.
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
Attachments:
heap_prepare_freeze_tuple.difftext/x-patch; name=heap_prepare_freeze_tuple.diffDownload
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 8deb344d09..ec227bac80 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6639,7 +6639,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
frz->t_infomask &= ~HEAP_XMAX_BITS;
frz->xmax = newxmax;
if (flags & FRM_MARK_COMMITTED)
- frz->t_infomask &= HEAP_XMAX_COMMITTED;
+ frz->t_infomask |= HEAP_XMAX_COMMITTED;
changed = true;
totally_frozen = false;
}
Teodor Sigaev wrote:
Playing around freezing tuple I found suspicious piece of code:
heap_prepare_freeze_tuple():
...
frz->t_infomask = tuple->t_infomask;
...
frz->t_infomask &= ~HEAP_XMAX_BITS;
frz->xmax = newxmax;
if (flags & FRM_MARK_COMMITTED)
frz->t_infomask &= HEAP_XMAX_COMMITTED;Seems, in last line it should be a bitwise OR instead of AND. Now this line
cleans all bits in t_infomask which later will be copied directly in tuple.
I think you're right.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jul 6, 2017 at 1:36 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Teodor Sigaev wrote:
Playing around freezing tuple I found suspicious piece of code:
heap_prepare_freeze_tuple():
...
frz->t_infomask = tuple->t_infomask;
...
frz->t_infomask &= ~HEAP_XMAX_BITS;
frz->xmax = newxmax;
if (flags & FRM_MARK_COMMITTED)
frz->t_infomask &= HEAP_XMAX_COMMITTED;Seems, in last line it should be a bitwise OR instead of AND. Now this line
cleans all bits in t_infomask which later will be copied directly in tuple.I think you're right.
I also think that's right. Should we back-patch it down to 9.3?
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Masahiko Sawada wrote:
On Thu, Jul 6, 2017 at 1:36 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
Teodor Sigaev wrote:
Playing around freezing tuple I found suspicious piece of code:
heap_prepare_freeze_tuple():
...
frz->t_infomask = tuple->t_infomask;
...
frz->t_infomask &= ~HEAP_XMAX_BITS;
frz->xmax = newxmax;
if (flags & FRM_MARK_COMMITTED)
frz->t_infomask &= HEAP_XMAX_COMMITTED;Seems, in last line it should be a bitwise OR instead of AND. Now this line
cleans all bits in t_infomask which later will be copied directly in tuple.I think you're right.
I also think that's right. Should we back-patch it down to 9.3?
Of course. I think this could cause data corruption.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers