On Wed, Jul 9, 2014 at 1:49 AM, Peter Geoghegan <p...@heroku.com> wrote:

> > 4) This is not nice, could it be possible to remove the stuff from
> varlena.c?
> > +/* Expand each Levenshtein distance variant */
> > +#include "levenshtein.c"
> > +#include "levenshtein.c"
> > Part of the same comment: only varstr_leven_less_equal is used to
> > calculate the distance, should we really move varstr_leven to core?
> > This clearly needs to be reworked as not just a copy-paste of the
> > things in fuzzystrmatch.
> > The flag LEVENSHTEIN_LESS_EQUAL should be let within fuzzystrmatch I
> think.
> So there'd be one variant within core and one within
> contrib/fuzzystrmatch? I don't think that's an improvement.
No. The main difference between varstr_leven_less_equal and varstr_leven is
the use of the extra argument max_d in the former. My argument here is
instead of blindly cut-pasting into core the code you are interested in to
evaluate the string distances, is to refactor it to have a unique function,
and to let the business with LEVENSHTEIN_LESS_EQUAL within
contrib/fuzzystrmatch. This will require some reshuffling of the distance
function, but by looking at this patch I am getting the feeling that this
is necessary, and should even be split into a first patch for fuzzystrmatch
that would facilitate its integration into core.
Also why is rest_of_char_same within varlena.c?

> 5) Do we want hints on system columns as well?
> I think it's obvious that the answer must be no. That's going to
> frequently result in suggestions of columns that users will complain
> aren't even there. If you know about the system columns, you can just
> get it right. They're supposed to be hidden for most purposes.
This may sound ridiculous, but I have already found myself mistyping ctid
by tid and cid while working on patches and modules that played with page
format, and needing a couple of minutes to understand what was going on
(bad morning). I would have welcomed such hints in those cases.

Reply via email to