Re: [PATCH] Remove twice assignment with var pageop (nbtree.c).

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Ranier Vilela <ranier_gyn(at)hotmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] Remove twice assignment with var pageop (nbtree.c).
Date: 2019-12-19 15:55:42
Message-ID: 404.1576770942@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Tue, Nov 26, 2019 at 01:45:10PM +0000, Ranier Vilela wrote:
>> Same case on nbtpage.c at line 1637, with var opaque.
>> make check, passed all 195 tests here with all commits.

> You were right about both of these, so removed in master. I am
> surprised no one else saw this before.

I don't think this is actually a good idea. What it is is a foot-gun,
because if anyone adds code there that wants to access the special area
of that particular page, it'll do the wrong thing, unless they remember
to put back the assignment of "opaque". The sequence of BufferGetPage()
and PageGetSpecialPointer() is a very standard switch-our-attention-
to-another-page locution in nbtree and other index AMs.

Any optimizing compiler will delete the dead store, we do not have
to do it by hand.

Let me put it this way: if we had the BufferGetPage() and
PageGetSpecialPointer() calls wrapped up as an "access new page" macro,
would we undo that in order to make this code change? We would not.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2019-12-19 15:59:27 Re: [PATCH] Increase the maximum value track_activity_query_size
Previous Message Bruce Momjian 2019-12-19 15:41:10 Re: [PATCH] Increase the maximum value track_activity_query_size