From: | Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru> |
---|---|
To: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
Cc: | Teodor Sigaev <teodor(at)sigaev(dot)ru>, 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-03-29 11:36:34 |
Message-ID: | 20180329143634.33c53348@wp.localdomain |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
--
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Attachment | Content-Type | Size |
---|---|---|
prefix_operator_and_spgist_v5.patch | text/x-patch | 17.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Gierth | 2018-03-29 11:37:05 | Re: committing inside cursor loop |
Previous Message | Alexander Korotkov | 2018-03-29 11:32:05 | Fix src/test/subscription/t/003_constraints.pl header comment |