Обсуждение: Tab completion for ALTER ... SET SCHEMA
Hi, It has annoys me every time I want to move a table to another schema that it completes to SET SCHEMA TO DEFAULT after a couple of presses of the tab key. So today I decided to get off my lazy ass to write a tiny patch to fix this behaviour. :) My first patch for PostgreSQL so a question: Should I add this to the open commitfest? ---- *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *************** psql_completion(char *text, int start, i *** 1387,1392 **** --- 1387,1399 ---- pg_strcasecmp(prev_wd, "USER") == 0) COMPLETE_WITH_QUERY(Query_for_list_of_roles); + /* ALTER <object> <name> SET SCHEMA */ + else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && + pg_strcasecmp(prev2_wd, "SET") == 0 && + pg_strcasecmp(prev_wd, "SCHEMA") == 0) + COMPLETE_WITH_QUERY(Query_for_list_of_schemas); + + /* BEGIN, END, ABORT */ else if (pg_strcasecmp(prev_wd, "BEGIN") == 0 || pg_strcasecmp(prev_wd, "END")== 0 || Regards, Andreas Karlsson
On Fri, Dec 17, 2010 at 8:34 PM, andreas <andreas@proxel.se> wrote: > It has annoys me every time I want to move a table to another schema > that it completes to SET SCHEMA TO DEFAULT after a couple of presses of > the tab key. So today I decided to get off my lazy ass to write a tiny > patch to fix this behaviour. :) > > My first patch for PostgreSQL so a question: Should I add this to the > open commitfest? In general, yes, but before you do that... the patch you've pasted below doesn't implement the behavior you've described above. Above, you're describing removing a completion, but below, you're adding one.So I'm confused. > ---- > > *** a/src/bin/psql/tab-complete.c > --- b/src/bin/psql/tab-complete.c > *************** psql_completion(char *text, int start, i > *** 1387,1392 **** > --- 1387,1399 ---- > pg_strcasecmp(prev_wd, "USER") == 0) > COMPLETE_WITH_QUERY(Query_for_list_of_roles); > > + /* ALTER <object> <name> SET SCHEMA */ > + else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && > + pg_strcasecmp(prev2_wd, "SET") == 0 && > + pg_strcasecmp(prev_wd, "SCHEMA") == 0) > + COMPLETE_WITH_QUERY(Query_for_list_of_schemas); > + > + > /* BEGIN, END, ABORT */ > else if (pg_strcasecmp(prev_wd, "BEGIN") == 0 || > pg_strcasecmp(prev_wd, "END") == 0 || > Two other points: 1. ALTER TABLE and ALTER FUNCTION both seem to have completions for this already, so maybe the above should be made specific to whatever other object type you're concerned about. 2. Attaching the diff makes it much easier to extract than embedding it in the body of the email. Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Ah, sorry forgot the most important part of my explanation. What it does is gets rid of the incorrect completion which comes from the completion rule for "SET foo TO bar" by adding the correct completion for "SET SCHEMA" higher up in the completion function. So instead of an incorrect completion we get the correct one. Regards, Andreas Karlsson On Fri, 2010-12-17 at 21:00 -0500, Robert Haas wrote: > On Fri, Dec 17, 2010 at 8:34 PM, andreas <andreas@proxel.se> wrote: > > It has annoys me every time I want to move a table to another schema > > that it completes to SET SCHEMA TO DEFAULT after a couple of presses of > > the tab key. So today I decided to get off my lazy ass to write a tiny > > patch to fix this behaviour. :) > > > > My first patch for PostgreSQL so a question: Should I add this to the > > open commitfest? > > In general, yes, but before you do that... the patch you've pasted > below doesn't implement the behavior you've described above. Above, > you're describing removing a completion, but below, you're adding one. > So I'm confused. > > > ---- > > > > *** a/src/bin/psql/tab-complete.c > > --- b/src/bin/psql/tab-complete.c > > *************** psql_completion(char *text, int start, i > > *** 1387,1392 **** > > --- 1387,1399 ---- > > pg_strcasecmp(prev_wd, "USER") == 0) > > COMPLETE_WITH_QUERY(Query_for_list_of_roles); > > > > + /* ALTER <object> <name> SET SCHEMA */ > > + else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && > > + pg_strcasecmp(prev2_wd, "SET") == 0 && > > + pg_strcasecmp(prev_wd, "SCHEMA") == 0) > > + COMPLETE_WITH_QUERY(Query_for_list_of_schemas); > > + > > + > > /* BEGIN, END, ABORT */ > > else if (pg_strcasecmp(prev_wd, "BEGIN") == 0 || > > pg_strcasecmp(prev_wd, "END") == 0 || > > > > Two other points: > > 1. ALTER TABLE and ALTER FUNCTION both seem to have completions for > this already, so maybe the above should be made specific to whatever > other object type you're concerned about. > > 2. Attaching the diff makes it much easier to extract than embedding > it in the body of the email. > > Thanks, > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
On Fri, Dec 17, 2010 at 9:14 PM, Andreas Karlsson <andreas@proxel.se> wrote: > Ah, sorry forgot the most important part of my explanation. > > What it does is gets rid of the incorrect completion which comes from > the completion rule for "SET foo TO bar" by adding the correct > completion for "SET SCHEMA" higher up in the completion function. > > So instead of an incorrect completion we get the correct one. But that's not necessarily wrong, if "foo" happens to be the name of a GUC. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, 2010-12-17 at 21:20 -0500, Robert Haas wrote: > On Fri, Dec 17, 2010 at 9:14 PM, Andreas Karlsson <andreas@proxel.se> wrote: > > What it does is gets rid of the incorrect completion which comes from > > the completion rule for "SET foo TO bar" by adding the correct > > completion for "SET SCHEMA" higher up in the completion function. > > > > So instead of an incorrect completion we get the correct one. > > But that's not necessarily wrong, if "foo" happens to be the name of a GUC. Agreed, which is why I made the new rule only match ALTER x x SET SCHEMA while the rest will fall down and match SET . So I should be safe since SCHEMA is a reserved words. When I think of it I may even have be unnecessary to require the ALTER verb. Regards, Andreas
On Fri, Dec 17, 2010 at 9:30 PM, Andreas Karlsson <andreas@proxel.se> wrote: > On Fri, 2010-12-17 at 21:20 -0500, Robert Haas wrote: >> On Fri, Dec 17, 2010 at 9:14 PM, Andreas Karlsson <andreas@proxel.se> wrote: >> > What it does is gets rid of the incorrect completion which comes from >> > the completion rule for "SET foo TO bar" by adding the correct >> > completion for "SET SCHEMA" higher up in the completion function. >> > >> > So instead of an incorrect completion we get the correct one. >> >> But that's not necessarily wrong, if "foo" happens to be the name of a GUC. > > Agreed, which is why I made the new rule only match > > ALTER x x SET SCHEMA > > while the rest will fall down and match > > SET > > . So I should be safe since SCHEMA is a reserved words. When I think of > it I may even have be unnecessary to require the ALTER verb. Hmm. Using 9.1devel, if I type ALTER TABLE foo SET SCHEMA <tab>, I get a list of schemas even without this patch. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Sorry, everyone. Ignore my patch. This was already fixed in HEAD and while I wrote my patch for the HEAD I somehow failed to spot that it was already fixed when testing without my patch. Nice that is fixed, and sorry for the noise. Andreas
On Fri, Dec 17, 2010 at 9:47 PM, Andreas Karlsson <andreas@proxel.se> wrote: > Sorry, everyone. > > Ignore my patch. This was already fixed in HEAD and while I wrote my > patch for the HEAD I somehow failed to spot that it was already fixed > when testing without my patch. > > Nice that is fixed, and sorry for the noise. No sweat. It's sort of weird the way it's set up. Apparently we complete with a list of schemas any time the previous word is SCHEMA. rhaas=# bumble schema <tab> information_schema pg_temp_1 pg_toast_temp_1 pg_catalog pg_toast public I guess that makes sense but the name words_after_create certainly gives one the wrong impression about how it's actually used. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company