Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2015-03-12 Thread Robert Haas
On Tue, Mar 10, 2015 at 4:03 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Mar 10, 2015 at 11:28 AM, Robert Haas robertmh...@gmail.com wrote: I'm prepared to commit this version if nobody finds a problem with it. It passes the additional regression tests you wrote. Looks good to me.

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2015-03-10 Thread Robert Haas
On Mon, Dec 22, 2014 at 7:34 PM, Peter Geoghegan p...@heroku.com wrote: On Mon, Dec 22, 2014 at 5:50 AM, Robert Haas robertmh...@gmail.com wrote: Looking over the latest patch, I think we could simplify the code so that you don't need multiple FuzzyAttrMatchState objects. Instead of creating

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2015-03-10 Thread Peter Geoghegan
On Tue, Mar 10, 2015 at 11:28 AM, Robert Haas robertmh...@gmail.com wrote: I'm prepared to commit this version if nobody finds a problem with it. It passes the additional regression tests you wrote. Looks good to me. Thanks. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2015-02-12 Thread Michael Paquier
On Tue, Dec 23, 2014 at 9:43 AM, Peter Geoghegan p...@heroku.com wrote: On Mon, Dec 22, 2014 at 4:34 PM, Peter Geoghegan p...@heroku.com wrote: To put it another way, creating a separate object obfuscates scanRTEForColumn(), since it's the only client of updateFuzzyAttrMatchState().

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-22 Thread Robert Haas
On Sat, Dec 20, 2014 at 7:30 PM, Peter Geoghegan p...@heroku.com wrote: On Sat, Dec 20, 2014 at 3:17 PM, Peter Geoghegan p...@heroku.com wrote: Attached patch implements this scheme. I had another thought: NAMEDATALEN + 1 is a better representation of infinity for matching purposes than

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-22 Thread Peter Geoghegan
On Mon, Dec 22, 2014 at 5:50 AM, Robert Haas robertmh...@gmail.com wrote: Looking over the latest patch, I think we could simplify the code so that you don't need multiple FuzzyAttrMatchState objects. Instead of creating a separate one for each RTE and then merging them, just have one. When

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-22 Thread Peter Geoghegan
On Mon, Dec 22, 2014 at 4:34 PM, Peter Geoghegan p...@heroku.com wrote: To put it another way, creating a separate object obfuscates scanRTEForColumn(), since it's the only client of updateFuzzyAttrMatchState(). Excuse me. I mean *not* creating a separate object -- having a unified state

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-20 Thread Peter Geoghegan
On Tue, Dec 16, 2014 at 12:18 PM, Stephen Frost sfr...@snowman.net wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: So my two cents is that when considering a qualified name, this patch should take levenshtein distance across the two components equally. There's no good reason to suppose that

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-20 Thread Peter Geoghegan
On Sat, Dec 20, 2014 at 3:17 PM, Peter Geoghegan p...@heroku.com wrote: Attached patch implements this scheme. I had another thought: NAMEDATALEN + 1 is a better representation of infinity for matching purposes than INT_MAX. I probably should have made that change, too. It would then not have

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-16 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote: So my two cents is that when considering a qualified name, this patch should take levenshtein distance across the two components equally. There's no good reason to suppose that typos will attack one name component more (nor less) than the other. Agreed

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-15 Thread Robert Haas
On Sun, Dec 14, 2014 at 8:24 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Dec 9, 2014 at 2:52 AM, Peter Geoghegan p...@heroku.com wrote: On Mon, Dec 8, 2014 at 9:43 AM, Peter Geoghegan p...@heroku.com wrote: I think it's very possible that the wrong alias may be provided by the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-15 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Sun, Dec 14, 2014 at 8:24 PM, Michael Paquier michael.paqu...@gmail.com wrote: Moving this patch to CF 2014-12 as work is still going on, note that it is currently marked with Robert as reviewer and that its current status is Needs review. The

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-14 Thread Michael Paquier
On Tue, Dec 9, 2014 at 2:52 AM, Peter Geoghegan p...@heroku.com wrote: On Mon, Dec 8, 2014 at 9:43 AM, Peter Geoghegan p...@heroku.com wrote: I think it's very possible that the wrong alias may be provided by the user, and that we should consider that when providing a hint. Note that the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-08 Thread Robert Haas
On Fri, Dec 5, 2014 at 3:45 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Dec 5, 2014 at 12:33 PM, Robert Haas robertmh...@gmail.com wrote: Well, if an alias is used, and you refer to an attribute using a non-alias name (i.e. the original table name), then you'll already get an error

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-08 Thread Peter Geoghegan
On Mon, Dec 8, 2014 at 9:31 AM, Robert Haas robertmh...@gmail.com wrote: Just that that's the case in which it seems useful to give a hint. I think it's very possible that the wrong alias may be provided by the user, and that we should consider that when providing a hint. Besides, considering

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-08 Thread Peter Geoghegan
On Mon, Dec 8, 2014 at 9:43 AM, Peter Geoghegan p...@heroku.com wrote: I think it's very possible that the wrong alias may be provided by the user, and that we should consider that when providing a hint. Note that the existing mechanism (the mechanism that I'm trying to improve) only ever shows

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-05 Thread Robert Haas
On Wed, Dec 3, 2014 at 9:21 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Dec 2, 2014 at 1:11 PM, Robert Haas robertmh...@gmail.com wrote: Basically, the case in which I think it's helpful to issue a suggestion here is when the user has used the table name rather than the alias name. I

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-05 Thread Peter Geoghegan
On Fri, Dec 5, 2014 at 12:33 PM, Robert Haas robertmh...@gmail.com wrote: Well, if an alias is used, and you refer to an attribute using a non-alias name (i.e. the original table name), then you'll already get an error suggesting that the alias be used instead -- of course, that's nothing new.

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-03 Thread Peter Geoghegan
On Tue, Dec 2, 2014 at 1:11 PM, Robert Haas robertmh...@gmail.com wrote: Basically, the case in which I think it's helpful to issue a suggestion here is when the user has used the table name rather than the alias name. I wonder if it's worth checking for that case specifically, in lieu of

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-12-02 Thread Robert Haas
On Tue, Nov 25, 2014 at 7:13 PM, Peter Geoghegan p...@heroku.com wrote: Alright, so let me summarize what I think are the next steps in working towards getting this patch committed. I should produce a new revision which: * Uses default costings. * Applies a generic final quality check that

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-30 Thread Peter Geoghegan
On Tue, Nov 25, 2014 at 4:13 PM, Peter Geoghegan p...@heroku.com wrote: If I don't hear anything in the next day or two, I'll more or less preserve aliases-related aspects of the patch. FYI, I didn't go ahead and work on this, because I thought that the thanksgiving holiday in the US probably

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Robert Haas
On Wed, Nov 19, 2014 at 2:33 PM, Peter Geoghegan p...@heroku.com wrote: I don't think that's the case. Other RTEs are penalized for having non-matching aliases here. The point I made did not, as far as I can see, have anything to do with non-matching aliases; it could arise with just a single

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: ... In other words, I think there's value in trying to clue somebody in when they've made a typo, but not when they've made a think-o. We won't be able to do the latter accurately enough to make it more useful than annoying. FWIW, I concur with

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Peter Geoghegan
On Thu, Nov 20, 2014 at 8:05 AM, Tom Lane t...@sss.pgh.pa.us wrote: I'm not particularly convinced that the f1 - f2 example is a useful behavior, and I'm downright horrified by the qty - quantity case. If the hint mechanism thinks the latter two are close enough together to suggest, it's going

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Peter Geoghegan
On Thu, Nov 20, 2014 at 7:32 AM, Robert Haas robertmh...@gmail.com wrote: In general, I think the cost of a bad suggestion is much lower than the benefit of a good one. You seem to be suggesting that they're equal. Or that they're equally likely in an organic situation. In my estimation, this

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Robert Haas
On Thu, Nov 20, 2014 at 1:30 PM, Peter Geoghegan p...@heroku.com wrote: I mean the suggestion of raising the cost threshold more gradually, not as a step function of the number of characters in the string [1] where it's either over 6 characters and must pass the 50% test, or isn't and has no

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Peter Geoghegan
On Thu, Nov 20, 2014 at 11:03 AM, Robert Haas robertmh...@gmail.com wrote: That does seem to give better results, but it still seems awfully complicated. If we just used Levenshtein with all-default cost factors and a distance cap equal to Max(strlen(what_user_typed), strlen(candidate_match),

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Robert Haas
On Thu, Nov 20, 2014 at 3:00 PM, Peter Geoghegan p...@heroku.com wrote: On Thu, Nov 20, 2014 at 11:03 AM, Robert Haas robertmh...@gmail.com wrote: That does seem to give better results, but it still seems awfully complicated. If we just used Levenshtein with all-default cost factors and a

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Peter Geoghegan
On Thu, Nov 20, 2014 at 12:14 PM, Robert Haas robertmh...@gmail.com wrote: Seems fine to me. If you typed relid rather than indexrelid or key rather than indkey, that's a thinko, not a typo. ikey for indkey could plausible be a typo, though you'd have to be having a fairly bad day at the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Andres Freund
On 2014-11-20 12:00:51 -0800, Peter Geoghegan wrote: In more concrete terms, this gets no suggestion: postgres=# select key from pg_index; ERROR: 42703: column key does not exist LINE 1: select key from pg_index; ^ I don't think that's a bad thing. Yes, for a human those

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Tom Lane
Peter Geoghegan p...@heroku.com writes: On Thu, Nov 20, 2014 at 11:03 AM, Robert Haas robertmh...@gmail.com wrote: That does seem to give better results, but it still seems awfully complicated. If we just used Levenshtein with all-default cost factors and a distance cap equal to

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Peter Geoghegan
On Thu, Nov 20, 2014 at 12:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: I do not have a problem with deciding that that is not a regression; in fact, not giving that hint seems like a good conservative behavior here. By your logic, we should also be prepared to suggest

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Robert Haas
On Thu, Nov 20, 2014 at 3:20 PM, Peter Geoghegan p...@heroku.com wrote: On Thu, Nov 20, 2014 at 12:14 PM, Robert Haas robertmh...@gmail.com wrote: Seems fine to me. If you typed relid rather than indexrelid or key rather than indkey, that's a thinko, not a typo. ikey for indkey could

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-20 Thread Peter Geoghegan
On Thu, Nov 20, 2014 at 12:50 PM, Robert Haas robertmh...@gmail.com wrote: I've got a few +1s, too, if you notice. Then maybe I spoke too soon. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Robert Haas
On Tue, Nov 18, 2014 at 8:03 PM, Tom Lane t...@sss.pgh.pa.us wrote: Peter Geoghegan p...@heroku.com writes: On Tue, Nov 18, 2014 at 3:29 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 17, 2014 at 3:04 PM, Peter Geoghegan p...@heroku.com wrote: postgres=# select qty from orderlines ;

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 5:43 AM, Robert Haas robertmh...@gmail.com wrote: I think we would be well-advised not to start inventing our own approximate matching algorithm. Peter's suggestion boils down to a guess that the default cost parameters for Levenshtein suck, and your suggestion boils

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Robert Haas
On Wed, Nov 19, 2014 at 12:33 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Nov 19, 2014 at 5:43 AM, Robert Haas robertmh...@gmail.com wrote: I think we would be well-advised not to start inventing our own approximate matching algorithm. Peter's suggestion boils down to a guess that the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 9:52 AM, Robert Haas robertmh...@gmail.com wrote: If you agree, then I'm not being clear enough. I don't think think that tinkering with the Levenshtein cost factors is a good idea, and I think it's unhelpful to suggest something when the suggestion and the original

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 10:22 AM, Peter Geoghegan p...@heroku.com wrote: Those are all very terse strings. What you're overlooking is what is broken by using straight Levenshtein distance, which includes things in the regression test that are reasonable and helpful. As I mentioned before,

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 10:33 AM, Peter Geoghegan p...@heroku.com wrote: there is no absolute quality requirement for strings of 6 or fewer requirements. I meant 6 or fewer *characters*, obviously. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 10:33 AM, Peter Geoghegan p...@heroku.com wrote: Maybe you'd prefer if there was a more gradual ramp-up to requiring a distance of no greater than 50% of the string size (normalized to take account of my non-default costings) I made this modification: diff --git

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Robert Haas
On Wed, Nov 19, 2014 at 1:22 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Nov 19, 2014 at 9:52 AM, Robert Haas robertmh...@gmail.com wrote: If you agree, then I'm not being clear enough. I don't think think that tinkering with the Levenshtein cost factors is a good idea, and I think

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 11:13 AM, Robert Haas robertmh...@gmail.com wrote: Apparently what they're doing is charging 0 for a transposition (which we don't have as a separate concept), 2 for a substitution, 1 for an insertion, and 3 for a deletion, with the constraint that anything with a total

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 11:13 AM, Robert Haas robertmh...@gmail.com wrote: That's precisely the time I think it's *most* important. In a very long string, the threshold should be LESS than 50%. My original proposal was no more than 3 characters of difference, but in any event not more than

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Alvaro Herrera
Robert Haas wrote: And the underlying Levenshtein implementation is here: https://github.com/git/git/blob/398dd4bd039680ba98497fbedffa415a43583c16/levenshtein.c Apparently what they're doing is charging 0 for a transposition (which we don't have as a separate concept), 2 for a

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 11:34 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: 0 for a transposition, wow. Again, they're optimizing for short strings (git commands) only. There just isn't that many transposition errors possible with a 4 character string. -- Peter Geoghegan -- Sent via

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Alvaro Herrera
Peter Geoghegan wrote: On Wed, Nov 19, 2014 at 11:34 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: 0 for a transposition, wow. Again, they're optimizing for short strings (git commands) only. There just isn't that many transposition errors possible with a 4 character string. If

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Peter Geoghegan
On Wed, Nov 19, 2014 at 11:54 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Peter Geoghegan wrote: On Wed, Nov 19, 2014 at 11:34 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: 0 for a transposition, wow. Again, they're optimizing for short strings (git commands) only. There just

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-19 Thread Alvaro Herrera
Peter Geoghegan wrote: On Wed, Nov 19, 2014 at 11:54 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Peter Geoghegan wrote: On Wed, Nov 19, 2014 at 11:34 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: 0 for a transposition, wow. Again, they're optimizing for short strings

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-18 Thread Robert Haas
On Mon, Nov 17, 2014 at 3:04 PM, Peter Geoghegan p...@heroku.com wrote: postgres=# select qty from orderlines ; ERROR: 42703: column qty does not exist LINE 1: select qty from orderlines ; ^ HINT: Perhaps you meant to reference the column orderlines.quantity. I don't buy

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-18 Thread Peter Geoghegan
On Tue, Nov 18, 2014 at 3:29 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 17, 2014 at 3:04 PM, Peter Geoghegan p...@heroku.com wrote: postgres=# select qty from orderlines ; ERROR: 42703: column qty does not exist LINE 1: select qty from orderlines ; ^ HINT:

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-18 Thread Tom Lane
Peter Geoghegan p...@heroku.com writes: On Tue, Nov 18, 2014 at 3:29 PM, Robert Haas robertmh...@gmail.com wrote: On Mon, Nov 17, 2014 at 3:04 PM, Peter Geoghegan p...@heroku.com wrote: postgres=# select qty from orderlines ; ERROR: 42703: column qty does not exist HINT: Perhaps you meant

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-17 Thread Robert Haas
On Sat, Nov 15, 2014 at 7:36 PM, Peter Geoghegan p...@heroku.com wrote: Attached patch incorporates this feedback. The only user-visible difference between this revision and the previous revision is that it's quite possible for two suggestion to originate from the same RTE (there is exactly

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-17 Thread Peter Geoghegan
On Mon, Nov 17, 2014 at 10:15 AM, Robert Haas robertmh...@gmail.com wrote: I'm grumpy about the distanceName() function. That seems too generic. If we're going to keep this as it is, I suggest something like computeRTEColumnDistance(). But see below. Fair point. On a related note, I'm also

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-15 Thread Peter Geoghegan
On Wed, Nov 12, 2014 at 12:59 PM, Robert Haas robertmh...@gmail.com wrote: On that topic, I think there's unanimous consensus against the design where equally-distant matches are treated differently based on whether they are in the same RTE or different RTEs. I think you need to change that

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-13 Thread Robert Haas
On Wed, Nov 12, 2014 at 10:42 PM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Nov 11, 2014 at 3:52 PM, Peter Geoghegan p...@heroku.com wrote: I'm pretty puzzled by this. Other than our agree to disagree and defer to committer position on the question of whether or not more than

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-13 Thread Peter Geoghegan
On Thu, Nov 13, 2014 at 8:57 AM, Robert Haas robertmh...@gmail.com wrote: My point is: I am not sure I can be defined as a reviewer of this patch or take any credit in this patch review knowing that the latest version submitted is a simple rebase of the version I did my first review on. Hence,

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-13 Thread Robert Haas
On Wed, Nov 12, 2014 at 8:00 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Nov 12, 2014 at 4:54 PM, Peter Geoghegan p...@heroku.com wrote: Attached patch moves the Levenshtein distance implementation into core. Oops. Somehow managed to send a *.patch.swp file. :-) Here is the actual

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-13 Thread Peter Geoghegan
On Thu, Nov 13, 2014 at 9:36 AM, Robert Haas robertmh...@gmail.com wrote: Committed. I changed varstr_leven() to varstr_levenshtein() because abbrvs cn mk the code hrd to undstnd. And to grep. Thanks. I'll produce a revision of patch 2/2 soon. And I removed the StaticAssertStmt you added,

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-12 Thread Robert Haas
On Sun, Nov 9, 2014 at 11:48 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Nov 7, 2014 at 12:57 PM, Robert Haas robertmh...@gmail.com wrote: Based on this review from a month ago, I'm going to mark this Waiting on Author. If nobody updates the patch in a few days, I'll mark it Returned

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-12 Thread Peter Geoghegan
On Wed, Nov 12, 2014 at 12:59 PM, Robert Haas robertmh...@gmail.com wrote: I agree with your proposed approach to moving Levenshtein into core. However, I think this should be separated into two patches, one of them moving the Levenshtein functionality into core, and the other adding the new

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-12 Thread Peter Geoghegan
On Wed, Nov 12, 2014 at 1:13 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Nov 12, 2014 at 12:59 PM, Robert Haas robertmh...@gmail.com wrote: I agree with your proposed approach to moving Levenshtein into core. However, I think this should be separated into two patches, one of them moving

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-12 Thread Peter Geoghegan
On Wed, Nov 12, 2014 at 4:54 PM, Peter Geoghegan p...@heroku.com wrote: Attached patch moves the Levenshtein distance implementation into core. Oops. Somehow managed to send a *.patch.swp file. :-) Here is the actual patch. -- Peter Geoghegan From b7df918f1a52107637600f3b22d1cff18bd07ae1 Mon

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-10 Thread Michael Paquier
On Mon, Nov 10, 2014 at 1:48 PM, Peter Geoghegan p...@heroku.com wrote: Reminder: I maintain a slight preference for only offering one suggestion per relation RTE, which is what this revision does (so no change there). If a committer who picks this up wants me to alter that, I don't mind

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-10 Thread Peter Geoghegan
On Mon, Nov 10, 2014 at 8:13 PM, Michael Paquier michael.paqu...@gmail.com wrote: Hm. The last version of this patch has not really changed since since my first review, and I have no more feedback to provide about it except what I already mentioned. I honestly don't think that this patch is

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-10 Thread Peter Geoghegan
On Mon, Nov 10, 2014 at 10:29 PM, Peter Geoghegan p...@heroku.com wrote: Why not? You've already said that you're happy to defer to whatever committer picks this up with regard to whether or not more than a single suggestion can come from an RTE. I agreed with this (i.e. I said I'd defer to

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-09 Thread Peter Geoghegan
On Fri, Nov 7, 2014 at 12:57 PM, Robert Haas robertmh...@gmail.com wrote: Based on this review from a month ago, I'm going to mark this Waiting on Author. If nobody updates the patch in a few days, I'll mark it Returned with Feedback. Thanks. Attached revision fixes the compiler warning that

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-09 Thread Michael Paquier
On Mon, Nov 10, 2014 at 1:48 PM, Peter Geoghegan p...@heroku.com wrote: On Fri, Nov 7, 2014 at 12:57 PM, Robert Haas robertmh...@gmail.com wrote: Based on this review from a month ago, I'm going to mark this Waiting on Author. If nobody updates the patch in a few days, I'll mark it

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-09 Thread Peter Geoghegan
On Sun, Nov 9, 2014 at 8:56 PM, Michael Paquier michael.paqu...@gmail.com wrote: FWIW, I still find this bit of code that this patch adds in varlena.c ugly: +#include levenshtein.c +#define LEVENSHTEIN_LESS_EQUAL +#include levenshtein.c +#undef LEVENSHTEIN_LESS_EQUAL Okay, but this is the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-11-07 Thread Robert Haas
On Mon, Oct 6, 2014 at 3:09 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 07/18/2014 10:47 AM, Michael Paquier wrote: On Fri, Jul 18, 2014 at 3:54 AM, Peter Geoghegan p...@heroku.com wrote: I am not opposed to moving the contrib code into core in the manner that you oppose. I

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-10-06 Thread Heikki Linnakangas
On 07/18/2014 10:47 AM, Michael Paquier wrote: On Fri, Jul 18, 2014 at 3:54 AM, Peter Geoghegan p...@heroku.com wrote: I am not opposed to moving the contrib code into core in the manner that you oppose. I don't feel strongly either way. I noticed in passing that your revision says this

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Robert Haas
On Thu, Jul 17, 2014 at 9:34 AM, Michael Paquier michael.paqu...@gmail.com wrote: Patch 1 does a couple of things: - fuzzystrmatch is dumped to 1.1, as Levenshtein functions are not part of it anymore, and moved to core. - Removal of the LESS_EQUAL flag that made the original submission patch

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: There are several possible methods of doing that, but I think the best one is just to leave the SQL-callable C functions in fuzzystrmatch and move only the underlying code that supports into core. I hadn't been paying close attention to this thread,

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Peter Geoghegan
On Wed, Jul 23, 2014 at 8:57 AM, Robert Haas robertmh...@gmail.com wrote: There are several possible methods of doing that, but I think the best one is just to leave the SQL-callable C functions in fuzzystrmatch and move only the underlying code that supports into core. For some reason I

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Alvaro Herrera
Peter Geoghegan wrote: For some reason I thought that that was what Michael was proposing - a more comprehensive move of code into core than the structuring that I proposed. I actually thought about a Levenshtein distance operator at one point months ago, before I entirely gave up on that.

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Peter Geoghegan
On Wed, Jul 23, 2014 at 1:10 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I had two thoughts: 1. Should we consider making levenshtein available to frontend programs as well as backend? I don't think so. Why would that be useful? 2. Would it provide better matching to use

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Michael Paquier
On Thu, Jul 24, 2014 at 1:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: There are several possible methods of doing that, but I think the best one is just to leave the SQL-callable C functions in fuzzystrmatch and move only the underlying code that

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-23 Thread Alvaro Herrera
Peter Geoghegan wrote: Maybe that would be marginally better than classic Levenshtein distance, but I doubt it would pay for itself. It's just more code to maintain. Are we really expecting to not get the best possible suggestion due to some number of transposition errors very frequently?

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-17 Thread Peter Geoghegan
I am not opposed to moving the contrib code into core in the manner that you oppose. I don't feel strongly either way. I noticed in passing that your revision says this *within* levenshtein.c: + * Guaranteed to work with Name datatype's cstrings. + * For full details see levenshtein.c. On Thu,

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Michael Paquier
On Wed, Jul 9, 2014 at 5:56 AM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jul 8, 2014 at 1:55 PM, Peter Geoghegan p...@heroku.com wrote: I was worried about the common case where a column name is misspelled that would otherwise be ambiguous, which is why that shows a HINT while the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Michael Paquier
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

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Peter Geoghegan
On Tue, Jul 8, 2014 at 11:10 PM, Michael Paquier michael.paqu...@gmail.com wrote: Showing up to 2 hints is fine as it does not pollute the error output with perhaps unnecessary messages. That's even more protective than for example git that prints all the equidistant candidates. However I can't

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Peter Geoghegan
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

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Alvaro Herrera
Peter Geoghegan wrote: On Tue, Jul 8, 2014 at 11:25 PM, Michael Paquier michael.paqu...@gmail.com wrote: 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

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Robert Haas
On Wed, Jul 9, 2014 at 2:10 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Wed, Jul 9, 2014 at 5:56 AM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jul 8, 2014 at 1:55 PM, Peter Geoghegan p...@heroku.com wrote: I was worried about the common case where a column name is misspelled

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Greg Stark
On Wed, Jul 9, 2014 at 7:29 AM, Peter Geoghegan p...@heroku.com wrote: Everyone is going to have an opinion on something like that. I was showing deference to the general concern about the absolute (as opposed to relative) quality of the HINTs in the event of equidistant matches by having no

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Peter Geoghegan
On Wed, Jul 9, 2014 at 8:08 AM, Greg Stark st...@mit.edu wrote: A simple rule is easier for users to understand as well as to code. I would humbly suggest the following: take all the unqualified column names, downcase them, check which ones match most closely the unmatched column. Show the top

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Peter Geoghegan
On Wed, Jul 9, 2014 at 8:06 AM, Robert Haas robertmh...@gmail.com wrote: Showing up to 2 hints is fine as it does not pollute the error output with perhaps unnecessary messages. That's even more protective than for example git that prints all the equidistant candidates. However I can't

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Alvaro Herrera
Peter Geoghegan wrote: On Wed, Jul 9, 2014 at 8:08 AM, Greg Stark st...@mit.edu wrote: A simple rule is easier for users to understand as well as to code. I would humbly suggest the following: take all the unqualified column names, downcase them, check which ones match most closely the

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Peter Geoghegan
On Wed, Jul 9, 2014 at 2:19 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: That's harder than it sounds. You need even more translatable strings for variant ereports(). Maybe it is possible to rephrase the message so that the translatable part doesn't need to concern with how many

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-09 Thread Peter Geoghegan
On Tue, Jul 8, 2014 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: 6) Sometimes no hints are returned... Even in simple cases like this one: =# create table foo (aa int, bb int); CREATE TABLE =# select ab from foo; ERROR: 42703: column ab does not exist LINE 1: select ab from

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-08 Thread Michael Paquier
On Sat, Jul 5, 2014 at 12:46 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: At 2014-07-02 15:51:08 -0700, p...@heroku.com wrote: Attached revision factors in everyone's concerns here, I think. Is anyone planning to review Peter's revised patch? I have been doing some functional tests, and

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-08 Thread Peter Geoghegan
Hi, On Tue, Jul 8, 2014 at 6:58 AM, Michael Paquier michael.paqu...@gmail.com wrote: 2) Checking process goes through all the existing columns of a relation even a difference of 1 with some other column(s) has already been found. As we try to limit the number of hints returned, this seems

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-08 Thread Alvaro Herrera
Peter Geoghegan wrote: 6) Sometimes no hints are returned... Even in simple cases like this one: =# create table foo (aa int, bb int); CREATE TABLE =# select ab from foo; ERROR: 42703: column ab does not exist LINE 1: select ab from foo; ^ LOCATION:

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-08 Thread Peter Geoghegan
On Tue, Jul 8, 2014 at 1:42 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: That's because those two candidates come from a single RTE and have an equal distance -- you'd see both suggestions if you joined two tables with each candidate, assuming that each table being joined didn't

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-08 Thread Peter Geoghegan
On Tue, Jul 8, 2014 at 1:55 PM, Peter Geoghegan p...@heroku.com wrote: I was worried about the common case where a column name is misspelled that would otherwise be ambiguous, which is why that shows a HINT while the single RTE case doesn't To be clear - I mean a HINT with two suggestions

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-04 Thread Abhijit Menon-Sen
At 2014-07-02 15:51:08 -0700, p...@heroku.com wrote: Attached revision factors in everyone's concerns here, I think. Is anyone planning to review Peter's revised patch? These new measures make the coding somewhat more complex than that of the initial version, although overall the parser code

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-07-02 Thread Peter Geoghegan
On Sun, Jun 29, 2014 at 7:30 AM, Tom Lane t...@sss.pgh.pa.us wrote: Although printing all candidates seems to be what's preferred by existing systems with similar facilities, I can see the point that constructing the message in a translatable fashion might be difficult. So personally I'd be

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-06-29 Thread Abhijit Menon-Sen
So, what's the status of this patch? There's been quite a lot of discussion (though only about the approach; no formal code/usage review has yet been posted), but as far as I can tell, it just tapered off without any particular consensus. -- Abhijit -- Sent via pgsql-hackers mailing list

Re: [HACKERS] Doing better at HINTing an appropriate column within errorMissingColumn()

2014-06-29 Thread Tom Lane
Abhijit Menon-Sen a...@2ndquadrant.com writes: So, what's the status of this patch? There's been quite a lot of discussion (though only about the approach; no formal code/usage review has yet been posted), but as far as I can tell, it just tapered off without any particular consensus. AFAICT,

  1   2   >