Re: Prefix operator for text and spgist support

From: Teodor Sigaev <teodor(at)sigaev(dot)ru>
To: Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Prefix operator for text and spgist support
Date: 2018-04-03 16:50:02
Message-ID: 03300255-cff2-b508-50f4-f00cca0a57a1@sigaev.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you, pushed with some editorization and renaming text_startswith to
starts_with

Ildus Kurbangaliev wrote:
> On Fri, 23 Mar 2018 11:45:33 +0300
> Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
>
>> On Thu, Mar 22, 2018 at 7:09 PM, Teodor Sigaev <teodor(at)sigaev(dot)ru>
>> wrote:
>>
>>> Patch looks resonable, but I see some place to improvement:
>>> spg_text_leaf_consistent() only needs to check with
>>> text_startswith() if reconstucted value came to leaf consistent is
>>> shorter than given prefix. For example, if level >= length of
>>> prefix then we guarantee that fully reconstracted is matched too.
>>> But do not miss that you may need to return value for index only
>>> scan, consult returnData field
>>>
>>> In attachment rebased and minorly edited version of your patch.
>>
>>
>> I took a look at this patch. In addition to Teodor's comments I can
>> note following.
>>
>> * This line looks strange for me.
>>
>> - if (strategy > 10)
>> + if (strategy > 10 && strategy !=
>> RTPrefixStrategyNumber)
>>
>> It's not because we added strategy != RTPrefixStrategyNumber condition
>> there.
>> It's because we already used magic number here and now have a mix of
>> magic number and macro constant in one line. Once we anyway touch
>> this place, could we get rid of magic numbers here?
>>
>> * I'm a little concern about operator name. We're going to choose @^
>> operator for
>> prefix search without any preliminary discussion. However,
>> personally I don't
>> have better ideas :)
>
> Teodor, Alexander, thanks for review. In new version I have added the
> optimization in spgist using level variable and also got rid of magic
> numbers.
>
> About the operator it's actually ^@ (not @^ :)), I thought about it and
> don't really have any idea what operator can be used instead.
>
> Attached version 5 of the patch.
>

--
Teodor Sigaev E-mail: teodor(at)sigaev(dot)ru
WWW: http://www.sigaev.ru/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Anthony Iliopoulos 2018-04-03 16:52:07 Re: PostgreSQL's handling of fsync() errors is unsafe and risks data loss at least on XFS
Previous Message Andres Freund 2018-04-03 16:43:56 Re: Optimize Arm64 crc32c implementation in Postgresql