Re: pg_trgm version 1.2

From: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
To: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_trgm version 1.2
Date: 2015-07-14 21:31:10
Message-ID: CAMkU=1zb=Q2gwy9+4vL4MPR=56qTksZ+H=WPGHhFh3_E9ENoHw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 7, 2015 at 6:33 AM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

>
>
> See Tom Lane's comment about downgrade scripts. I think just remove it is
> a right solution.
>

The new patch removes the downgrade path and the ability to install the old
version.

(If anyone wants an easy downgrade path for testing, they can keep using
the prior patch--no functional changes)

It also added a comment before the trigramsMatchGraph call.

I retained the palloc and the loop to promote the ternary array to a binary
array. While I also think it is tempting to get rid of that by abusing the
type system and would do it that way in my own standalone code, it seems
contrary to way the project usually does things. And I couldn't measure a
difference in performance.

....

> Let's consider '^(?!.*def).*abc' regular expression as an example. It
> matches strings which contains 'abc' and don't contains 'def'.
>
> # SELECT 'abc' ~ '^(?!.*def).*abc';
> ?column?
> ----------
> t
> (1 row)
>
> # SELECT 'def abc' ~ '^(?!.*def).*abc';
> ?column?
> ----------
> f
> (1 row)
>
> # SELECT 'abc def' ~ '^(?!.*def).*abc';
> ?column?
> ----------
> f
> (1 row)
>
> Theoretically, our trigram regex processing could support negative
> matching of 'def' trigram, i.e. trigramsMatchGraph(abc = true, def = false)
> = true but trigramsMatchGraph(abc = true, def = true) = false. Actually, it
> doesn't because trigramsMatchGraph() implements a monotonic function. I
> just think it should be stated explicitly.
>

Do you think it is likely to change to stop being monotonic and so support
the (def=GIN_TRUE) => false case?

^(?!.*def) seems like a profoundly niche situation. (Although one that I
might actually start using myself now that I know it isn't just a Perl-ism).

It doesn't make any difference to this patch, other than perhaps how to
word the comments.

Cheers,

Jeff

Attachment Content-Type Size
pg_trgm_1_2_v003.patch application/octet-stream 17.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2015-07-14 21:57:34 Re: patch : Allow toast tables to be moved to a different tablespace
Previous Message Robert Haas 2015-07-14 21:22:02 Re: ctidscan as an example of custom-scan (Re: [v9.5] Custom Plan API)