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.korot...@postgrespro.ru> wrote:

On Thu, Mar 22, 2018 at 7:09 PM, Teodor Sigaev <teo...@sigaev.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: teo...@sigaev.ru
                                                   WWW: http://www.sigaev.ru/

Reply via email to