PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Peter Geoghegan <pg(at)bowt(dot)ie>
Subject: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?
Date: 2021-04-22 05:10:13
Message-ID: CALj2ACUMqTR9nErh99FbOBmzCXE9=gXNqhBiwYOhejJJS1LXqQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

In the PageGetItemIdCareful() introduced by commit a9ce839a, it seems
like we are using btree page pd_special structure BTPageOpaqueData for
error case without max aligning it.
if (ItemIdGetOffset(itemid) + ItemIdGetLength(itemid) >
BLCKSZ - sizeof(BTPageOpaqueData))
ereport(ERROR,

I'm not sure if it is intentional. ISTM that this was actually not a
problem because the BTPageOpaqueData already has all-aligned(???)
members (3 uint32, 2 uint16). But it might be a problem if we add
unaligned bytes. PageInit always max aligns this structure, when we
initialize the btree page in _bt_pageini and in all other places we
max align it before use. Since this is an error throwing path, I think
we should max align it just to be on the safer side. While on it, I
think we can also replace BLCKSZ with PageGetPageSize(page).

Attaching a small patch. Thoughts?

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
v1-0001-MAXALIGN-sizeof-BTPageOpaqueData-in-PageGetItemId.patch application/octet-stream 1.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ian Zagorskikh 2021-04-22 05:46:52 Procedure of modification of autoconf scripts
Previous Message Masahiko Sawada 2021-04-22 05:09:15 Re: Replication slot stats misgivings