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" > > +#define LEVENSHTEIN_LESS_EQUAL > > +#include "levenshtein.c" > > +#undef LEVENSHTEIN_LESS_EQUAL > > 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. -- Michael
