Re: [HACKERS] patch: autocomplete for functions
On Sun, Feb 19, 2012 at 12:10 PM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I found so this extremely simple patch should be useful. It helps for pattern SELECT fx(); There was thread about it. Hi Pavel, I signed up to be reviewer for this patch, and finally got around to taking a look. This thread, and the thread about Peter's earlier patch along the same lines have gotten a bit muddled, so allow me some recap for my own sanity. The first point to be addressed, is that Pavel's patch is basically a subset of Peter's earlier[1] patch. Pavel's patch autocompletes SELECT TAB with possible function names. Peter's patch autocompletes both possible column names and possible function names. So, which version, if any, would we want? Both Tom[2] and depesz[3] have asked for column names to be autocompleted if we're going to go down this road, and I personally would find completion of column names handy. Others [5][6] have asked for function names to be (also?) autocompleted here, so it seems reasonable that we'd want to include both. I counted two general objections to Peter's patch in these threads, namely: 1.) Complaints about the tab-completion not covering all cases, possibly leading to user frustration at our inconsistency. [2] [4] 2.) Concerns that the tab-completion wouldn't be useful given how many results we'd have from system columns and functions [7] I do agree these are two legitimate concerns. However, for #1, our tab-completion is and has always been incomplete. I take the basic goal of the tab-completion machinery to be offer a suggestion when we're pretty sure we know what the user wants, otherwise stay quiet. There are all sorts of places where our reliance on inspecting back only a few words into the current line and not having a true command parser prevents us from being able to make tab-completion guesses, but that's been accepted so far, and I don't think it's fair to raise the bar for this patch. Re: concern #2, Tom complained about there being a bunch of possible column and function completions in the regression test database. That may be true, but if you look at this slightly-modified version of the query Peter's patch proposes: SELECT substring(name, 1, 3) AS sub, COUNT(*) FROM ( SELECT attname FROM pg_attribute WHERE NOT attisdropped UNION SELECT proname || '(' FROM pg_proc p WHERE pg_catalog.pg_function_is_visible(p.oid)) t (name) GROUP BY sub ORDER BY COUNT(*) DESC; I count only 384 distinct 3-length prefixes in an empty database, thanks to many built-in columns and functions sharing the same prefix (e.g. int or pg_). Obviously, there is plenty of room left for 3-length prefixes, out of the 27^3 or more possibilities. In some casual mucking around in my own databases, I found the column-completion rather useful, and typing 3 characters of a column-name to be sufficient to give matches which were at least non-builtin attributes, and often a single unique match. There were some ideas down-thread about how we might filter out some of the noise in these completions, which would be interesting. I'd be happy with the patch as-is though, modulo the attisdropped and pg_function_is_visible() tweaks to the query. Josh [1] http://archives.postgresql.org/message-id/1328820579.11241.4.camel%40vanquo.pezone.net [2] http://archives.postgresql.org/message-id/7745.1328855069%40sss.pgh.pa.us [3] http://www.depesz.com/2011/07/08/wish-list-for-psql/ [4] http://archives.postgresql.org/message-id/13612.1328887227%40sss.pgh.pa.us [5] http://archives.postgresql.org/message-id/CA%2BTgmoY7wRGgBbFhKwfASqrNOPwZwCjfm1AhL82769Xx-SyduA%40mail.gmail.com [6] http://archives.postgresql.org/message-id/20120210140637.GB19783%40ldn-qws-004.delacy.com [7] http://archives.postgresql.org/message-id/9966.1331920074%40sss.pgh.pa.us -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
On Mon, Mar 19, 2012 at 1:01 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: I'm rather of the contrary opinion -- surely if we're going to complete function names, we should only complete those that are in schemas in the path; similarly for column names. I think it makes sense to only include currently-visible functions, but *not* only columns from currently visible tables, since we won't know yet whether the user intends to schema-qualify the table name. (BTW I didn't check but does this completion work if I schema-qualify a column name?) Peter's proposed tab-completion only kicks in for the column-name itself. Keep in mind, the user might be trying to enter: SELECT schema.table.column ... SELECT table.column ... SELECT table_alias.column ... SELECT column ... and presumably want to tab-complete the second token somehow. I'm a bit leery about trying to tab-complete those first two, and the third is right out. Just having the fourth would make me happy. Josh -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
On mån, 2012-03-19 at 15:53 -0400, Tom Lane wrote: This connects somewhat to the discussions we've had in the past about trying to get not-intended-for-public-use functions out of the standard search path. Not that you want to put a full visibility check into the tab-completion query, but if it could simply exclude a pg_private namespace, that probably wouldn't be too expensive. I wonder if this could be worked out using pg_depend. For example, give me all functions that are not referenced by some object in pg_catalog. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
On fre, 2012-03-16 at 13:47 -0400, Tom Lane wrote: I'm a bit concerned about whether that's actually going to be useful. A quick check shows that in the regression database, the proposed patch produces 3246 possible completions, which suggests that by the time you get down to a unique match you're going to have typed most of the name anyway. Well, the regression test database is not really an example of real-life object naming, I think. I tried this out on a couple of real databases and found it quite handy. BTW, you should at least exclude dropped columns, I think. Yes. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
Peter Eisentraut pete...@gmx.net writes: On fre, 2012-03-16 at 13:47 -0400, Tom Lane wrote: I'm a bit concerned about whether that's actually going to be useful. A quick check shows that in the regression database, the proposed patch produces 3246 possible completions, which suggests that by the time you get down to a unique match you're going to have typed most of the name anyway. Well, the regression test database is not really an example of real-life object naming, I think. Perhaps not, but a solid 2000 of those names are from the system-created entries in pg_proc, and the regression DB doesn't have an especially large number of tables either. I doubt that real DBs are likely to have materially fewer completions. This connects somewhat to the discussions we've had in the past about trying to get not-intended-for-public-use functions out of the standard search path. Not that you want to put a full visibility check into the tab-completion query, but if it could simply exclude a pg_private namespace, that probably wouldn't be too expensive. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
Excerpts from Tom Lane's message of lun mar 19 16:53:49 -0300 2012: This connects somewhat to the discussions we've had in the past about trying to get not-intended-for-public-use functions out of the standard search path. Not that you want to put a full visibility check into the tab-completion query, I'm rather of the contrary opinion -- surely if we're going to complete function names, we should only complete those that are in schemas in the path; similarly for column names. (BTW I didn't check but does this completion work if I schema-qualify a column name?) but if it could simply exclude a pg_private namespace, that probably wouldn't be too expensive. +1 -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
On tor, 2012-03-15 at 16:36 -0300, Alvaro Herrera wrote: Excerpts from Peter Eisentraut's message of jue mar 15 16:25:53 -0300 2012: On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: I found so this extremely simple patch should be useful. It helps for pattern SELECT fx(); Isn't that just a subset of what I had proposed? http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net So do you intend to commit your patch? Well, there was quite a bit of discussion about it, but it appears that most concerns were addressed at the end. So yes, I guess, unless someone wants further discussion. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
Peter Eisentraut pete...@gmx.net writes: On tor, 2012-03-15 at 16:36 -0300, Alvaro Herrera wrote: Excerpts from Peter Eisentraut's message of jue mar 15 16:25:53 -0300 2012: Isn't that just a subset of what I had proposed? http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net So do you intend to commit your patch? Well, there was quite a bit of discussion about it, but it appears that most concerns were addressed at the end. So yes, I guess, unless someone wants further discussion. I'm a bit concerned about whether that's actually going to be useful. A quick check shows that in the regression database, the proposed patch produces 3246 possible completions, which suggests that by the time you get down to a unique match you're going to have typed most of the name anyway. BTW, you should at least exclude dropped columns, I think. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: I found so this extremely simple patch should be useful. It helps for pattern SELECT fx(); Isn't that just a subset of what I had proposed? http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net There was thread about it. Which thread was that? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
2012/3/15 Peter Eisentraut pete...@gmx.net: On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: I found so this extremely simple patch should be useful. It helps for pattern SELECT fx(); Isn't that just a subset of what I had proposed? http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net There was thread about it. Which thread was that? probably yours :) Regards Pavel -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: autocomplete for functions
Excerpts from Peter Eisentraut's message of jue mar 15 16:25:53 -0300 2012: On sön, 2012-02-19 at 20:10 +0100, Pavel Stehule wrote: I found so this extremely simple patch should be useful. It helps for pattern SELECT fx(); Isn't that just a subset of what I had proposed? http://archives.postgresql.org/message-id/1328820579.11241.4.ca...@vanquo.pezone.net So do you intend to commit your patch? -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] patch: autocomplete for functions
Hello I found so this extremely simple patch should be useful. It helps for pattern SELECT fx(); There was thread about it. Regards Pavel *** ./src/bin/psql/tab-complete.c.orig 2012-02-05 11:28:48.0 +0100 --- ./src/bin/psql/tab-complete.c 2012-02-19 20:05:05.241626625 +0100 *** *** 2555,2562 COMPLETE_WITH_CONST(IS); /* SELECT */ ! /* naah . . . */ ! /* SET, RESET, SHOW */ /* Complete with a variable name */ else if ((pg_strcasecmp(prev_wd, SET) == 0 --- 2555,2562 COMPLETE_WITH_CONST(IS); /* SELECT */ ! else if (pg_strcasecmp(prev_wd, SELECT) == 0) ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_functions, NULL); /* SET, RESET, SHOW */ /* Complete with a variable name */ else if ((pg_strcasecmp(prev_wd, SET) == 0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers