Re: [HACKERS] Tab completion for view triggers in psql
On Thu, Dec 16, 2010 at 07:40:31AM -0500, Greg Smith wrote: > David Fetter wrote: > >That we're in the position of having prevN_wd for N = 1..5 as the > >current code exists is a sign that we need to refactor the whole > >thing, as you've suggested before. > > > >I'll work up a design and prototype for this by this weekend. > > Great. I don't think issues around tab completion are enough to > block the next alpha though, and it sounds like the next stage of > this needs to gel a bit more before it will be ready to commit > anyway. I'm going to mark the remaining bits here as returned for > now, and trust that you'll continue chugging away on this so we can > get it into the next CF early. Will chug. "By this weekend" may have been a touch optimistic. "This weekend" seems a little more realistic :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
David Fetter wrote: That we're in the position of having prevN_wd for N = 1..5 as the current code exists is a sign that we need to refactor the whole thing, as you've suggested before. I'll work up a design and prototype for this by this weekend. Great. I don't think issues around tab completion are enough to block the next alpha though, and it sounds like the next stage of this needs to gel a bit more before it will be ready to commit anyway. I'm going to mark the remaining bits here as returned for now, and trust that you'll continue chugging away on this so we can get it into the next CF early. -- Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD PostgreSQL Training, Services and Supportwww.2ndQuadrant.us "PostgreSQL 9.0 High Performance": http://www.2ndQuadrant.com/books -- 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] Tab completion for view triggers in psql
On Mon, Dec 13, 2010 at 10:48:54PM -0500, Robert Haas wrote: > On Tue, Nov 30, 2010 at 9:15 AM, David Fetter wrote: > >> Patch attached. If you think my changes are ok, > >> please change the patch status to "Ready for Committer". > > > > Done :) > > I have committed part of this patch. Great! > The rest is attached. I don't know that there's any problem with > it, but I ran out of steam. The issue with not committing it is that having a two-word condition (INSTEAD OF vs. BEFORE or AFTER) means that thing that know about preceding BEFORE or AFTER now have to look one word further backward, at least as tab completion works now. That we're in the position of having prevN_wd for N = 1..5 as the current code exists is a sign that we need to refactor the whole thing, as you've suggested before. I'll work up a design and prototype for this by this weekend. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Tue, Nov 30, 2010 at 9:15 AM, David Fetter wrote: >> Patch attached. If you think my changes are ok, >> please change the patch status to "Ready for Committer". > > Done :) I have committed part of this patch. The rest is attached. I don't know that there's any problem with it, but I ran out of steam. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index c88d671..9c027f0 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -384,6 +384,21 @@ static const SchemaQuery Query_for_list_of_tsv = { NULL }; +static const SchemaQuery Query_for_list_of_tv = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind IN ('r', 'v')", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL +}; + static const SchemaQuery Query_for_list_of_views = { /* catname */ "pg_catalog.pg_class c", @@ -681,7 +696,8 @@ psql_completion(char *text, int start, int end) *prev2_wd, *prev3_wd, *prev4_wd, - *prev5_wd; + *prev5_wd, + *prev6_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", @@ -720,7 +736,7 @@ psql_completion(char *text, int start, int end) completion_info_charp2 = NULL; /* - * Scan the input line before our current position for the last five + * Scan the input line before our current position for the last six * words. According to those we'll make some smart decisions on what the * user is probably intending to type. TODO: Use strtokx() to do this. */ @@ -729,6 +745,7 @@ psql_completion(char *text, int start, int end) prev3_wd = previous_word(start, 2); prev4_wd = previous_word(start, 3); prev5_wd = previous_word(start, 4); + prev6_wd = previous_word(start, 5); /* If a backslash command was started, continue */ if (text[0] == '\\') @@ -1025,14 +1042,6 @@ psql_completion(char *text, int start, int end) COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger); } - /* - * If we have ALTER TRIGGER ON, then add the correct tablename - */ - else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && - pg_strcasecmp(prev3_wd, "TRIGGER") == 0 && - pg_strcasecmp(prev_wd, "ON") == 0) - COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); - /* ALTER TRIGGER ON */ else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && pg_strcasecmp(prev2_wd, "ON") == 0) @@ -1644,7 +1653,7 @@ psql_completion(char *text, int start, int end) else if (pg_strcasecmp(prev4_wd, "AS") == 0 && pg_strcasecmp(prev3_wd, "ON") == 0 && pg_strcasecmp(prev_wd, "TO") == 0) - COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); + COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tv, NULL); /* CREATE SERVER */ else if (pg_strcasecmp(prev3_wd, "CREATE") == 0 && @@ -1711,10 +1720,15 @@ psql_completion(char *text, int start, int end) COMPLETE_WITH_LIST(list_CREATETRIGGER); } /* complete CREATE TRIGGER BEFORE,AFTER with an event */ - else if (pg_strcasecmp(prev4_wd, "CREATE") == 0 && + else if ((pg_strcasecmp(prev4_wd, "CREATE") == 0 && pg_strcasecmp(prev3_wd, "TRIGGER") == 0 && (pg_strcasecmp(prev_wd, "BEFORE") == 0 || - pg_strcasecmp(prev_wd, "AFTER") == 0)) + pg_strcasecmp(prev_wd, "AFTER") == 0)) || + (pg_strcasecmp(prev5_wd, "CREATE") == 0 && + pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && + pg_strcasecmp(prev2_wd, "INSTEAD") == 0 && + pg_strcasecmp(prev_wd, "OF") == 0)) + { static const char *const list_CREATETRIGGER_EVENTS[] = {"INSERT", "DELETE", "UPDATE", "TRUNCATE", NULL}; @@ -1722,10 +1736,14 @@ psql_completion(char *text, int start, int end) COMPLETE_WITH_LIST(list_CREATETRIGGER_EVENTS); } /* complete CREATE TRIGGER BEFORE,AFTER sth with OR,ON */ - else if (pg_strcasecmp(prev5_wd, "CREATE") == 0 && + else if ((pg_strcasecmp(prev5_wd, "CREATE") == 0 && pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && (pg_strcasecmp(prev2_wd, "BEFORE") == 0 || - pg_strcasecmp(prev2_wd, "AFTER") == 0)) + pg_strcasecmp(prev2_wd, "AFTER") == 0)) || + (pg_strcasecmp(prev6_wd, "CREATE") == 0 && + pg_strcasecmp(prev5_wd, "TRIGGER") == 0 && + (pg_strcasecmp(prev3_wd, "INSTEAD") == 0 && + (pg_strcasecmp(prev2_wd, "OF") == 0 { static const char *const list_CREATETRIGGER2[] = {"ON", "OR", NULL}; @@ -1742,6 +1760,11 @@ psql_completion(char *text, int start, int end) pg_strcasecmp(prev3_wd, "AFTER") == 0) && pg_strcasecmp(prev_wd, "ON") == 0) COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); + else if (pg_strcasecmp(prev6_wd, "TRIGGER") == 0 && + pg_strcasecmp(prev4_wd, "INSTEAD") == 0 && + pg_strcasecmp(prev3_wd, "OF") == 0 && + pg_strcasecmp(prev_wd, "ON") == 0) + COMPLETE_WITH_SCHEMA_QUERY(Que
Re: [HACKERS] Tab completion for view triggers in psql
On Tue, Nov 30, 2010 at 08:13:37PM +0900, Itagaki Takahiro wrote: > On Tue, Nov 30, 2010 at 18:18, David Fetter wrote: > >> It expands all tables (and views) in tab-completion after INSERT, > >> UPDATE, and DELETE FROM. Is it an intended change? > > I found it was a simple bug; we need ( ) around selcondition. Oh. Heh. Thanks for tracking this down. > In addition, I modified your patch a bit: > > * I added a separated CREATE TRIGGER INSTEAD OF if-branch > because it doesn't accept tables actually; it only accepts > views. OTOH, BEFORE or AFTER only accepts tables. OK > * I think "t.tgtype & (1 << N) <> 0" is more natural > bit operation than "t.tgtype | (1 << N) = t.tgtype". OK > Patch attached. If you think my changes are ok, > please change the patch status to "Ready for Committer". Done :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Tue, Nov 30, 2010 at 18:18, David Fetter wrote: >> It expands all tables (and views) in tab-completion after INSERT, >> UPDATE, and DELETE FROM. Is it an intended change? I found it was a simple bug; we need ( ) around selcondition. In addition, I modified your patch a bit: * I added a separated CREATE TRIGGER INSTEAD OF if-branch because it doesn't accept tables actually; it only accepts views. OTOH, BEFORE or AFTER only accepts tables. * I think "t.tgtype & (1 << N) <> 0" is more natural bit operation than "t.tgtype | (1 << N) = t.tgtype". Patch attached. If you think my changes are ok, please change the patch status to "Ready for Committer". -- Itagaki Takahiro psql_view_trigger_tab_completion_6.diff Description: Binary data -- 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] Tab completion for view triggers in psql
On Tue, Nov 30, 2010 at 05:48:04PM +0900, Itagaki Takahiro wrote: > On Wed, Nov 24, 2010 at 12:21, David Fetter wrote: > > Please find attached a patch changing both this and "updateable" to > > "updatable," also per the very handy git grep I just learned about :) > > I think the patch has two issues to be fixed. > > It expands all tables (and views) in tab-completion after INSERT, > UPDATE, and DELETE FROM. Is it an intended change? IMHO, I don't > want to expand any schema because my console is filled with system > tables and duplicated tables with or without schema names :-( . > > (original) > =# INSERT INTO [tab] > information_schema. pg_temp_1. pg_toast_temp_1. tbl > pg_catalog. pg_toast.public. > > (patched) > =# INSERT INTO [tab] > Display all 113 possibilities? (y or n) Yes. I believe that the question of having INSERT INTO [tab] check for permissions is a separate issue from this patch. This patch does bring the question of whether it *should* check such permission into more focus, though. Whether it should is probably a matter for a separate thread, though. I could create arguments in both directions. > Also, event names are not completed after INSTEAD OF: > > =# CREATE TRIGGER trg BEFORE [tab] > DELETEINSERTTRUNCATE UPDATE > =# CREATE TRIGGER trg INSTEAD OF [tab] > (no candidates) This one's fixed in the attached patch, although subsequent events, as in =# CREATE TRIGGER trg BEFORE INSERT OR [tab] are not. Thanks for your review :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 4c468a8..0aba07c 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -303,6 +303,57 @@ static const SchemaQuery Query_for_list_of_tables = { NULL }; +/* The bit masks for the following three functions come from + * src/include/catalog/pg_trigger.h. + */ +static const SchemaQuery Query_for_list_of_insertables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 2) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL +}; + +static const SchemaQuery Query_for_list_of_deletables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 3) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL +}; + +static const SchemaQuery Query_for_list_of_updatables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 4) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL +}; + static const SchemaQuery Query_for_list_of_tisv = { /* catname */ "pg_catalog.pg_class c", @@ -333,6 +384,21 @@ static const SchemaQuery Query_for_list_of_tsv = { NULL }; +static const SchemaQuery Query_for_list_of_tv = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind IN ('r', 'v')", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL +}; + static const SchemaQuery Query_for_list_of_views = { /* catname */ "pg_catalog.pg_class c", @@ -630,7 +696,8 @@ psql_completion(char *text, int start, int end) *prev2_wd, *prev3_wd, *prev4_wd, - *prev5_wd; + *prev5_wd, + *prev6_wd; static const char *const sql_commands[] = {
Re: [HACKERS] Tab completion for view triggers in psql
On Wed, Nov 24, 2010 at 12:21, David Fetter wrote: > Please find attached a patch changing both this and "updateable" to > "updatable," also per the very handy git grep I just learned about :) I think the patch has two issues to be fixed. It expands all tables (and views) in tab-completion after INSERT, UPDATE, and DELETE FROM. Is it an intended change? IMHO, I don't want to expand any schema because my console is filled with system tables and duplicated tables with or without schema names :-( . (original) =# INSERT INTO [tab] information_schema. pg_temp_1. pg_toast_temp_1. tbl pg_catalog. pg_toast.public. (patched) =# INSERT INTO [tab] Display all 113 possibilities? (y or n) Also, event names are not completed after INSTEAD OF: =# CREATE TRIGGER trg BEFORE [tab] DELETEINSERTTRUNCATE UPDATE =# CREATE TRIGGER trg INSTEAD OF [tab] (no candidates) -- Itagaki Takahiro -- 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] Tab completion for view triggers in psql
On Wed, Nov 24, 2010 at 11:01:28PM -0500, Josh Kupershmidt wrote: > On Tue, Nov 23, 2010 at 10:21 PM, David Fetter wrote: > > Please find attached a patch changing both this and "updateable" to > > "updatable," also per the very handy git grep I just learned about :) > > I looked a little more at this patch today. I didn't find any serious > problems, though it would have helped me (and maybe other reviewers) > to have a comprehensive list of the SQL statements which the patch > implements autocompletion for. OK, will add those to the description. > Not sure how hard this would be to add in, but the following gets > autocompleted with an INSERT: > CREATE TRIGGER mytrg BEFORE IN[TAB] > while the following doesn't find any autocompletions: > CREATE TRIGGER mytrg BEFORE INSERT OR UP[TAB] I might be able to hack something like this together :) > Also, this existed before the patch, but this SQL: > CREATE TRIGGER mytrg INSTEAD OF INSERT ON [TAB] > autocompletes to: > CREATE TRIGGER mytrg INSTEAD OF INSERT ON SET > > not sure how that "SET" is getting in there -- some incorrect tab > completion match? Very likely. At some point, we will have to redo this stuff entirely with a not-yet-invented parser API which will require a live connection to the database in order to work correctly. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Tue, Nov 23, 2010 at 10:21 PM, David Fetter wrote: > Please find attached a patch changing both this and "updateable" to > "updatable," also per the very handy git grep I just learned about :) I looked a little more at this patch today. I didn't find any serious problems, though it would have helped me (and maybe other reviewers) to have a comprehensive list of the SQL statements which the patch implements autocompletion for. Not sure how hard this would be to add in, but the following gets autocompleted with an INSERT: CREATE TRIGGER mytrg BEFORE IN[TAB] while the following doesn't find any autocompletions: CREATE TRIGGER mytrg BEFORE INSERT OR UP[TAB] Also, this existed before the patch, but this SQL: CREATE TRIGGER mytrg INSTEAD OF INSERT ON [TAB] autocompletes to: CREATE TRIGGER mytrg INSTEAD OF INSERT ON SET not sure how that "SET" is getting in there -- some incorrect tab completion match? 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] Tab completion for view triggers in psql
On Tue, Nov 23, 2010 at 09:37:57PM -0500, Josh Kupershmidt wrote: > On Fri, Oct 29, 2010 at 10:33 AM, David Fetter wrote: > > That seems like a matter for a separate patch. Looking this over, I > > found I'd created a query that can never get used, so please find > > enclosed the next version of the patch :) > > I like "deletables" better than "deleteables" for > Query_for_list_of_deleteables. Sources: dictionary.com and a git grep > through the rest of the PG source. Thanks for the review. Please find attached a patch changing both this and "updateable" to "updatable," also per the very handy git grep I just learned about :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *** *** 303,308 static const SchemaQuery Query_for_list_of_tables = { --- 303,359 NULL }; + /* The bit masks for the following three functions come from + * src/include/catalog/pg_trigger.h. + */ + static const SchemaQuery Query_for_list_of_insertables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 2) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_deletables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 3) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_updatables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 4) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_tisv = { /* catname */ "pg_catalog.pg_class c", *** *** 333,338 static const SchemaQuery Query_for_list_of_tsv = { --- 384,404 NULL }; + static const SchemaQuery Query_for_list_of_tv = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind IN ('r', 'v')", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_views = { /* catname */ "pg_catalog.pg_class c", *** *** 630,636 psql_completion(char *text, int start, int end) *prev2_wd, *prev3_wd, *prev4_wd, ! *prev5_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", --- 696,703 *prev2_wd, *prev3_wd, *prev4_wd, ! *prev5_wd, ! *prev6_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", *** *** 669,675 psql_completion(char *text, int start, int end) completion_info_charp2 = NULL; /* !* Scan the input line before our current position for the last five * words. According to those we'll make some smart decisions on what the * user is probably intending to type. TODO: Use strtokx() to do this. */ --- 736,742 completion_info_charp2 = NULL; /* !* Scan the input line before our current position for the last six * words. Acco
Re: [HACKERS] Tab completion for view triggers in psql
On Fri, Oct 29, 2010 at 10:33 AM, David Fetter wrote: > That seems like a matter for a separate patch. Looking this over, I > found I'd created a query that can never get used, so please find > enclosed the next version of the patch :) I like "deletables" better than "deleteables" for Query_for_list_of_deleteables. Sources: dictionary.com and a git grep through the rest of the PG source. 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] Tab completion for view triggers in psql
Excerpts from David Fetter's message of dom nov 21 21:17:12 -0300 2010: > Given its small and isolated nature, I was hoping we could get this in > sooner rather than later. As I understand it, CFs are there to review > patches that take significant effort for even a committer to > understand, so this doesn't really fit that model. It's a 300 line patch -- far from small. It takes me 10 minutes to go through a 3 line change in docs. Maybe that makes me slow, but then if I'm too slow for your patch, I probably don't want to handle it anyhow. Please let's try to not work around the process. (Also, like Robert, I work from the CF app, not from the mailing list anymore. It''s far more convenient -- at least when people follow the rules.) -- Álvaro Herrera 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] Tab completion for view triggers in psql
On Sun, Nov 21, 2010 at 08:27:34PM -0500, Robert Haas wrote: > On Sun, Nov 21, 2010 at 7:17 PM, David Fetter wrote: > > On Sun, Nov 21, 2010 at 07:09:08PM -0500, Robert Haas wrote: > >> On Sun, Nov 21, 2010 at 4:05 PM, David Fetter wrote: > >> >> > Could someone please commit this? :) > >> >> > >> >> Eh... was there some reason you didn't add it to the > >> >> CommitFest app? > >> > > >> > I forgot. > >> > >> A fair excuse. :-) > >> > >> >> Because that's what I work from. > >> > > >> > It's pretty trivial, but I don't feel comfortable adding it > >> > after the close. :/ > >> > >> So add it to the next one, and we'll get it then if nobody picks > >> it up sooner... > > > > Given its small and isolated nature, I was hoping we could get > > this in sooner rather than later. As I understand it, CFs are > > there to review patches that take significant effort for even a > > committer to understand, so this doesn't really fit that model. > > Well, then add it to this one if you think that's more appropriate. Done. :) > My point is simple: I review patches because they are in the CF > queue. Your point seems to be: put mine ahead of the others, and > review it immediately. Someone else may very well be willing to do > that; I'm not. Fair enough. > > David (refraining from mentioning anything about the time taken > > today to discuss this vs. the time it would have taken to push the > > thing) > > Mention anything you want. My guess is it would take me an hour. > You're certainly right that this discussion is a waste of time, but > possibly not for the reasons you are supposing. LOL! Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Sun, Nov 21, 2010 at 7:17 PM, David Fetter wrote: > On Sun, Nov 21, 2010 at 07:09:08PM -0500, Robert Haas wrote: >> On Sun, Nov 21, 2010 at 4:05 PM, David Fetter wrote: >> >> > Could someone please commit this? :) >> >> >> >> Eh... was there some reason you didn't add it to the CommitFest app? >> > >> > I forgot. >> >> A fair excuse. :-) >> >> >> Because that's what I work from. >> > >> > It's pretty trivial, but I don't feel comfortable adding it >> > after the close. :/ >> >> So add it to the next one, and we'll get it then if nobody picks it up >> sooner... > > Given its small and isolated nature, I was hoping we could get this in > sooner rather than later. As I understand it, CFs are there to review > patches that take significant effort for even a committer to > understand, so this doesn't really fit that model. Well, then add it to this one if you think that's more appropriate. My point is simple: I review patches because they are in the CF queue. Your point seems to be: put mine ahead of the others, and review it immediately. Someone else may very well be willing to do that; I'm not. > David (refraining from mentioning anything about the time taken today > to discuss this vs. the time it would have taken to push the thing) Mention anything you want. My guess is it would take me an hour. You're certainly right that this discussion is a waste of time, but possibly not for the reasons you are supposing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Tab completion for view triggers in psql
On Sun, Nov 21, 2010 at 07:09:08PM -0500, Robert Haas wrote: > On Sun, Nov 21, 2010 at 4:05 PM, David Fetter wrote: > >> > Could someone please commit this? :) > >> > >> Eh... was there some reason you didn't add it to the CommitFest app? > > > > I forgot. > > A fair excuse. :-) > > >> Because that's what I work from. > > > > It's pretty trivial, but I don't feel comfortable adding it > > after the close. :/ > > So add it to the next one, and we'll get it then if nobody picks it up > sooner... Given its small and isolated nature, I was hoping we could get this in sooner rather than later. As I understand it, CFs are there to review patches that take significant effort for even a committer to understand, so this doesn't really fit that model. Cheers, David (refraining from mentioning anything about the time taken today to discuss this vs. the time it would have taken to push the thing) -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Sun, Nov 21, 2010 at 4:05 PM, David Fetter wrote: >> > Could someone please commit this? :) >> >> Eh... was there some reason you didn't add it to the CommitFest app? > > I forgot. A fair excuse. :-) >> Because that's what I work from. > > It's pretty trivial, but I don't feel comfortable adding it > after the close. :/ So add it to the next one, and we'll get it then if nobody picks it up sooner... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Tab completion for view triggers in psql
On Sun, Nov 21, 2010 at 03:36:58PM -0500, Robert Haas wrote: > On Sun, Nov 21, 2010 at 1:07 PM, David Fetter wrote: > > On Fri, Oct 29, 2010 at 08:33:00AM -0700, David Fetter wrote: > >> On Tue, Oct 26, 2010 at 11:55:07AM +0900, Itagaki Takahiro wrote: > >> > On Tue, Oct 26, 2010 at 11:34 AM, David Fetter wrote: > >> > >> Do we need to 'add' it? > >> > > Possibly. My understanding is that it couldn't really replace it. > >> > > >> > Ah, I see. I was wrong. We can have modification privileges for > >> > views even if they have no INSTEAD OF triggers. > >> > >> Right. > >> > >> > So, I think your original patch is the best solution. We could use > >> > has_table_privilege() additionally, but we need to consider any > >> > other places if we use it. For example, DROP privileges, etc. > >> > >> That seems like a matter for a separate patch. Looking this over, I > >> found I'd created a query that can never get used, so please find > >> enclosed the next version of the patch :) > > > > Could someone please commit this? :) > > Eh... was there some reason you didn't add it to the CommitFest app? I forgot. > Because that's what I work from. It's pretty trivial, but I don't feel comfortable adding it after the close. :/ Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Sun, Nov 21, 2010 at 1:07 PM, David Fetter wrote: > On Fri, Oct 29, 2010 at 08:33:00AM -0700, David Fetter wrote: >> On Tue, Oct 26, 2010 at 11:55:07AM +0900, Itagaki Takahiro wrote: >> > On Tue, Oct 26, 2010 at 11:34 AM, David Fetter wrote: >> > >> Do we need to 'add' it? >> > > Possibly. My understanding is that it couldn't really replace it. >> > >> > Ah, I see. I was wrong. We can have modification privileges for >> > views even if they have no INSTEAD OF triggers. >> >> Right. >> >> > So, I think your original patch is the best solution. We could use >> > has_table_privilege() additionally, but we need to consider any >> > other places if we use it. For example, DROP privileges, etc. >> >> That seems like a matter for a separate patch. Looking this over, I >> found I'd created a query that can never get used, so please find >> enclosed the next version of the patch :) > > Could someone please commit this? :) Eh... was there some reason you didn't add it to the CommitFest app? Because that's what I work from. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] Tab completion for view triggers in psql
On Fri, Oct 29, 2010 at 08:33:00AM -0700, David Fetter wrote: > On Tue, Oct 26, 2010 at 11:55:07AM +0900, Itagaki Takahiro wrote: > > On Tue, Oct 26, 2010 at 11:34 AM, David Fetter wrote: > > >> Do we need to 'add' it? > > > Possibly. My understanding is that it couldn't really replace it. > > > > Ah, I see. I was wrong. We can have modification privileges for > > views even if they have no INSTEAD OF triggers. > > Right. > > > So, I think your original patch is the best solution. We could use > > has_table_privilege() additionally, but we need to consider any > > other places if we use it. For example, DROP privileges, etc. > > That seems like a matter for a separate patch. Looking this over, I > found I'd created a query that can never get used, so please find > enclosed the next version of the patch :) Could someone please commit this? :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 11:55:07AM +0900, Itagaki Takahiro wrote: > On Tue, Oct 26, 2010 at 11:34 AM, David Fetter wrote: > >> Do we need to 'add' it? > > Possibly. My understanding is that it couldn't really replace it. > > Ah, I see. I was wrong. We can have modification privileges for > views even if they have no INSTEAD OF triggers. Right. > So, I think your original patch is the best solution. We could use > has_table_privilege() additionally, but we need to consider any > other places if we use it. For example, DROP privileges, etc. That seems like a matter for a separate patch. Looking this over, I found I'd created a query that can never get used, so please find enclosed the next version of the patch :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *** *** 303,308 static const SchemaQuery Query_for_list_of_tables = { --- 303,359 NULL }; + /* The bit masks for the following three functions come from + * src/include/catalog/pg_trigger.h. + */ + static const SchemaQuery Query_for_list_of_insertables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 2) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_deleteables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 3) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_updateables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 4) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_tisv = { /* catname */ "pg_catalog.pg_class c", *** *** 333,338 static const SchemaQuery Query_for_list_of_tsv = { --- 384,404 NULL }; + static const SchemaQuery Query_for_list_of_tv = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind IN ('r', 'v')", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_views = { /* catname */ "pg_catalog.pg_class c", *** *** 630,636 psql_completion(char *text, int start, int end) *prev2_wd, *prev3_wd, *prev4_wd, ! *prev5_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", --- 696,703 *prev2_wd, *prev3_wd, *prev4_wd, ! *prev5_wd, ! *prev6_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", *** *** 669,675 psql_completion(char *text, int start, int end) completion_info_charp2 = NULL; /* !* Scan the input line before our current position for the last five * words. According to those we'll make some smart decisions on what the * user is probably intending to type. TODO: Use strtokx() to do this. */ --- 736,742 completion_info_charp2 = NULL; /* !
Re: [HACKERS] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 12:35:13PM +0100, Dean Rasheed wrote: > On 25 October 2010 21:01, David Fetter wrote: > > Folks, > > > > Please find attached patch for $subject :) > > > > Thanks for looking at this. I forgot about tab completion. > > I think that the change to ALTER TRIGGER is not necessary. AFAICT it > works OK unmodified. In fact, the modified code here: > > *** 971,977 psql_completion(char *text, int start, int end) > else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && >pg_strcasecmp(prev3_wd, "TRIGGER") == 0 && >pg_strcasecmp(prev_wd, "ON") == 0) > ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); > > /* ALTER TRIGGER ON */ > else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && > --- 1055,1061 > else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && >pg_strcasecmp(prev3_wd, "TRIGGER") == 0 && >pg_strcasecmp(prev_wd, "ON") == 0) > ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_writeables, NULL); > > /* ALTER TRIGGER ON */ > else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && > > appears to be unreachable, because it is preceded by > > else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && > pg_strcasecmp(prev3_wd, "TRIGGER") == 0) > { > completion_info_charp = prev2_wd; > COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger); > } It is indeed unreachable. > which works for tables and views, and makes the next "elseif" > impossible to satisfy. So I think that block could just be deleted, > right? Yes. Good catch. New patch attached :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *** *** 303,308 static const SchemaQuery Query_for_list_of_tables = { --- 303,375 NULL }; + /* The bit masks for the following three functions come from + * src/include/catalog/pg_trigger.h. + */ + static const SchemaQuery Query_for_list_of_insertables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 2) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_deleteables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 3) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_updateables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 4) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_writeables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND (t.tgtype & ( (1<<2) | (1<<3) | (1<<4)))::bool)", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_tisv = { /* catname */ "pg_catalog.pg_class c", *** *** 333,338 static const SchemaQuery Query_for_list_of_tsv = { --- 400,420 NULL }; + static const SchemaQuery Query_for_list_of_tv = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relk
Re: [HACKERS] Tab completion for view triggers in psql
On 25 October 2010 21:01, David Fetter wrote: > Folks, > > Please find attached patch for $subject :) > Thanks for looking at this. I forgot about tab completion. I think that the change to ALTER TRIGGER is not necessary. AFAICT it works OK unmodified. In fact, the modified code here: *** 971,977 psql_completion(char *text, int start, int end) else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && pg_strcasecmp(prev3_wd, "TRIGGER") == 0 && pg_strcasecmp(prev_wd, "ON") == 0) ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL); /* ALTER TRIGGER ON */ else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && --- 1055,1061 else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && pg_strcasecmp(prev3_wd, "TRIGGER") == 0 && pg_strcasecmp(prev_wd, "ON") == 0) ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_writeables, NULL); /* ALTER TRIGGER ON */ else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && appears to be unreachable, because it is preceded by else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 && pg_strcasecmp(prev3_wd, "TRIGGER") == 0) { completion_info_charp = prev2_wd; COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger); } which works for tables and views, and makes the next "elseif" impossible to satisfy. So I think that block could just be deleted, right? Regards, Dean > Cheers, > David. > -- > David Fetter http://fetter.org/ > Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter > Skype: davidfetter XMPP: david.fet...@gmail.com > iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics > > Remember to vote! > Consider donating to Postgres: http://www.postgresql.org/about/donate > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > -- 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] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 11:34 AM, David Fetter wrote: >> Do we need to 'add' it? > Possibly. My understanding is that it couldn't really replace it. Ah, I see. I was wrong. We can have modification privileges for views even if they have no INSTEAD OF triggers. So, I think your original patch is the best solution. We could use has_table_privilege() additionally, but we need to consider any other places if we use it. For example, DROP privileges, etc. -- Itagaki Takahiro -- 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] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 11:10:53AM +0900, Itagaki Takahiro wrote: > On Tue, Oct 26, 2010 at 10:53 AM, David Fetter wrote: > >> How about has_table_privilege() to filter candidate relations > > > > That's orthogonal to tgtype (snip) Shall I send a new patch with > > that added? > > Do we need to 'add' it? Possibly. My understanding is that it couldn't really replace it. > I intended to replace the JOIN with pg_trigger to > has_table_privilege() (and relkind IN ('r', 'v')) for INSERT, > UPDATE, and DELETE cases. Query_for_list_of_writeables might still > require your patch, though. My understanding is that there are two parts to this: 1. Does the view have the operation (INSERT, UPDATE, or DELETE) defined on it at all? 2. Can the current role actually perform the operation defined? If a view has at least one trigger, that view will have corresponding entries in pg_trigger, and the tgtype entry determines which operations have been defined, hence that EXISTS() query. This establishes part 1. The call to has_table_privilege() establishes part 2. If I've misunderstood, please let me know :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 10:53 AM, David Fetter wrote: >> How about has_table_privilege() to filter candidate relations > > That's orthogonal to tgtype (snip) > Shall I send a new patch with that added? Do we need to 'add' it? I intended to replace the JOIN with pg_trigger to has_table_privilege() (and relkind IN ('r', 'v')) for INSERT, UPDATE, and DELETE cases. Query_for_list_of_writeables might still require your patch, though. -- Itagaki Takahiro -- 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] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 10:30:49AM +0900, Itagaki Takahiro wrote: > On Tue, Oct 26, 2010 at 5:01 AM, David Fetter wrote: > > Please find attached patch for $subject :) > > Thank you for maintaining psql tab completion, but I'm not sure > whether tgtype is the best column for the purpose. How about > has_table_privilege() to filter candidate relations in > Query_for_list_of_insertables/deleteables/updateables? That's orthogonal to tgtype, as far as I can tell. The tgtype test is to tell whether it's possible for anyone to do the operation on the view, where has_table_privilege, as I understand it, tells whether some particular role that privilege. Shall I send a new patch with that added? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Tab completion for view triggers in psql
On Tue, Oct 26, 2010 at 5:01 AM, David Fetter wrote: > Please find attached patch for $subject :) Thank you for maintaining psql tab completion, but I'm not sure whether tgtype is the best column for the purpose. How about has_table_privilege() to filter candidate relations in Query_for_list_of_insertables/deleteables/updateables? -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Tab completion for view triggers in psql
Folks, Please find attached patch for $subject :) Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate *** a/src/bin/psql/tab-complete.c --- b/src/bin/psql/tab-complete.c *** *** 303,308 static const SchemaQuery Query_for_list_of_tables = { --- 303,375 NULL }; + /* The bit masks for the following three functions come from + * src/include/catalog/pg_trigger.h. + */ + static const SchemaQuery Query_for_list_of_insertables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 2) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_deleteables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 3) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_updateables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND t.tgtype | (1 << 4) = t.tgtype))", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + + static const SchemaQuery Query_for_list_of_writeables = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind = 'r' OR (c.relkind = 'v' AND c.relhastriggers AND EXISTS " + "(SELECT 1 FROM pg_catalog.pg_trigger t WHERE t.tgrelid = c.oid AND (t.tgtype & ( (1<<2) | (1<<3) | (1<<4)))::bool)", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_tisv = { /* catname */ "pg_catalog.pg_class c", *** *** 333,338 static const SchemaQuery Query_for_list_of_tsv = { --- 400,420 NULL }; + static const SchemaQuery Query_for_list_of_tv = { + /* catname */ + "pg_catalog.pg_class c", + /* selcondition */ + "c.relkind IN ('r', 'v')", + /* viscondition */ + "pg_catalog.pg_table_is_visible(c.oid)", + /* namespace */ + "c.relnamespace", + /* result */ + "pg_catalog.quote_ident(c.relname)", + /* qualresult */ + NULL + }; + static const SchemaQuery Query_for_list_of_views = { /* catname */ "pg_catalog.pg_class c", *** *** 630,636 psql_completion(char *text, int start, int end) *prev2_wd, *prev3_wd, *prev4_wd, ! *prev5_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", --- 712,719 *prev2_wd, *prev3_wd, *prev4_wd, ! *prev5_wd, ! *prev6_wd; static const char *const sql_commands[] = { "ABORT", "ALTER", "ANALYZE", "BEGIN", "CHECKPOINT", "CLOSE", "CLUSTER", *** *** 669,675 psql_completion(char *text, int start, int end) completion_info_charp2 = NULL; /* !* Scan the input line before our current position for the last five * words. According to those we'll make some smart decisions on what the * user is probably intending to type. TODO: Use strtokx() to do this. */ --- 752,758 completion_info_charp2 = NULL; /* !* Scan the input line before our current position for the last six * words. Acco