Re: Fillfactor for GIN indexes - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Fillfactor for GIN indexes
Date
Msg-id CAB7nPqT+F9yY7KkVvMXu4SwrKa9VDzE1bS2CHNZfMvEhBQESZQ@mail.gmail.com
Whole thread Raw
In response to Re: Fillfactor for GIN indexes  (Alexander Korotkov <aekorotkov@gmail.com>)
Responses Re: Fillfactor for GIN indexes
List pgsql-hackers


On Fri, Jul 10, 2015 at 7:13 PM, Alexander Korotkov <aekorotkov@gmail.com> wrote:
On Fri, Jul 10, 2015 at 4:54 AM, Michael Paquier <michael.paquier@gmail.com> wrote:


On Thu, Jul 9, 2015 at 10:33 PM, Alexander Korotkov wrote:
> [...]

+     /* Caclculate max data size on page according to fillfactor */
s/Caclculate/Calculate

When creating a simple gin index, I am seeing that GinGetMaxDataSize gets called with ginEntryInsert:
  * frame #0: 0x000000010a49d72e postgres`GinGetMaxDataSize(index=0x0000000114168378, isBuild='\x01') + 14 at gindatapage.c:436
    frame #1: 0x000000010a49ecbe postgres`createPostingTree(index=0x0000000114168378, items=0x0000000114a9c038, nitems=35772, buildStats=0x00007fff55787350) + 302 at gindatapage.c:1754
    frame #2: 0x000000010a493423 postgres`buildFreshLeafTuple(ginstate=0x00007fff55784d90, attnum=1, key=2105441, category='\0', items=0x0000000114a9c038, nitem=35772, buildStats=0x00007fff55787350) + 339 at gininsert.c:158
    frame #3: 0x000000010a492f84 postgres`ginEntryInsert(ginstate=0x00007fff55784d90, attnum=1, key=2105441, category='\0', items=0x0000000114a9c038, nitem=35772, buildStats=0x00007fff55787350) + 916 at gininsert.c:228

And as far as I can see GinGetMaxDataSize uses isBuild:
+ int
+ GinGetMaxDataSize(Relation index, bool isBuild)
+ {
+     int fillfactor, result;
+
+     /* Grab option value which affects only index build */
+     if (!isBuild)
However isBuild is not set in this code path, see http://www.postgresql.org/message-id/CAB7nPqSc4VQ9mHKqm_YvAfcTEhO-iUY8SKbXYdnMGnAi1XnPaw@mail.gmail.com where I reported the problem. So this code is somewhat broken, no? I am attaching to this email the patch in question that should be applied on top of Alexander's latest version.

I didn't get what is problem. Was this stacktrace during index build? If so, GinGetMaxDataSize really should get isBuild='\x01'. It is set by following line

maxdatasize = GinGetMaxDataSize(index, buildStats ? true: false);

Yeah, I just find confusing that we actually rely on the fact that buildStats is NULL or not here instead of passing directly the isBuild flag of GinBtree for example. But that's a point of detail..
--
Michael

pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: ctidscan as an example of custom-scan (Re: [v9.5] Custom Plan API)
Next
From: David Rowley
Date:
Subject: Re: Minor code improvements to create_foreignscan_plan/ExecInitForeignScan