Re: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?
От | Dilip Kumar |
---|---|
Тема | Re: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)? |
Дата | |
Msg-id | CAFiTN-vf6NbH44ipXGntJoVJi5y9mJM8dbYZaqgKunrcLVbcUg@mail.gmail.com обсуждение исходный текст |
Ответ на | PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)? (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>) |
Ответы |
Re: PageGetItemIdCareful - should we MAXALIGN sizeof(BTPageOpaqueData)?
|
Список | pgsql-hackers |
On Thu, Apr 22, 2021 at 10:40 AM Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote: > > 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? +1 for changing to MAXALIGN(sizeof(BTPageOpaqueData)). -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
В списке pgsql-hackers по дате отправления: