Re: Use correct collation in pg_trgm - Mailing list pgsql-hackers

From David Geier
Subject Re: Use correct collation in pg_trgm
Date
Msg-id 40afd8a3-d22e-437f-8eec-94462a6960d2@gmail.com
Whole thread
In response to Re: Use correct collation in pg_trgm  (solai v <solai.cdac@gmail.com>)
Responses Re: Use correct collation in pg_trgm
List pgsql-hackers
Hi Solai!

> I reviewed the v7 patch series and I tested the changes with a Turkish
> ICU collation and verified that the collation is now correctly
> propagated for both trigram lowercasing and word-boundary detection.
> For the lowercasing changes, show_trgm('ISTANBUL' COLLATE "tr-x-icu")
> now produces the same trigrams as show_trgm('ıstanbul' COLLATE
> turkish), and:
> 
> similarity('ıstanbul' COLLATE turkish,
>            'ISTANBUL' COLLATE turkish)
> 
> returns 1, whereas the unpatched behavior produced 0.5.
> 
> I also tested the word-boundary handling using: helloıtestIworldİcode,
> with both Turkish ICU and "C" collations. The generated trigrams
> differ appropriately between the two collations, confirming that the
> collation is being used while identifying word boundaries. I tested
> the index paths as well. A GIN trigram index and a GiST trigram index
> both work correctly for a query using the matching Turkish collation.
> And also tested an explicitly "C"-collated LIKE query against the
> Turkish-collated column with sequential scans disabled. The planner
> did not use the Turkish trigram index and instead showed a disabled
> sequential scan, which avoids using an index with an incompatible
> collation. Finally, I ran the complete pg_trgm installcheck suite and
> all 5 tests passed successfully. I then ran: git diff HEAD~2..HEAD
> --check, which reported one minor formatting issue:
> contrib/pg_trgm/sql/pg_trgm_collation.sql:29: new blank line at EOF.
> This appears to be only a patch formatting issue. I have not modified
> the contributor's patch for this; I am mentioning it here as a minor
> review comment.
> Other than the above minor formatting point, I did not find any
> functional issues during my testing.

Thanks for the thorough review.

I think with that it's ready for committer.
Marked accordingly in the commitfest app.

--
David Geier



pgsql-hackers by date:

Previous
From: David Geier
Date:
Subject: Re: Reducing relcache memory usage 2: shrink sizeof(RelationData)
Next
From: "zengxx"
Date:
Subject: Skip a redundant singleton GROUP BY node