| 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: | Whole Thread | Raw Message | 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/
| 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 |