psql tab completion for updatable foreign tables

Started by Bernd Helmleover 12 years ago13 messages
#1Bernd Helmle
mailings@oopsware.de
1 attachment(s)

Recently i got annoyed that psql doesn't tab complete to updatable foreign
tables.

Attached is a patch to address this. I'm using the new
pg_relation_is_updatable() function to accomplish this. The function could
also be used for the trigger based stuff in the query, but i haven't
touched this part of the query. The patch ist against HEAD, but maybe it's
worth to apply this to REL9_3_STABLE, too, but not sure what our policy is
at this state...

--
Thanks

Bernd

Attachments:

psql_tab_complete_updatable_fdw.patchapplication/octet-stream; name=psql_tab_complete_updatable_fdw.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 8eb9f83..7859d8c
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** static const SchemaQuery Query_for_list_
*** 377,383 ****
  	"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) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
--- 377,384 ----
  	"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) <> 0))"
! 	" OR (c.relkind = 'f' AND (pg_catalog.pg_relation_is_updatable(c.oid, 'f') & (1 << 3) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
*************** static const SchemaQuery Query_for_list_
*** 393,399 ****
  	"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) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
--- 394,401 ----
  	"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) <> 0))"
! 	" OR (c.relkind = 'f' AND (pg_catalog.pg_relation_is_updatable(c.oid, 'f') & (1 << 4) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
*************** static const SchemaQuery Query_for_list_
*** 409,415 ****
  	"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) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
--- 411,418 ----
  	"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) <> 0))"
! 	" OR (c.relkind = 'f' AND (pg_catalog.pg_relation_is_updatable(c.oid, 'f') & (1 << 2) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
#2Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Bernd Helmle (#1)
1 attachment(s)
Re: psql tab completion for updatable foreign tables

On 8 July 2013 12:46, Bernd Helmle <mailings@oopsware.de> wrote:

Recently i got annoyed that psql doesn't tab complete to updatable foreign
tables.

Attached is a patch to address this. I'm using the new
pg_relation_is_updatable() function to accomplish this. The function could
also be used for the trigger based stuff in the query, but i haven't touched
this part of the query. The patch ist against HEAD, but maybe it's worth to
apply this to REL9_3_STABLE, too, but not sure what our policy is at this
state...

+1

A couple of points though:

* pg_relation_is_updatable is only available in 9.3, whereas psql may
connect to older servers, so it needs to guard against that.

* If we're doing this, I think we should do it for auto-updatable
views at the same time.

There was concern that pg_relation_is_updatable() would end up opening
every relation in the database, hammering performance. I now realise
that these tab-complete queries have a limit (1000 by default) so I
don't think this is such an issue. I tested it by creating 10,000
randomly named auto-updatable views on top of a table, and didn't see
any performance problems.

Here's an updated patch based on the above points.

Regards,
Dean

Attachments:

tab_complete.patchapplication/octet-stream; name=tab_complete.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 8eb9f83..6e42f1e
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** static const SchemaQuery Query_for_list_
*** 371,376 ****
--- 371,381 ----
  
  /* The bit masks for the following three functions come from
   * src/include/catalog/pg_trigger.h.
+  *
+  * If the server version is 9.3 or higher, we use the function
+  * pg_relation_is_updatable() to test foreign tables and views.
+  * This returns a bit mask based on the CmdType enumeration in
+  * src/include/nodes/nodes.h.
   */
  static const SchemaQuery Query_for_list_of_insertables = {
  	/* catname */
*************** static const SchemaQuery Query_for_list_
*** 387,392 ****
--- 392,412 ----
  	/* qualresult */
  	NULL
  };
+ static const SchemaQuery Query_for_list_of_insertables_93 = {
+ 	/* catname */
+ 	"pg_catalog.pg_class c",
+ 	/* selcondition */
+ 	"(c.relkind = 'r' OR "
+ 	"(c.relkind IN ('f', 'v') AND (pg_catalog.pg_relation_is_updatable(c.oid, true) & (1 << 3) <> 0)))",
+ 	/* 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 */
*************** static const SchemaQuery Query_for_list_
*** 403,408 ****
--- 423,443 ----
  	/* qualresult */
  	NULL
  };
+ static const SchemaQuery Query_for_list_of_deletables_93 = {
+ 	/* catname */
+ 	"pg_catalog.pg_class c",
+ 	/* selcondition */
+ 	"(c.relkind = 'r' OR "
+ 	"(c.relkind IN ('f', 'v') AND (pg_catalog.pg_relation_is_updatable(c.oid, true) & (1 << 4) <> 0)))",
+ 	/* 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 */
*************** static const SchemaQuery Query_for_list_
*** 419,424 ****
--- 454,474 ----
  	/* qualresult */
  	NULL
  };
+ static const SchemaQuery Query_for_list_of_updatables_93 = {
+ 	/* catname */
+ 	"pg_catalog.pg_class c",
+ 	/* selcondition */
+ 	"(c.relkind = 'r' OR "
+ 	"(c.relkind IN ('f', 'v') AND (pg_catalog.pg_relation_is_updatable(c.oid, true) & (1 << 2) <> 0)))",
+ 	/* 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_relations = {
  	/* catname */
*************** psql_completion(char *text, int start, i
*** 2362,2368 ****
  	/* Complete DELETE FROM with a list of tables */
  	else if (pg_strcasecmp(prev2_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev_wd, "FROM") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_deletables, NULL);
  	/* Complete DELETE FROM <table> */
  	else if (pg_strcasecmp(prev3_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev2_wd, "FROM") == 0)
--- 2412,2423 ----
  	/* Complete DELETE FROM with a list of tables */
  	else if (pg_strcasecmp(prev2_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev_wd, "FROM") == 0)
! 	{
! 		if (pset.sversion >= 90300)
! 			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_deletables_93, NULL);
! 		else
! 			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_deletables, NULL);
! 	}
  	/* Complete DELETE FROM <table> */
  	else if (pg_strcasecmp(prev3_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev2_wd, "FROM") == 0)
*************** psql_completion(char *text, int start, i
*** 2732,2738 ****
  	/* Complete INSERT INTO with table names */
  	else if (pg_strcasecmp(prev2_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev_wd, "INTO") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_insertables, NULL);
  	/* Complete "INSERT INTO <table> (" with attribute names */
  	else if (pg_strcasecmp(prev4_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev3_wd, "INTO") == 0 &&
--- 2787,2798 ----
  	/* Complete INSERT INTO with table names */
  	else if (pg_strcasecmp(prev2_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev_wd, "INTO") == 0)
! 	{
! 		if (pset.sversion >= 90300)
! 			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_insertables_93, NULL);
! 		else
! 			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_insertables, NULL);
! 	}
  	/* Complete "INSERT INTO <table> (" with attribute names */
  	else if (pg_strcasecmp(prev4_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev3_wd, "INTO") == 0 &&
*************** psql_completion(char *text, int start, i
*** 3133,3139 ****
  /* UPDATE */
  	/* If prev. word is UPDATE suggest a list of tables */
  	else if (pg_strcasecmp(prev_wd, "UPDATE") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_updatables, NULL);
  	/* Complete UPDATE <table> with "SET" */
  	else if (pg_strcasecmp(prev2_wd, "UPDATE") == 0)
  		COMPLETE_WITH_CONST("SET");
--- 3193,3204 ----
  /* UPDATE */
  	/* If prev. word is UPDATE suggest a list of tables */
  	else if (pg_strcasecmp(prev_wd, "UPDATE") == 0)
! 	{
! 		if (pset.sversion >= 90300)
! 			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_updatables_93, NULL);
! 		else
! 			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_updatables, NULL);
! 	}
  	/* Complete UPDATE <table> with "SET" */
  	else if (pg_strcasecmp(prev2_wd, "UPDATE") == 0)
  		COMPLETE_WITH_CONST("SET");
#3Bernd Helmle
mailings@oopsware.de
In reply to: Dean Rasheed (#2)
Re: psql tab completion for updatable foreign tables

--On 8. Juli 2013 16:04:31 +0000 Dean Rasheed <dean.a.rasheed@gmail.com>
wrote:

* pg_relation_is_updatable is only available in 9.3, whereas psql may
connect to older servers, so it needs to guard against that.

Oh of course, i forgot about this. Thanks for pointing out.

* If we're doing this, I think we should do it for auto-updatable
views at the same time.

There was concern that pg_relation_is_updatable() would end up opening
every relation in the database, hammering performance. I now realise
that these tab-complete queries have a limit (1000 by default) so I
don't think this is such an issue. I tested it by creating 10,000
randomly named auto-updatable views on top of a table, and didn't see
any performance problems.

Here's an updated patch based on the above points.

Okay, are you adding this to the september commitfest?

--
Thanks

Bernd

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Bernd Helmle (#3)
Re: psql tab completion for updatable foreign tables

On 11 July 2013 00:03, Bernd Helmle <mailings@oopsware.de> wrote:

--On 8. Juli 2013 16:04:31 +0000 Dean Rasheed <dean.a.rasheed@gmail.com>
wrote:

* pg_relation_is_updatable is only available in 9.3, whereas psql may
connect to older servers, so it needs to guard against that.

Oh of course, i forgot about this. Thanks for pointing out.

* If we're doing this, I think we should do it for auto-updatable
views at the same time.

There was concern that pg_relation_is_updatable() would end up opening
every relation in the database, hammering performance. I now realise
that these tab-complete queries have a limit (1000 by default) so I
don't think this is such an issue. I tested it by creating 10,000
randomly named auto-updatable views on top of a table, and didn't see
any performance problems.

Here's an updated patch based on the above points.

Okay, are you adding this to the september commitfest?

OK, I've done that. I think that it's too late for 9.3.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Samrat Revagade
revagade.samrat@gmail.com
In reply to: Dean Rasheed (#4)
Re: psql tab completion for updatable foreign tables

Okay, are you adding this to the september commitfest?

OK, I've done that. I think that it's too late for 9.3.

+1 for idea.

I have tested patch and got surprising results with Cent-OS
Patch is working fine for Cent-OS 6.2 and RHEL 6.3
But is is giving problem on Cent-OS 6.3 (tab complete for local tables but
not for foreign tables)

I have used following script:

Two postgres servers are running by using ports 5432 and 5433.
Server with port 5432 has postgres_fdw installed and will interact with the
remote server running under port 5433.

psql -p 5433 -c "CREATE TABLE aa_remote (a int, b int)" postgres
postgres=# CREATE EXTENSION postgres_fdw;
postgres=# CREATE SERVER postgres_server FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (host 'localhost', port '5433', dbname 'postgres');
postgres=# CREATE USER MAPPING FOR PUBLIC SERVER postgres_server OPTIONS
(password '');
postgres=# CREATE FOREIGN TABLE aa_foreign (a int, b int) SERVER
postgres_server OPTIONS (table_name 'aa_remote');
postgres=# INSERT into aa_foreign values (1,2);

But while doing any operation on aa_foreign tab do not complete on Cent-OS
6.3 (tab complete for local tables but not for foreign tables)
Is that a problem ?

Regards,
Samrat Revagade

#6Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Samrat Revagade (#5)
Re: psql tab completion for updatable foreign tables

On 20 September 2013 11:29, Samrat Revagade <revagade.samrat@gmail.com> wrote:

Okay, are you adding this to the september commitfest?

OK, I've done that. I think that it's too late for 9.3.

+1 for idea.

I have tested patch and got surprising results with Cent-OS
Patch is working fine for Cent-OS 6.2 and RHEL 6.3
But is is giving problem on Cent-OS 6.3 (tab complete for local tables but
not for foreign tables)

I have used following script:

Two postgres servers are running by using ports 5432 and 5433.
Server with port 5432 has postgres_fdw installed and will interact with the
remote server running under port 5433.

psql -p 5433 -c "CREATE TABLE aa_remote (a int, b int)" postgres
postgres=# CREATE EXTENSION postgres_fdw;
postgres=# CREATE SERVER postgres_server FOREIGN DATA WRAPPER postgres_fdw
OPTIONS (host 'localhost', port '5433', dbname 'postgres');
postgres=# CREATE USER MAPPING FOR PUBLIC SERVER postgres_server OPTIONS
(password '');
postgres=# CREATE FOREIGN TABLE aa_foreign (a int, b int) SERVER
postgres_server OPTIONS (table_name 'aa_remote');
postgres=# INSERT into aa_foreign values (1,2);

But while doing any operation on aa_foreign tab do not complete on Cent-OS
6.3 (tab complete for local tables but not for foreign tables)
Is that a problem ?

Hmm. It works for me. What does pg_relation_is_updatable() return for
your foreign table?

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Samrat Revagade
revagade.samrat@gmail.com
In reply to: Dean Rasheed (#6)
Re: psql tab completion for updatable foreign tables

On Fri, Sep 20, 2013 at 7:54 PM, Dean Rasheed <dean.a.rasheed@gmail.com>wrote:

On 20 September 2013 11:29, Samrat Revagade <revagade.samrat@gmail.com>
wrote:

Okay, are you adding this to the september commitfest?

OK, I've done that. I think that it's too late for 9.3.

+1 for idea.

I have tested patch and got surprising results with Cent-OS
Patch is working fine for Cent-OS 6.2 and RHEL 6.3
But is is giving problem on Cent-OS 6.3 (tab complete for local tables

but

not for foreign tables)

I have used following script:

Two postgres servers are running by using ports 5432 and 5433.
Server with port 5432 has postgres_fdw installed and will interact with

the

remote server running under port 5433.

psql -p 5433 -c "CREATE TABLE aa_remote (a int, b int)" postgres
postgres=# CREATE EXTENSION postgres_fdw;
postgres=# CREATE SERVER postgres_server FOREIGN DATA WRAPPER

postgres_fdw

OPTIONS (host 'localhost', port '5433', dbname 'postgres');
postgres=# CREATE USER MAPPING FOR PUBLIC SERVER postgres_server OPTIONS
(password '');
postgres=# CREATE FOREIGN TABLE aa_foreign (a int, b int) SERVER
postgres_server OPTIONS (table_name 'aa_remote');
postgres=# INSERT into aa_foreign values (1,2);

But while doing any operation on aa_foreign tab do not complete on

Cent-OS

6.3 (tab complete for local tables but not for foreign tables)
Is that a problem ?

Hmm. It works for me. What does pg_relation_is_updatable() return for
your foreign table?

Sorry .my environment has some problem. when I compiled it with fresh
installation of Cent-OS 6.3 it worked.
Patch :
1. Applies cleanly to git master
2. Has necessary source code comments
3. Follows coding standards
4. Solves use case.

#8Pavan Deolasee
pavan.deolasee@gmail.com
In reply to: Samrat Revagade (#7)
Re: psql tab completion for updatable foreign tables

On Mon, Oct 14, 2013 at 7:20 PM, Samrat Revagade
<revagade.samrat@gmail.com>wrote:

Sorry .my environment has some problem.

May be you were using old version of psql ? IIRC tab-completion relies
heavily on the psql side,

Thanks,
Pavan
--
Pavan Deolasee
http://www.linkedin.com/in/pavandeolasee

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Dean Rasheed (#2)
Re: psql tab completion for updatable foreign tables

On Mon, 2013-07-08 at 16:04 +0000, Dean Rasheed wrote:

There was concern that pg_relation_is_updatable() would end up opening
every relation in the database, hammering performance. I now realise
that these tab-complete queries have a limit (1000 by default) so I
don't think this is such an issue. I tested it by creating 10,000
randomly named auto-updatable views on top of a table, and didn't see
any performance problems.

Even if performance isn't a problem, do we really want tab completion
interfering with table locking? That might be a step too far.

Personally, I think this is too fancy anyway. I'd just complete all
views and foreign tables and be done with it. We don't inspect
permissions either, for example. This might be too confusing for users.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Peter Eisentraut (#9)
Re: psql tab completion for updatable foreign tables

On 17 October 2013 03:29, Peter Eisentraut <peter_e@gmx.net> wrote:

On Mon, 2013-07-08 at 16:04 +0000, Dean Rasheed wrote:

There was concern that pg_relation_is_updatable() would end up opening
every relation in the database, hammering performance. I now realise
that these tab-complete queries have a limit (1000 by default) so I
don't think this is such an issue. I tested it by creating 10,000
randomly named auto-updatable views on top of a table, and didn't see
any performance problems.

Even if performance isn't a problem, do we really want tab completion
interfering with table locking? That might be a step too far.

That's a good point. Currently it's calling pg_table_is_visible(),
which is doing similar work opening each relation, but it isn't
locking any of them.

Personally, I think this is too fancy anyway. I'd just complete all
views and foreign tables and be done with it. We don't inspect
permissions either, for example. This might be too confusing for users.

Yeah, I think you're probably right.

Regards,
Dean

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Robert Haas
robertmhaas@gmail.com
In reply to: Dean Rasheed (#10)
Re: psql tab completion for updatable foreign tables

On Fri, Oct 18, 2013 at 1:34 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

Personally, I think this is too fancy anyway. I'd just complete all
views and foreign tables and be done with it. We don't inspect
permissions either, for example. This might be too confusing for users.

Yeah, I think you're probably right.

I tend to agree. When the rules were simple (i.e. pretty much nothing
was updateable) it might have made sense to make tab completion hew to
them, but they're complex enough now that I think it no longer does.
There are now three different ways that a view can be updateable
(auto, trigger, rule) and the rules are complex.

Based on that it sounds like we need a new version of this patch. If
that's not going to happen RSN, we should mark this returned with
feedback and it can be resubmitted if and when someone finds the time
to update it.

Thanks,

--
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

#12Dean Rasheed
dean.a.rasheed@gmail.com
In reply to: Robert Haas (#11)
1 attachment(s)
Re: psql tab completion for updatable foreign tables

On 18 October 2013 16:41, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Oct 18, 2013 at 1:34 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

Personally, I think this is too fancy anyway. I'd just complete all
views and foreign tables and be done with it. We don't inspect
permissions either, for example. This might be too confusing for users.

Yeah, I think you're probably right.

I tend to agree. When the rules were simple (i.e. pretty much nothing
was updateable) it might have made sense to make tab completion hew to
them, but they're complex enough now that I think it no longer does.
There are now three different ways that a view can be updateable
(auto, trigger, rule) and the rules are complex.

Based on that it sounds like we need a new version of this patch. If
that's not going to happen RSN, we should mark this returned with
feedback and it can be resubmitted if and when someone finds the time
to update it.

OK, here's a new version that just completes with all tables, views
and foreign tables.

Doing this makes the insert, update and delete queries all the same,
which means there's not much point in keeping all three, so I've just
kept Query_for_list_of_updatables for use with INSERT, UPDATE and
DELETE.

Regards,
Dean

Attachments:

tab_complete.v3.patchapplication/octet-stream; name=tab_complete.v3.patchDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index ae8f837..2841bf3
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** static const SchemaQuery Query_for_list_
*** 369,415 ****
  	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) <> 0)))",
! 	/* 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) <> 0)))",
! 	/* 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) <> 0)))",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
--- 369,380 ----
  	NULL
  };
  
! /* Relations supporting INSERT, UPDATE or DELETE */
  static const SchemaQuery Query_for_list_of_updatables = {
  	/* catname */
  	"pg_catalog.pg_class c",
  	/* selcondition */
! 	"c.relkind IN ('r', 'f', 'v')",
  	/* viscondition */
  	"pg_catalog.pg_table_is_visible(c.oid)",
  	/* namespace */
*************** psql_completion(char *text, int start, i
*** 2362,2368 ****
  	/* Complete DELETE FROM with a list of tables */
  	else if (pg_strcasecmp(prev2_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev_wd, "FROM") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_deletables, NULL);
  	/* Complete DELETE FROM <table> */
  	else if (pg_strcasecmp(prev3_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev2_wd, "FROM") == 0)
--- 2327,2333 ----
  	/* Complete DELETE FROM with a list of tables */
  	else if (pg_strcasecmp(prev2_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev_wd, "FROM") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_updatables, NULL);
  	/* Complete DELETE FROM <table> */
  	else if (pg_strcasecmp(prev3_wd, "DELETE") == 0 &&
  			 pg_strcasecmp(prev2_wd, "FROM") == 0)
*************** psql_completion(char *text, int start, i
*** 2732,2738 ****
  	/* Complete INSERT INTO with table names */
  	else if (pg_strcasecmp(prev2_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev_wd, "INTO") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_insertables, NULL);
  	/* Complete "INSERT INTO <table> (" with attribute names */
  	else if (pg_strcasecmp(prev4_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev3_wd, "INTO") == 0 &&
--- 2697,2703 ----
  	/* Complete INSERT INTO with table names */
  	else if (pg_strcasecmp(prev2_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev_wd, "INTO") == 0)
! 		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_updatables, NULL);
  	/* Complete "INSERT INTO <table> (" with attribute names */
  	else if (pg_strcasecmp(prev4_wd, "INSERT") == 0 &&
  			 pg_strcasecmp(prev3_wd, "INTO") == 0 &&
#13Robert Haas
robertmhaas@gmail.com
In reply to: Dean Rasheed (#12)
Re: psql tab completion for updatable foreign tables

On Sat, Oct 19, 2013 at 5:44 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

On 18 October 2013 16:41, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Oct 18, 2013 at 1:34 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:

Personally, I think this is too fancy anyway. I'd just complete all
views and foreign tables and be done with it. We don't inspect
permissions either, for example. This might be too confusing for users.

Yeah, I think you're probably right.

I tend to agree. When the rules were simple (i.e. pretty much nothing
was updateable) it might have made sense to make tab completion hew to
them, but they're complex enough now that I think it no longer does.
There are now three different ways that a view can be updateable
(auto, trigger, rule) and the rules are complex.

Based on that it sounds like we need a new version of this patch. If
that's not going to happen RSN, we should mark this returned with
feedback and it can be resubmitted if and when someone finds the time
to update it.

OK, here's a new version that just completes with all tables, views
and foreign tables.

Doing this makes the insert, update and delete queries all the same,
which means there's not much point in keeping all three, so I've just
kept Query_for_list_of_updatables for use with INSERT, UPDATE and
DELETE.

Committed.

--
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