Re: Wrong value in metapage of GIN INDEX.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "imai(dot)yoshikazu(at)fujitsu(dot)com" <imai(dot)yoshikazu(at)fujitsu(dot)com>
Cc: "'Moon, Insung'" <tsukiwamoon(dot)pgsql(at)gmail(dot)com>, "keisuke(dot)kuroda(dot)3862(at)gmail(dot)com" <keisuke(dot)kuroda(dot)3862(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Wrong value in metapage of GIN INDEX.
Date: 2019-11-04 18:55:02
Message-ID: 2198.1572893702@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"imai(dot)yoshikazu(at)fujitsu(dot)com" <imai(dot)yoshikazu(at)fujitsu(dot)com> writes:
> Moon-san, kuroda.keisuke-san
> On Thu, Aug 29, 2019 at 8:20 AM, Moon, Insung wrote:
>> The patch is very simple.
>> Fix to increase the value of nEntries only when a non-duplicate GIN index leaf added.

> Does nentries show the number of entries in the leaf pages?
> If so, the fix seems to be correct.

I looked at this issue. The code in ginEntryInsert is not obviously wrong
by itself; it depends on what you think nEntries is supposed to count.
However, ginvacuum.c updates nEntries to the sum of PageGetMaxOffsetNumber()
across all the index's leaf pages, ie the number of surviving leaf items.

It's hard to see how ginvacuum could reverse-engineer a value that would
match what ginEntryInsert is doing, so probably we ought to define
nEntries as the number of leaf items, which seems to make the proposed
patch correct. (It could use a bit more commentary though.)

I'm inclined to apply this to HEAD only; it doesn't seem significant
enough to justify back-patching.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-11-04 19:04:40 Re: Missed check for too-many-children in bgworker spawning
Previous Message Andres Freund 2019-11-04 18:53:00 Re: Missed check for too-many-children in bgworker spawning