Re: daitch_mokotoff module

Tomas Vondra <tomas.vondra@enterprisedb.com>

From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Andrew Dunstan <andrew@dunslane.net>, Dag Lem <dag@nimrod.no>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-12-13T16:18:07Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Avoid using non-ASCII commentary in daitch_mokotoff.c.

  2. Remove some non-ASCII symbols from a comment.

  3. Remove useless dependencies in daitch_mokotoff_header.pl.

  4. Pacify perlcritic.

  5. Add support for Daitch-Mokotoff Soundex in contrib/fuzzystrmatch.

  6. Enable routine running of citext's UTF8-specific test cases.

On 12/13/21 16:05, Andrew Dunstan wrote:
> 
> On 12/13/21 09:26, Tomas Vondra wrote:
>> On 12/13/21 14:38, Dag Lem wrote:
>>> Please find attached an updated patch, with the following fixes:
>>>
>>> * Replaced remaining malloc/free with palloc/pfree.
>>> * Made "make check" pass.
>>> * Updated notes on other implementations.
>>>
>>
>> Thanks, looks interesting. A couple generic comments, based on a quick
>> code review.
>>
>> 1) Can the extension be marked as trusted, just like fuzzystrmatch?
>>
>> 2) The docs really need an explanation of what the extension is for,
>> not just a link to fuzzystrmatch. Also, a couple examples would be
>> helpful, I guess - similarly to fuzzystrmatch. The last line in the
>> docs is annoyingly long.
> 
> 
> It's not clear to me why we need a new module for this. Wouldn't it be
> better just to add the new function to fuzzystrmatch?
> 

Yeah, that's a valid point. I think we're quite conservative about 
adding more contrib modules, and adding a function to an existing one 
works around a lot of that.

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company