On Fri, Jul 10, 2015 at 7:13 PM, Alexander Korotkov <aekorot...@gmail.com>
wrote:
> On Fri, Jul 10, 2015 at 4:54 AM, Michael Paquier <
> michael.paqu...@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-iuy8skbxydnmgnai1xn...@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
