Re: Prefix operator for text and spgist support
Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
From: Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
To: Alexander Korotkov <a.korotkov@postgrespro.ru>
Cc: Teodor Sigaev <teodor@sigaev.ru>, Arthur Zakirov
<a.zakirov@postgrespro.ru>, PostgreSQL Hackers
<pgsql-hackers@lists.postgresql.org>
Date: 2018-03-29T11:36:34Z
Lists: pgsql-hackers
Attachments
- prefix_operator_and_spgist_v5.patch (text/x-patch) patch v5
On Fri, 23 Mar 2018 11:45:33 +0300 Alexander Korotkov <a.korotkov@postgrespro.ru> wrote: > On Thu, Mar 22, 2018 at 7:09 PM, Teodor Sigaev <teodor@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. -- --- Ildus Kurbangaliev Postgres Professional: http://www.postgrespro.com Russian Postgres Company
Commits
-
Add prefix operator for TEXT type.
- 710d90da1fd8 11.0 landed