On 06/17/2014 01:36 PM, Ian Barwick wrote:
Thanks for this patch; I'm playing around with rules at the moment and
it was
very useful. A quick review:

- applies cleanly to HEAD

- does what it claims, i.e. adds tab completion support for this syntax:

     ALTER TABLE table { ENABLE | DISABLE } [ ALWAYS | REPLICA ] { RULE
| TRIGGER } rule_or_trigger
     DROP TRIGGER trigger ON relation { CASCADE | RESTRICT }
     DROP RULE rule ON relation { CASCADE | RESTRICT }

- code style is consistent with the project style

Thanks for the review.

One issue - the table's internal triggers will also be listed. which can
result in
something like this:

This is a bit of an extreme case, but I don't think manually manipulating
internal triggers (which can only be done as a superuser) is a common
enough
operation to justify their inclusion. I suggest adding
'AND tgisinternal is FALSE' to 'Query_for_trigger_of_table' to hide them.

Good suggestion. I have attached a patch which filters out the internal triggers, both for ALTER TABLE and DROP TRIGGER. I am not entirely sure about the DROP TRIGGER case but I think I prefer no auto completion of RI triggers.

--
Andreas Karlsson

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 3bb727f..be5c3c5
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** static const SchemaQuery Query_for_list_
*** 626,631 ****
--- 626,639 ----
  "         WHERE pg_catalog.quote_ident(conname)='%s')"
  
  /* the silly-looking length condition is just to eat up the current word */
+ #define Query_for_rule_of_table \
+ "SELECT pg_catalog.quote_ident(rulename) "\
+ "  FROM pg_catalog.pg_class c1, pg_catalog.pg_rewrite "\
+ " WHERE c1.oid=ev_class and (%d = pg_catalog.length('%s'))"\
+ "       and pg_catalog.quote_ident(c1.relname)='%s'"\
+ "       and pg_catalog.pg_table_is_visible(c1.oid)"
+ 
+ /* the silly-looking length condition is just to eat up the current word */
  #define Query_for_list_of_tables_for_rule \
  "SELECT pg_catalog.quote_ident(relname) "\
  "  FROM pg_catalog.pg_class"\
*************** static const SchemaQuery Query_for_list_
*** 635,640 ****
--- 643,657 ----
  "         WHERE pg_catalog.quote_ident(rulename)='%s')"
  
  /* the silly-looking length condition is just to eat up the current word */
+ #define Query_for_trigger_of_table \
+ "SELECT pg_catalog.quote_ident(tgname) "\
+ "  FROM pg_catalog.pg_class c1, pg_catalog.pg_trigger "\
+ " WHERE c1.oid=tgrelid and (%d = pg_catalog.length('%s'))"\
+ "       and pg_catalog.quote_ident(c1.relname)='%s'"\
+ "       and pg_catalog.pg_table_is_visible(c1.oid)"\
+ "       and not tgisinternal"
+ 
+ /* the silly-looking length condition is just to eat up the current word */
  #define Query_for_list_of_tables_for_trigger \
  "SELECT pg_catalog.quote_ident(relname) "\
  "  FROM pg_catalog.pg_class"\
*************** static const pgsql_thing_t words_after_c
*** 774,780 ****
  	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
  	{"TEMPLATE", Query_for_list_of_ts_templates, NULL, THING_NO_SHOW},
  	{"TEXT SEARCH", NULL, NULL},
! 	{"TRIGGER", "SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s'"},
  	{"TYPE", NULL, &Query_for_list_of_datatypes},
  	{"UNIQUE", NULL, NULL, THING_NO_DROP},		/* for CREATE UNIQUE INDEX ... */
  	{"UNLOGGED", NULL, NULL, THING_NO_DROP},	/* for CREATE UNLOGGED TABLE
--- 791,797 ----
  	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */
  	{"TEMPLATE", Query_for_list_of_ts_templates, NULL, THING_NO_SHOW},
  	{"TEXT SEARCH", NULL, NULL},
! 	{"TRIGGER", "SELECT pg_catalog.quote_ident(tgname) FROM pg_catalog.pg_trigger WHERE substring(pg_catalog.quote_ident(tgname),1,%d)='%s' AND NOT tgisinternal"},
  	{"TYPE", NULL, &Query_for_list_of_datatypes},
  	{"UNIQUE", NULL, NULL, THING_NO_DROP},		/* for CREATE UNIQUE INDEX ... */
  	{"UNLOGGED", NULL, NULL, THING_NO_DROP},	/* for CREATE UNLOGGED TABLE
*************** psql_completion(const char *text, int st
*** 1409,1414 ****
--- 1426,1463 ----
  
  		COMPLETE_WITH_LIST(list_ALTERENABLE2);
  	}
+ 	else if (pg_strcasecmp(prev5_wd, "ALTER") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "TABLE") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "ENABLE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "RULE") == 0)
+ 	{
+ 		completion_info_charp = prev3_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_rule_of_table);
+ 	}
+ 	else if (pg_strcasecmp(prev6_wd, "ALTER") == 0 &&
+ 			 pg_strcasecmp(prev5_wd, "TABLE") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "ENABLE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "RULE") == 0)
+ 	{
+ 		completion_info_charp = prev4_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_rule_of_table);
+ 	}
+ 	else if (pg_strcasecmp(prev5_wd, "ALTER") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "TABLE") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "ENABLE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "TRIGGER") == 0)
+ 	{
+ 		completion_info_charp = prev3_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_trigger_of_table);
+ 	}
+ 	else if (pg_strcasecmp(prev6_wd, "ALTER") == 0 &&
+ 			 pg_strcasecmp(prev5_wd, "TABLE") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "ENABLE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "TRIGGER") == 0)
+ 	{
+ 		completion_info_charp = prev4_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_trigger_of_table);
+ 	}
  	/* ALTER TABLE xxx INHERIT */
  	else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
  			 pg_strcasecmp(prev3_wd, "TABLE") == 0 &&
*************** psql_completion(const char *text, int st
*** 1424,1429 ****
--- 1473,1479 ----
  	{
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, "");
  	}
+ 	/* ALTER TABLE xxx DISABLE */
  	else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
  			 pg_strcasecmp(prev3_wd, "TABLE") == 0 &&
  			 pg_strcasecmp(prev_wd, "DISABLE") == 0)
*************** psql_completion(const char *text, int st
*** 1433,1438 ****
--- 1483,1504 ----
  
  		COMPLETE_WITH_LIST(list_ALTERDISABLE);
  	}
+ 	else if (pg_strcasecmp(prev5_wd, "ALTER") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "TABLE") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "DISABLE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "RULE") == 0)
+ 	{
+ 		completion_info_charp = prev3_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_rule_of_table);
+ 	}
+ 	else if (pg_strcasecmp(prev5_wd, "ALTER") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "TABLE") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "DISABLE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "TRIGGER") == 0)
+ 	{
+ 		completion_info_charp = prev3_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_trigger_of_table);
+ 	}
  
  	/* ALTER TABLE xxx ALTER */
  	else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
*************** psql_completion(const char *text, int st
*** 2587,2592 ****
--- 2653,2681 ----
  		COMPLETE_WITH_LIST(list_ALTERTEXTSEARCH);
  	}
  
+ 	/* DROP TRIGGER */
+ 	else if (pg_strcasecmp(prev3_wd, "DROP") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "TRIGGER") == 0)
+ 	{
+ 		COMPLETE_WITH_CONST("ON");
+ 	}
+ 	else if (pg_strcasecmp(prev4_wd, "DROP") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "TRIGGER") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "ON") == 0)
+ 	{
+ 		completion_info_charp = prev2_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger);
+ 	}
+ 	else if (pg_strcasecmp(prev5_wd, "DROP") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "TRIGGER") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "ON") == 0)
+ 	{
+ 		static const char *const list_DROPCR[] =
+ 		{"CASCADE", "RESTRICT", NULL};
+ 
+ 		COMPLETE_WITH_LIST(list_DROPCR);
+ 	}
+ 
  	/* DROP EVENT TRIGGER */
  	else if (pg_strcasecmp(prev2_wd, "DROP") == 0 &&
  			 pg_strcasecmp(prev_wd, "EVENT") == 0)
*************** psql_completion(const char *text, int st
*** 2600,2605 ****
--- 2689,2717 ----
  		COMPLETE_WITH_QUERY(Query_for_list_of_event_triggers);
  	}
  
+ 	/* DROP RULE */
+ 	else if (pg_strcasecmp(prev3_wd, "DROP") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "RULE") == 0)
+ 	{
+ 		COMPLETE_WITH_CONST("ON");
+ 	}
+ 	else if (pg_strcasecmp(prev4_wd, "DROP") == 0 &&
+ 			 pg_strcasecmp(prev3_wd, "RULE") == 0 &&
+ 			 pg_strcasecmp(prev_wd, "ON") == 0)
+ 	{
+ 		completion_info_charp = prev2_wd;
+ 		COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_rule);
+ 	}
+ 	else if (pg_strcasecmp(prev5_wd, "DROP") == 0 &&
+ 			 pg_strcasecmp(prev4_wd, "RULE") == 0 &&
+ 			 pg_strcasecmp(prev2_wd, "ON") == 0)
+ 	{
+ 		static const char *const list_DROPCR[] =
+ 		{"CASCADE", "RESTRICT", NULL};
+ 
+ 		COMPLETE_WITH_LIST(list_DROPCR);
+ 	}
+ 
  /* EXECUTE, but not EXECUTE embedded in other commands */
  	else if (pg_strcasecmp(prev_wd, "EXECUTE") == 0 &&
  			 prev2_wd[0] == '\0')
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to