Re: Configurable Penalty Costs for Levenshtein

Hi,

Following patch implements configurable penalty costs for levenshtein
distance metric in fuzzystrmatch contrib module.

It doesn't introduce any compatibility issues. Just implements

levenshtein(text,text,int,int,int)

function on top of fuzzystrmatch.c:levenshtein_internal(). At the
same time, levenshtein(text,text) becomes just a wrapper for
levenshtein(text,text,1,1,1) call.

BTW, in current CVS tip

if (cols/rows == 0) ...

checks in fuzzyztrmatch.c:levenshtein() never fail because of

cols/rows = strlen(...) + 1;

FYI.

Regards.

Volkan YAZICI <yazicivo(at)ttnet(dot)net(dot)tr> writes:
> Following patch implements configurable penalty costs for levenshtein
> distance metric in fuzzystrmatch contrib module.

Is there a problem with the patch? Would anybody mind helping me to
figure out the reason of this lack of interest, after 15 days.

Regards.

Volkan YAZICI wrote:
> Volkan YAZICI <yazicivo(at)ttnet(dot)net(dot)tr> writes:
> > Following patch implements configurable penalty costs for levenshtein
> > distance metric in fuzzystrmatch contrib module.
>
> Is there a problem with the patch? Would anybody mind helping me to
> figure out the reason of this lack of interest, after 15 days.

I will review it in the next few days. Sorry. I am just doing a
cleanup now.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Volkan YAZICI <yazicivo(at)ttnet(dot)net(dot)tr> writes:
> Volkan YAZICI <yazicivo(at)ttnet(dot)net(dot)tr> writes:
>> Following patch implements configurable penalty costs for levenshtein
>> distance metric in fuzzystrmatch contrib module.

> Is there a problem with the patch? Would anybody mind helping me to
> figure out the reason of this lack of interest, after 15 days.

The reason is the project schedule: we're trying to get 8.3 out the
door. No one is likely to pay any attention to new-feature patches
until after 8.4 development begins.

regards, tom lane

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Volkan YAZICI wrote:
> Hi,
>
> Following patch implements configurable penalty costs for levenshtein
> distance metric in fuzzystrmatch contrib module.
>
> It doesn't introduce any compatibility issues. Just implements
>
> levenshtein(text,text,int,int,int)
>
> function on top of fuzzystrmatch.c:levenshtein_internal(). At the
> same time, levenshtein(text,text) becomes just a wrapper for
> levenshtein(text,text,1,1,1) call.
>
> BTW, in current CVS tip
>
> if (cols/rows == 0) ...
>
> checks in fuzzyztrmatch.c:levenshtein() never fail because of
>
> cols/rows = strlen(...) + 1;
>
> FYI.
>
>
> Regards.

Content-Description: Configurable Penalty Costs for Levenshtein

>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: You can help support the PostgreSQL project by donating at
>
> http://www.postgresql.org/about/donate

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

I noticed a small typo in the patch.

prev = palloc((m + n) * sizeof(char));

line should look like

prev = palloc(2 * m * sizeof(char));

instead.

Regards.

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Volkan YAZICI wrote:
> Hi,
>
> I noticed a small typo in the patch.
>
> prev = palloc((m + n) * sizeof(char));
>
> line should look like
>
> prev = palloc(2 * m * sizeof(char));
>
> instead.
>
>
> Regards.
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Volkan YAZICI <yazicivo(at)ttnet(dot)net(dot)tr> writes:
> I noticed a small typo in the patch.
> prev = palloc((m + n) * sizeof(char));
> line should look like
> prev = palloc(2 * m * sizeof(char));
> instead.

If that's wrong, aren't the comments and the length restriction limit
also wrong?

regards, tom lane

Because of lack of reply from the author, this has been saved for the
next commit-fest:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Tom Lane wrote:
> Volkan YAZICI <yazicivo(at)ttnet(dot)net(dot)tr> writes:
> > I noticed a small typo in the patch.
> > prev = palloc((m + n) * sizeof(char));
> > line should look like
> > prev = palloc(2 * m * sizeof(char));
> > instead.
>
> If that's wrong, aren't the comments and the length restriction limit
> also wrong?
>
> regards, tom lane
>
> --
> Sent via pgsql-patches mailing list (pgsql-patches(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-patches

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

Sorry for the delay, but the reply of Tom didn't reach me. I've modified
the patch according to Tom's comments. I hope I am not too late.

Regards.

Volkan YAZICI wrote:
> Hi,
>
> Sorry for the delay, but the reply of Tom didn't reach me. I've modified
> the patch according to Tom's comments. I hope I am not too late.

OK, great. I will re-add it to the current queue and add this email as
well.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Volkan YAZICI <yazicivo(at)ttmail(dot)com> writes:
> Sorry for the delay, but the reply of Tom didn't reach me. I've modified
> the patch according to Tom's comments. I hope I am not too late.

Applied after considerable revision. This patch:

* introduced a memory stomp that was not there before (I strongly
recommend testing C code in an --enable-cassert build)
* added a user-visible feature without documenting it
* undid a conflicting patch that had been applied since your first version
* removed a number of useful comments from the code

I cleaned it up and applied anyway, but generally we expect a higher
quality standard for patches that are claimed to be ready to apply.

regards, tom lane

On Thu, 03 Apr 2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> Volkan YAZICI <yazicivo(at)ttmail(dot)com> writes:
>> Sorry for the delay, but the reply of Tom didn't reach me. I've modified
>> the patch according to Tom's comments. I hope I am not too late.
>
> Applied after considerable revision. This patch:
>
> * introduced a memory stomp that was not there before (I strongly
> recommend testing C code in an --enable-cassert build)
> * added a user-visible feature without documenting it
> * undid a conflicting patch that had been applied since your first version
> * removed a number of useful comments from the code
>
> I cleaned it up and applied anyway, but generally we expect a higher
> quality standard for patches that are claimed to be ready to apply.

Thanks so much for your kindness. Please don't hesistate to reject the
patch next time by dropping me an email with the above lines mentioning
about your considerations, and I'll happily fix it at my best and resend
it. I don't want to interrupt your work with such trivial stuff.

Regards.