On Tue, Jul 8, 2014 at 11:25 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: >> 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?
Just as before, rest_of_char_same() exists for the express purpose of being called by the two variants varstr_leven_less_equal() and varstr_leven(). Why wouldn't I copy it over too along with those two? Where do you propose to put it? Obviously the existing macro hacks (that I haven't changed) that build the two variants are not terribly pretty, but they're not arbitrary either. They reflect the fact that there is no natural way to add callbacks or something like that. If you pretended that the core code didn't have to care about one case or the other, and that contrib was somehow obligated to hook in its own handler for the !LEVENSHTEIN_LESS_EQUAL case that it now only cares about, then you'd end up with an even bigger mess. Besides, with the patch the core code is calling varstr_leven_less_equal(), which is the bigger of the two variants - it's the LEVENSHTEIN_LESS_EQUAL case, not the !LEVENSHTEIN_LESS_EQUAL case that core cares about for the purposes of building HINTs. In short, I don't know what you mean. What would that reshuffling actually look like? >> > 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 think that it's clearly not worth it, even if it is true that a minority sometimes make this mistake. Most users don't know that there are system columns. It's not even close to being worth it to bring that into this. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers