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


Reply via email to