Re: [HACKERS] Tab completion for view triggers in psql

2010-12-16 Thread Greg Smith

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

2010-12-16 Thread David Fetter
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 da...@fetter.org 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

2010-12-14 Thread David Fetter
On Mon, Dec 13, 2010 at 10:48:54PM -0500, Robert Haas wrote:
 On Tue, Nov 30, 2010 at 9:15 AM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-12-13 Thread Robert Haas
On Tue, Nov 30, 2010 at 9:15 AM, David Fetter da...@fetter.org 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 sth 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 name ON name */
 	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 name */
 	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 name 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 name 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(Query_for_list_of_views, NULL);
 	/* complete CREATE TRIGGER ... EXECUTE with PROCEDURE */
 	else if 

Re: [HACKERS] Tab completion for view triggers in psql

2010-11-30 Thread Itagaki Takahiro
On Wed, Nov 24, 2010 at 12:21, David Fetter da...@fetter.org 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

2010-11-30 Thread David Fetter
On Tue, Nov 30, 2010 at 05:48:04PM +0900, Itagaki Takahiro wrote:
 On Wed, Nov 24, 2010 at 12:21, David Fetter da...@fetter.org 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 da...@fetter.org 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[] = {
ABORT, ALTER, ANALYZE, BEGIN, CHECKPOINT, CLOSE, 

Re: [HACKERS] Tab completion for view triggers in psql

2010-11-30 Thread Itagaki Takahiro
On Tue, Nov 30, 2010 at 18:18, David Fetter da...@fetter.org 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

2010-11-30 Thread David Fetter
On Tue, Nov 30, 2010 at 08:13:37PM +0900, Itagaki Takahiro wrote:
 On Tue, Nov 30, 2010 at 18:18, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-11-26 Thread David Fetter
On Wed, Nov 24, 2010 at 11:01:28PM -0500, Josh Kupershmidt wrote:
 On Tue, Nov 23, 2010 at 10:21 PM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-11-24 Thread Josh Kupershmidt
On Tue, Nov 23, 2010 at 10:21 PM, David Fetter da...@fetter.org 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

2010-11-23 Thread Josh Kupershmidt
On Fri, Oct 29, 2010 at 10:33 AM, David Fetter da...@fetter.org 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

2010-11-23 Thread David Fetter
On Tue, Nov 23, 2010 at 09:37:57PM -0500, Josh Kupershmidt wrote:
 On Fri, Oct 29, 2010 at 10:33 AM, David Fetter da...@fetter.org 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 da...@fetter.org 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. According to those we'll make some smart decisions on what the
 * 

Re: [HACKERS] Tab completion for view triggers in psql

2010-11-22 Thread Alvaro Herrera
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 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] Tab completion for view triggers in psql

2010-11-21 Thread David Fetter
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 da...@fetter.org 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 da...@fetter.org 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

2010-11-21 Thread Robert Haas
On Sun, Nov 21, 2010 at 1:07 PM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-11-21 Thread David Fetter
On Sun, Nov 21, 2010 at 03:36:58PM -0500, Robert Haas wrote:
 On Sun, Nov 21, 2010 at 1:07 PM, David Fetter da...@fetter.org 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 da...@fetter.org 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 da...@fetter.org 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

2010-11-21 Thread Robert Haas
On Sun, Nov 21, 2010 at 4:05 PM, David Fetter da...@fetter.org 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

2010-11-21 Thread David Fetter
On Sun, Nov 21, 2010 at 07:09:08PM -0500, Robert Haas wrote:
 On Sun, Nov 21, 2010 at 4:05 PM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-11-21 Thread Robert Haas
On Sun, Nov 21, 2010 at 7:17 PM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-11-21 Thread David Fetter
On Sun, Nov 21, 2010 at 08:27:34PM -0500, Robert Haas wrote:
 On Sun, Nov 21, 2010 at 7:17 PM, David Fetter da...@fetter.org 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 da...@fetter.org 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 da...@fetter.org 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

2010-10-29 Thread David Fetter
On Tue, Oct 26, 2010 at 11:55:07AM +0900, Itagaki Takahiro wrote:
 On Tue, Oct 26, 2010 at 11:34 AM, David Fetter da...@fetter.org 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 da...@fetter.org 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;
  
/*
!* Scan the input line before our current position for the 

Re: [HACKERS] Tab completion for view triggers in psql

2010-10-26 Thread Dean Rasheed
On 25 October 2010 21:01, David Fetter da...@fetter.org 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 name ON name */
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 name ON name */
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 da...@fetter.org 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

2010-10-26 Thread David Fetter
On Tue, Oct 26, 2010 at 12:35:13PM +0100, Dean Rasheed wrote:
 On 25 October 2010 21:01, David Fetter da...@fetter.org 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 name ON name */
   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 name ON name */
   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 da...@fetter.org 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  ( (12) | (13) | (14)))::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),
+   

[HACKERS] Tab completion for view triggers in psql

2010-10-25 Thread David Fetter
Folks,

Please find attached patch for $subject :)

Cheers,
David.
-- 
David Fetter da...@fetter.org 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  ( (12) | (13) | (14)))::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. According to those we'll make some smart decisions on what the
 * user is probably 

Re: [HACKERS] Tab completion for view triggers in psql

2010-10-25 Thread Itagaki Takahiro
On Tue, Oct 26, 2010 at 5:01 AM, David Fetter da...@fetter.org 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


Re: [HACKERS] Tab completion for view triggers in psql

2010-10-25 Thread David Fetter
On Tue, Oct 26, 2010 at 10:30:49AM +0900, Itagaki Takahiro wrote:
 On Tue, Oct 26, 2010 at 5:01 AM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-10-25 Thread Itagaki Takahiro
On Tue, Oct 26, 2010 at 10:53 AM, David Fetter da...@fetter.org 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

2010-10-25 Thread David Fetter
On Tue, Oct 26, 2010 at 11:10:53AM +0900, Itagaki Takahiro wrote:
 On Tue, Oct 26, 2010 at 10:53 AM, David Fetter da...@fetter.org 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 da...@fetter.org 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

2010-10-25 Thread Itagaki Takahiro
On Tue, Oct 26, 2010 at 11:34 AM, David Fetter da...@fetter.org 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