Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Andres Freund]

On 2015-09-08 07:06:04 +0200, Pavel Stehule wrote:
> 2015-09-07 21:44 GMT+02:00 Andres Freund :
> > The spellings for boolean values were a relatively small subset of what
> > the backend accepts - it's now on,off,true,false,yes,no,1,0. I'm not
> > sure whether that's a good idea. Comments?
> >
> 
> if somebody prefer true, false, and we will support only on, off, then the
> tabcomplete will not be too user friendly :(
> 
> "1, 0" can be out - but other?

After sleeping on it I think we should keep all of them - they'll show
for lots of "boolean like" GUCs (e.g. constraint_exclusion, sync_commit,
huge_pages) so not showing them for booleans just seems
inconsisten. Unless somebody protests pdq I'll push it that way.

Andres


-- 
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] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Alvaro Herrera]

Andres Freund wrote:
> On 2015-09-08 07:06:04 +0200, Pavel Stehule wrote:
> > 2015-09-07 21:44 GMT+02:00 Andres Freund :
> > > The spellings for boolean values were a relatively small subset of what
> > > the backend accepts - it's now on,off,true,false,yes,no,1,0. I'm not
> > > sure whether that's a good idea. Comments?
> > 
> > if somebody prefer true, false, and we will support only on, off, then the
> > tabcomplete will not be too user friendly :(
> > 
> > "1, 0" can be out - but other?
> 
> After sleeping on it I think we should keep all of them - they'll show
> for lots of "boolean like" GUCs (e.g. constraint_exclusion, sync_commit,
> huge_pages) so not showing them for booleans just seems
> inconsisten. Unless somebody protests pdq I'll push it that way.

Yeah, seems fine to list the whole lot.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
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] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

2015-09-07 21:44 GMT+02:00 Andres Freund :

> Hi,
>
> On 2015-09-02 22:58:21 +0200, Pavel Stehule wrote:
> > > Won't that mean that enum variables don't complete to default anymore?
>
> > no, it does
> >
> > #define Query_for_enum \
> > " SELECT name FROM ( "\
> > "   SELECT unnest(enumvals) AS name "\
> > "FROM pg_catalog.pg_settings "\
> > "   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
> > "   UNION SELECT 'DEFAULT' ) ss "\
> > 
> > "  WHERE pg_catalog.substring(name,1,%%d)='%%s'"
>
> Ah.
>
> I've added a quote_ident() around unnest() - otherwise the
> auto-completions for default_transaction_isolation will mostly be wrong
> due to spaces.
>
> I also renamed get_vartype into get_guctype, changed the comment as I
> found the reference to the pg type system confusing, and more
> importantly made it not return a static buffer.
>
> The spellings for boolean values were a relatively small subset of what
> the backend accepts - it's now on,off,true,false,yes,no,1,0. I'm not
> sure whether that's a good idea. Comments?
>

if somebody prefer true, false, and we will support only on, off, then the
tabcomplete will not be too user friendly :(

"1, 0" can be out - but other?

>
> Andres
>

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Andres Freund]

Hi,

On 2015-09-02 22:58:21 +0200, Pavel Stehule wrote:
> > Won't that mean that enum variables don't complete to default anymore?

> no, it does
> 
> #define Query_for_enum \
> " SELECT name FROM ( "\
> "   SELECT unnest(enumvals) AS name "\
> "FROM pg_catalog.pg_settings "\
> "   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
> "   UNION SELECT 'DEFAULT' ) ss "\
> 
> "  WHERE pg_catalog.substring(name,1,%%d)='%%s'"

Ah.

I've added a quote_ident() around unnest() - otherwise the
auto-completions for default_transaction_isolation will mostly be wrong
due to spaces.

I also renamed get_vartype into get_guctype, changed the comment as I
found the reference to the pg type system confusing, and more
importantly made it not return a static buffer.

The spellings for boolean values were a relatively small subset of what
the backend accepts - it's now on,off,true,false,yes,no,1,0. I'm not
sure whether that's a good idea. Comments?

Andres
>From 279cdbdaed568a9dd95e18b4bb5c3098a0791008 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 7 Sep 2015 21:28:10 +0200
Subject: [PATCH] psql: Generic tab completion support for enum and bool GUCs.

Author: Pavel Stehule
Reviewed-By: Andres Freund
Discussion: 5594fe7a.5050...@iki.fi
---
 src/bin/psql/tab-complete.c | 132 
 1 file changed, 97 insertions(+), 35 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9303f6a..85207cc 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -757,6 +757,15 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   (SELECT polrelid FROM pg_catalog.pg_policy "\
 " WHERE pg_catalog.quote_ident(polname)='%s')"
 
+#define Query_for_enum \
+" SELECT name FROM ( "\
+"   SELECT pg_catalog.quote_ident(pg_catalog.unnest(enumvals)) AS name "\
+" FROM pg_catalog.pg_settings "\
+"WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
+"UNION ALL " \
+"   SELECT 'DEFAULT' ) ss "\
+"  WHERE pg_catalog.substring(name,1,%%d)='%%s'"
+
 /*
  * This is a list of all "things" in Pgsql, which can show up after CREATE or
  * DROP; and there is also a query to get a list of them.
@@ -843,10 +852,13 @@ static char **complete_from_variables(const char *text,
 static char *complete_from_files(const char *text, int state);
 
 static char *pg_strdup_keyword_case(const char *s, const char *ref);
+static char *escape_string(const char *text);
 static PGresult *exec_query(const char *query);
 
 static void get_previous_words(int point, char **previous_words, int nwords);
 
+static char *get_guctype(const char *varname);
+
 #ifdef NOT_USED
 static char *quote_file_name(char *text, int match_type, char *quote_pointer);
 static char *dequote_file_name(char *text, char quote_char);
@@ -3594,6 +3606,7 @@ psql_completion(const char *text, int start, int end)
 	else if (pg_strcasecmp(prev3_wd, "SET") == 0 &&
 			 (pg_strcasecmp(prev_wd, "TO") == 0 || strcmp(prev_wd, "=") == 0))
 	{
+		/* special cased code for individual GUCs */
 		if (pg_strcasecmp(prev2_wd, "DateStyle") == 0)
 		{
 			static const char *const my_list[] =
@@ -3604,20 +3617,6 @@ psql_completion(const char *text, int start, int end)
 
 			COMPLETE_WITH_LIST(my_list);
 		}
-		else if (pg_strcasecmp(prev2_wd, "IntervalStyle") == 0)
-		{
-			static const char *const my_list[] =
-			{"postgres", "postgres_verbose", "sql_standard", "iso_8601", NULL};
-
-			COMPLETE_WITH_LIST(my_list);
-		}
-		else if (pg_strcasecmp(prev2_wd, "GEQO") == 0)
-		{
-			static const char *const my_list[] =
-			{"ON", "OFF", "DEFAULT", NULL};
-
-			COMPLETE_WITH_LIST(my_list);
-		}
 		else if (pg_strcasecmp(prev2_wd, "search_path") == 0)
 		{
 			COMPLETE_WITH_QUERY(Query_for_list_of_schemas
@@ -3627,10 +3626,34 @@ psql_completion(const char *text, int start, int end)
 		}
 		else
 		{
-			static const char *const my_list[] =
-			{"DEFAULT", NULL};
+			/* generic, type based, GUC support */
 
-			COMPLETE_WITH_LIST(my_list);
+			char	   *guctype = get_guctype(prev2_wd);
+
+			if (guctype && strcmp(guctype, "enum") == 0)
+			{
+char		querybuf[1024];
+
+snprintf(querybuf, 1024, Query_for_enum, prev2_wd);
+COMPLETE_WITH_QUERY(querybuf);
+			}
+			else if (guctype && strcmp(guctype, "bool") == 0)
+			{
+static const char *const my_list[] =
+{"on", "off", "true", "false", "yes", "no", "1", "0", "DEFAULT", NULL};
+
+COMPLETE_WITH_LIST(my_list);
+			}
+			else
+			{
+static const char *const my_list[] =
+{"DEFAULT", NULL};
+
+COMPLETE_WITH_LIST(my_list);
+			}
+
+			if (guctype)
+free(guctype);
 		}
 	}
 
@@ -4173,30 +4196,15 @@ _complete_from_query(int is_schema_query, const char *text, int state)
 		result = NULL;
 
 		/* Set up suitably-escaped copies of textual inputs */
-		e_text = pg_malloc(string_length * 2 + 1);
-		PQescapeString(e_text, text, string_length);
+		e_text = escape_string(text);
 
 		if 

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

Hi

2015-09-02 15:23 GMT+02:00 Andres Freund :

> Hi,
>
> On 2015-07-08 14:50:37 +0200, Pavel Stehule wrote:
> > - static const char *const my_list[] =
> > - {"DEFAULT", NULL};
> > + /* fallback for GUC settings */
> >
> > - COMPLETE_WITH_LIST(my_list);
> > + char *vartype = get_vartype(prev2_wd);
> > +
> > + if (strcmp(vartype, "enum") == 0)
> > + {
> > + char querybuf[1024];
> > +
> > + snprintf(querybuf, 1024, Query_for_enum,
> prev2_wd);
> > + COMPLETE_WITH_QUERY(querybuf);
> > + }
>
> Won't that mean that enum variables don't complete to default anymore?
>

no, it does

#define Query_for_enum \
" SELECT name FROM ( "\
"   SELECT unnest(enumvals) AS name "\
"FROM pg_catalog.pg_settings "\
"   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
"   UNION SELECT 'DEFAULT' ) ss "\

"  WHERE pg_catalog.substring(name,1,%%d)='%%s'"



>
> > +static char *
> > +get_vartype(const char *varname)
> > +{
> > + PQExpBufferData query_buffer;
> > + char*e_varname;
> > + PGresult *result;
> > + int string_length;
> > + static char resbuf[10];
> > +
> > + initPQExpBuffer(_buffer);
> > +
> > + string_length = strlen(varname);
> > + e_varname = pg_malloc(string_length * 2 + 1);
> > + PQescapeString(e_varname, varname, string_length);
>
> Independent of this patch, we really shouldn't do this in several places
> :(
>

fixed

>
> > + appendPQExpBuffer(_buffer,
> > + "SELECT vartype FROM pg_settings WHERE
> pg_catalog.lower(name) = pg_catalog.lower('%s')",
> > +  e_varname);
>
> Missing pg_catalog for pg_settings.
>

fixed

>
> Greetings,
>
> Andres Freund
>

I am sending new version

Regards

Pavel
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 816deda..61216e1
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*** static const SchemaQuery Query_for_list_
*** 757,762 
--- 757,770 
  "   (SELECT polrelid FROM pg_catalog.pg_policy "\
  " WHERE pg_catalog.quote_ident(polname)='%s')"
  
+ #define Query_for_enum \
+ " SELECT name FROM ( "\
+ "   SELECT unnest(enumvals) AS name "\
+ "FROM pg_catalog.pg_settings "\
+ "   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') "\
+ "   UNION SELECT 'DEFAULT' ) ss "\
+ "  WHERE pg_catalog.substring(name,1,%%d)='%%s'"
+ 
  /*
   * This is a list of all "things" in Pgsql, which can show up after CREATE or
   * DROP; and there is also a query to get a list of them.
*** static PGresult *exec_query(const char *
*** 847,852 
--- 855,863 
  
  static void get_previous_words(int point, char **previous_words, int nwords);
  
+ static char *get_vartype(const char *varname);
+ static char *escape_string(const char *text);
+ 
  #ifdef NOT_USED
  static char *quote_file_name(char *text, int match_type, char *quote_pointer);
  static char *dequote_file_name(char *text, char quote_char);
*** psql_completion(const char *text, int st
*** 3604,3623 
  
  			COMPLETE_WITH_LIST(my_list);
  		}
- 		else if (pg_strcasecmp(prev2_wd, "IntervalStyle") == 0)
- 		{
- 			static const char *const my_list[] =
- 			{"postgres", "postgres_verbose", "sql_standard", "iso_8601", NULL};
- 
- 			COMPLETE_WITH_LIST(my_list);
- 		}
- 		else if (pg_strcasecmp(prev2_wd, "GEQO") == 0)
- 		{
- 			static const char *const my_list[] =
- 			{"ON", "OFF", "DEFAULT", NULL};
- 
- 			COMPLETE_WITH_LIST(my_list);
- 		}
  		else if (pg_strcasecmp(prev2_wd, "search_path") == 0)
  		{
  			COMPLETE_WITH_QUERY(Query_for_list_of_schemas
--- 3615,3620 
*** psql_completion(const char *text, int st
*** 3627,3636 
  		}
  		else
  		{
! 			static const char *const my_list[] =
! 			{"DEFAULT", NULL};
  
! 			COMPLETE_WITH_LIST(my_list);
  		}
  	}
  
--- 3624,3654 
  		}
  		else
  		{
! 			/* fallback for GUC settings */
  
! 			char *vartype = get_vartype(prev2_wd);
! 
! 			if (strcmp(vartype, "enum") == 0)
! 			{
! char querybuf[1024];
! 
! snprintf(querybuf, 1024, Query_for_enum, prev2_wd);
! COMPLETE_WITH_QUERY(querybuf);
! 			}
! 			else if (strcmp(vartype, "bool") == 0)
! 			{
! static const char *const my_list[] =
! {"ON", "OFF", "DEFAULT", NULL};
! 
! COMPLETE_WITH_LIST(my_list);
! 			}
! 			else
! 			{
! static const char *const my_list[] =
! {"DEFAULT", NULL};
! 
! COMPLETE_WITH_LIST(my_list);
! 			}
  		}
  	}
  
*** _complete_from_query(int is_schema_query
*** 4166,4195 
  		result = NULL;
  
  		/* Set up suitably-escaped copies of textual inputs */
! 		e_text = pg_malloc(string_length * 2 + 1);
! 		PQescapeString(e_text, text, string_length);
  
  		if 

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Andres Freund]

Hi,

On 2015-07-08 14:50:37 +0200, Pavel Stehule wrote:
> - static const char *const my_list[] =
> - {"DEFAULT", NULL};
> + /* fallback for GUC settings */
>  
> - COMPLETE_WITH_LIST(my_list);
> + char *vartype = get_vartype(prev2_wd);
> +
> + if (strcmp(vartype, "enum") == 0)
> + {
> + char querybuf[1024];
> +
> + snprintf(querybuf, 1024, Query_for_enum, 
> prev2_wd);
> + COMPLETE_WITH_QUERY(querybuf);
> + }

Won't that mean that enum variables don't complete to default anymore?

> +static char *
> +get_vartype(const char *varname)
> +{
> + PQExpBufferData query_buffer;
> + char*e_varname;
> + PGresult *result;
> + int string_length;
> + static char resbuf[10];
> +
> + initPQExpBuffer(_buffer);
> +
> + string_length = strlen(varname);
> + e_varname = pg_malloc(string_length * 2 + 1);
> + PQescapeString(e_varname, varname, string_length);

Independent of this patch, we really shouldn't do this in several places
:(

> + appendPQExpBuffer(_buffer,
> + "SELECT vartype FROM pg_settings WHERE pg_catalog.lower(name) = 
> pg_catalog.lower('%s')",
> +  e_varname);

Missing pg_catalog for pg_settings.

Greetings,

Andres Freund


-- 
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] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

2015-07-02 11:03 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:

 On 05/29/2015 10:41 AM, Pavel Stehule wrote:

 2015-05-29 9:28 GMT+02:00 Jeevan Chalke jeevan.cha...@gmail.com:

  I agree with Peter that We don't tab-complete everything we possibly
 could, but using tabs after SET ROLE TO  provides DEFAULT as an
 option
 which seems wrong.
 This patch adds list of roles over there, which I guess good to have than
 giving something unusual.

  ...

 But back to this topic. I am thinking so it is little bit different due
 fact so we support two very syntax for one feature. And looks little bit
 strange, so one way is supported by autocomplete and second not.


 Yeah, it's a bit strange. We have a specific autocomplete rule for SET
 ROLE, but SET ROLE TO is treated as a generic GUC. With your patch, we'd
 also lose the auto-completion to SET ROLE TO DEFAULT.

 I think we want to encourage people to use the SQL-standard syntax SET
 ROLE ... rather than the PostgreSQL-specific SET ROLE TO  On the
 whole, this just doesn't seem like much of an improvement. I'll mark this
 as 'rejected' in the commitfest app.

 PS. I note that the auto-completion for SET XXX TO ... is pretty lousy in
 general. We have rules for DateStyle, IntervalStyle, GEQO and search_path,
 but that's it. That could be expanded a lot. All enum-type GUCs could be
 handled with a single rule that queries pg_settings.enumvals, for example,
 and booleans would be easy too. But that's a different story.


I wrote a patch for fallback tabcomplete for bool and enum GUC variables

Regards

Pavel



 - Heikki


commit 7749d7d3fabf468dbe2c5f397add9f8e31f59614
Author: Pavel Stehule pavel.steh...@gooddata.com
Date:   Wed Jul 8 14:24:55 2015 +0200

initial

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 0683548..c4e56c8 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -742,6 +742,14 @@ static const SchemaQuery Query_for_list_of_matviews = {
FROM pg_catalog.pg_tablesample_method \
   WHERE substring(pg_catalog.quote_ident(tsmname),1,%d)='%s'
 
+#define Query_for_enum \
+ SELECT name FROM ( \
+   SELECT unnest(enumvals) AS name \
+FROM pg_catalog.pg_settings \
+   WHERE pg_catalog.lower(name)=pg_catalog.lower('%s') \
+   UNION SELECT 'DEFAULT' ) ss \
+  WHERE pg_catalog.substring(name,1,%%d)='%%s'
+
 /*
  * This is a list of all things in Pgsql, which can show up after CREATE or
  * DROP; and there is also a query to get a list of them.
@@ -832,6 +840,8 @@ static PGresult *exec_query(const char *query);
 
 static void get_previous_words(int point, char **previous_words, int nwords);
 
+static char *get_vartype(const char *varname);
+
 #ifdef NOT_USED
 static char *quote_file_name(char *text, int match_type, char *quote_pointer);
 static char *dequote_file_name(char *text, char quote_char);
@@ -3548,20 +3558,6 @@ psql_completion(const char *text, int start, int end)
 
 			COMPLETE_WITH_LIST(my_list);
 		}
-		else if (pg_strcasecmp(prev2_wd, IntervalStyle) == 0)
-		{
-			static const char *const my_list[] =
-			{postgres, postgres_verbose, sql_standard, iso_8601, NULL};
-
-			COMPLETE_WITH_LIST(my_list);
-		}
-		else if (pg_strcasecmp(prev2_wd, GEQO) == 0)
-		{
-			static const char *const my_list[] =
-			{ON, OFF, DEFAULT, NULL};
-
-			COMPLETE_WITH_LIST(my_list);
-		}
 		else if (pg_strcasecmp(prev2_wd, search_path) == 0)
 		{
 			COMPLETE_WITH_QUERY(Query_for_list_of_schemas
@@ -3571,10 +3567,31 @@ psql_completion(const char *text, int start, int end)
 		}
 		else
 		{
-			static const char *const my_list[] =
-			{DEFAULT, NULL};
+			/* fallback for GUC settings */
 
-			COMPLETE_WITH_LIST(my_list);
+			char *vartype = get_vartype(prev2_wd);
+
+			if (strcmp(vartype, enum) == 0)
+			{
+char querybuf[1024];
+
+snprintf(querybuf, 1024, Query_for_enum, prev2_wd);
+COMPLETE_WITH_QUERY(querybuf);
+			}
+			else if (strcmp(vartype, bool) == 0)
+			{
+static const char *const my_list[] =
+{ON, OFF, DEFAULT, NULL};
+
+COMPLETE_WITH_LIST(my_list);
+			}
+			else
+			{
+static const char *const my_list[] =
+{DEFAULT, NULL};
+
+COMPLETE_WITH_LIST(my_list);
+			}
 		}
 	}
 
@@ -4636,6 +4653,42 @@ get_previous_words(int point, char **previous_words, int nwords)
 	}
 }
 
+static char *
+get_vartype(const char *varname)
+{
+	PQExpBufferData query_buffer;
+	char	*e_varname;
+	PGresult *result;
+	int	string_length;
+	static char resbuf[10];
+
+	initPQExpBuffer(query_buffer);
+
+	string_length = strlen(varname);
+	e_varname = pg_malloc(string_length * 2 + 1);
+	PQescapeString(e_varname, varname, string_length);
+
+	appendPQExpBuffer(query_buffer,
+		SELECT vartype FROM pg_settings WHERE pg_catalog.lower(name) = pg_catalog.lower('%s'),
+			 e_varname);
+
+	result = exec_query(query_buffer.data);
+	termPQExpBuffer(query_buffer);
+	free(e_varname);
+
+	resbuf[0] = '\0';
+
+	if (PQresultStatus(result) == PGRES_TUPLES_OK)
+	{
+		if (PQntuples(result)  0)
+			

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

2015-07-02 11:03 GMT+02:00 Heikki Linnakangas hlinn...@iki.fi:

 On 05/29/2015 10:41 AM, Pavel Stehule wrote:

 2015-05-29 9:28 GMT+02:00 Jeevan Chalke jeevan.cha...@gmail.com:

  I agree with Peter that We don't tab-complete everything we possibly
 could, but using tabs after SET ROLE TO  provides DEFAULT as an
 option
 which seems wrong.
 This patch adds list of roles over there, which I guess good to have than
 giving something unusual.

  ...

 But back to this topic. I am thinking so it is little bit different due
 fact so we support two very syntax for one feature. And looks little bit
 strange, so one way is supported by autocomplete and second not.


 Yeah, it's a bit strange. We have a specific autocomplete rule for SET
 ROLE, but SET ROLE TO is treated as a generic GUC. With your patch, we'd
 also lose the auto-completion to SET ROLE TO DEFAULT.

 I think we want to encourage people to use the SQL-standard syntax SET
 ROLE ... rather than the PostgreSQL-specific SET ROLE TO  On the
 whole, this just doesn't seem like much of an improvement. I'll mark this
 as 'rejected' in the commitfest app.


ok

Pavel


 PS. I note that the auto-completion for SET XXX TO ... is pretty lousy in
 general. We have rules for DateStyle, IntervalStyle, GEQO and search_path,
 but that's it. That could be expanded a lot. All enum-type GUCs could be
 handled with a single rule that queries pg_settings.enumvals, for example,
 and booleans would be easy too. But that's a different story.


 - Heikki

Re: [HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Heikki Linnakangas]

On 05/29/2015 10:41 AM, Pavel Stehule wrote:

2015-05-29 9:28 GMT+02:00 Jeevan Chalke jeevan.cha...@gmail.com:


I agree with Peter that We don't tab-complete everything we possibly
could, but using tabs after SET ROLE TO  provides DEFAULT as an option
which seems wrong.
This patch adds list of roles over there, which I guess good to have than
giving something unusual.


...

But back to this topic. I am thinking so it is little bit different due
fact so we support two very syntax for one feature. And looks little bit
strange, so one way is supported by autocomplete and second not.


Yeah, it's a bit strange. We have a specific autocomplete rule for SET 
ROLE, but SET ROLE TO is treated as a generic GUC. With your patch, 
we'd also lose the auto-completion to SET ROLE TO DEFAULT.


I think we want to encourage people to use the SQL-standard syntax SET 
ROLE ... rather than the PostgreSQL-specific SET ROLE TO  On the 
whole, this just doesn't seem like much of an improvement. I'll mark 
this as 'rejected' in the commitfest app.


PS. I note that the auto-completion for SET XXX TO ... is pretty lousy 
in general. We have rules for DateStyle, IntervalStyle, GEQO and 
search_path, but that's it. That could be expanded a lot. All enum-type 
GUCs could be handled with a single rule that queries 
pg_settings.enumvals, for example, and booleans would be easy too. But 
that's a different story.


- Heikki



--
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] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

2015-05-29 9:28 GMT+02:00 Jeevan Chalke jeevan.cha...@gmail.com:

 The following review has been posted through the commitfest application:
 make installcheck-world:  not tested
 Implements feature:   tested, passed
 Spec compliant:   tested, passed
 Documentation:not tested

 I agree with Peter that We don't tab-complete everything we possibly
 could, but using tabs after SET ROLE TO  provides DEFAULT as an option
 which seems wrong.
 This patch adds list of roles over there, which I guess good to have than
 giving something unusual.


Surely, we cannot to support tab-complete everywhere. But if we can do it
simply, we should to do it. Why:

1. It is good help for beginners

2. I am PostgreSQL lecture and evangelist in Czech Republic and Slovak
Republic. The tabcomplete possibility is surprisingly good factor for
accepting PostgreSQL concept, architecture, psql

But back to this topic. I am thinking so it is little bit different due
fact so we support two very syntax for one feature. And looks little bit
strange, so one way is supported by autocomplete and second not.



 I reviewed this straight forward patch and looks good to me.

 Since we might not want this, review is done and thus passing it to
 committer to decide.


ok



 The new status of this patch is: Ready for Committer


Thank you very much

Regards

Pavel



 --
 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] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Jeevan Chalke]

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:not tested

I agree with Peter that We don't tab-complete everything we possibly could, 
but using tabs after SET ROLE TO  provides DEFAULT as an option which seems 
wrong.
This patch adds list of roles over there, which I guess good to have than 
giving something unusual.

I reviewed this straight forward patch and looks good to me.

Since we might not want this, review is done and thus passing it to committer 
to decide.

The new status of this patch is: Ready for Committer


-- 
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] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Peter Eisentraut]

On 5/22/15 6:35 AM, Pavel Stehule wrote:
 we support SET ROLE name and SET ROLE TO name. Second form isn't
 supported by tabcomplete. Attached trivial patch fixes it.

We don't tab-complete everything we possibly could.  The documentation
only lists the first form, so I don't think we necessarily need to
complete the second form.



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

[HACKERS] psql tabcomplete - minor bugfix - tabcomplete for SET ROLE TO xxx

[Pavel Stehule]

Hi

we support SET ROLE name and SET ROLE TO name. Second form isn't supported
by tabcomplete. Attached trivial patch fixes it.

Regards

Pavel
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 750e29d..7110102
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*** psql_completion(const char *text, int st
*** 3552,3557 
--- 3552,3561 
   AND nspname not like 'pg\\_temp%%' 
   UNION SELECT 'DEFAULT' );
  		}
+ 		else if (pg_strcasecmp(prev2_wd, role) == 0)
+ 		{
+ 			COMPLETE_WITH_QUERY(Query_for_list_of_roles);
+ 		}
  		else
  		{
  			static const char *const my_list[] =

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