Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

[Piotr Stefaniak]

On 2017-05-21 03:00, Tom Lane wrote:
> I wrote:
>> Also, I found two places where an overlength comment line is simply busted
>> altogether --- notice that a character is missing at the split point:
> 
> I found the cause of that: you need to apply this patch:
> 
> --- freebsd_indent/pr_comment.c~  2017-05-17 14:59:31.548442801 -0400
> +++ freebsd_indent/pr_comment.c   2017-05-20 20:51:16.447332977 -0400
> @@ -344,8 +353,8 @@ pr_comment(void)
>   {
>   int len = strlen(t_ptr);
>  
> - CHECK_SIZE_COM(len);
> - memmove(e_com, t_ptr, len);
> + CHECK_SIZE_COM(len + 1);
> + memmove(e_com, t_ptr, len + 1);
>   last_bl = strpbrk(e_com, " \t");
>   e_com += len;
>   }
> 
> As the code stands, the strpbrk call is being applied to a
> not-null-terminated string and therefore is sometimes producing an
> insane value of last_bl, messing up decisions later in the comment.
> Having the memmove include the trailing \0 resolves that.

I have been analyzing this and came to different conclusions. Foremost,
a strpbrk() call like that finds the first occurrence of either space or
a tab, but last_bl means "last blank" - it's used for marking where to
wrap a comment line if it turns out to be too long. The previous coding
moved the character sequence byte after byte, updating last_bl every
time it was copying one of the two characters. I've rewritten that part as:
CHECK_SIZE_COM(len);
memmove(e_com, t_ptr, len);
-   last_bl = strpbrk(e_com, " \t");
e_com += len;
+   last_bl = NULL;
+   for (t_ptr = e_com - 1; t_ptr > e_com - len; t_ptr--)
+   if (*t_ptr == ' ' || *t_ptr == '\t') {
+   last_bl = t_ptr;
+   break;
+   }
}


But then I also started to wonder if there is any case when there's more
than one character to copy and I haven't found one yet. It looks like
} while (!memchr("*\n\r\b\t", *buf_ptr, 6) &&
(now_col <= adj_max_col || !last_bl));
guarantees that if we're past adj_max_col, it'll only be one non-space
character. But I'm not sure yet.



-- 
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] Variable substitution in psql backtick expansion

[Pavel Stehule]

Hi

2017-04-02 21:57 GMT+02:00 Fabien COELHO :

>
> More to the point, we already have that.  See c.h:
>>
>> #define CppAsString(identifier) #identifier
>> #define CppAsString2(x) CppAsString(x)
>>
>
> Thanks for the pointer. I grepped for them without success. Here is a v4.


I am sending a review of this patch.

This patch has trivial implementation - and there are not any objection to
used new variable names.

1. there was not any problem with patching, compiling
2. all regress tests passed
3. no problems with doc build

I'll mark this patch as ready for commiters

Regards

Pavel


>
> --
> Fabien

Re: [HACKERS] Race conditions with WAL sender PID lookups

[Erik Rijkers]

On 2017-05-20 14:40, Michael Paquier wrote:
On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada 
 wrote:

Also, as Horiguchi-san pointed out earlier, walreceiver seems need the
similar fix.


Actually, now that I look at it, ready_to_display should as well be
protected by the lock of the WAL receiver, so it is incorrectly placed
in walreceiver.h. As you are pointing out, pg_stat_get_wal_receiver()
is lazy as well, and that's new in 10, so we have an open item here
for both of them. And I am the author for both things. No issues
spotted in walreceiverfuncs.c after review.

I am adding an open item so as both issues are fixed in PG10. With the
WAL sender part, I think that this should be a group shot.

So what do you think about the attached?



[walsnd-pid-races-v3.patch]



With this patch on current master my logical replication tests 
(pgbench-over-logical-replication) run without errors for the first time 
in many days (even weeks).


I'll do still more and longer tests but I have gathered already a long 
streak of successful runs since you posted the patch so I am getting 
convinced this patch is solved the problem that I was experiencing.


Pity it didn't make the beta.


thanks,

Erik Rijkers



--
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] bumping HASH_VERSION to 3

[Bruce Momjian]

On Sun, May 21, 2017 at 09:11:29AM +0530, Amit Kapila wrote:
> On Sun, May 21, 2017 at 5:26 AM, Bruce Momjian  wrote:
> > On Fri, May 19, 2017 at 04:54:27PM -0400, Robert Haas wrote:
> >> On Tue, May 16, 2017 at 9:25 AM, Amit Kapila  
> >> wrote:
> >> > On Tue, May 16, 2017 at 5:14 PM, Robert Haas  
> >> > wrote:
> >> >> On Tue, May 16, 2017 at 7:31 AM, Amit Kapila  
> >> >> wrote:
> >> >>> I will send an updated patch once we agree on above points.
> >> >>
> >> >> Sounds good.
> >> >
> >> > Attached patch addresses all the comments as discussed.
> >>
> >> Committed.
> >
> > Just checking, but you reused some of my code from the 8.3-8.4 migration
> > in pg_upgrade that was removed a while back, rather than writing it from
> > scratch, right?
> >
> 
> Yes, I have adapted it to what is required for hash indexes, adjusted
> the code for v10 and there are some other minor changes in code.

Great, thanks.  It was hard to do originally so I am glad you could
reuse it.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] bumping HASH_VERSION to 3

[Amit Kapila]

On Sun, May 21, 2017 at 5:26 AM, Bruce Momjian  wrote:
> On Fri, May 19, 2017 at 04:54:27PM -0400, Robert Haas wrote:
>> On Tue, May 16, 2017 at 9:25 AM, Amit Kapila  wrote:
>> > On Tue, May 16, 2017 at 5:14 PM, Robert Haas  wrote:
>> >> On Tue, May 16, 2017 at 7:31 AM, Amit Kapila  
>> >> wrote:
>> >>> I will send an updated patch once we agree on above points.
>> >>
>> >> Sounds good.
>> >
>> > Attached patch addresses all the comments as discussed.
>>
>> Committed.
>
> Just checking, but you reused some of my code from the 8.3-8.4 migration
> in pg_upgrade that was removed a while back, rather than writing it from
> scratch, right?
>

Yes, I have adapted it to what is required for hash indexes, adjusted
the code for v10 and there are some other minor changes in code.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


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

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

[Tom Lane]

I wrote:
> Also, I found two places where an overlength comment line is simply busted
> altogether --- notice that a character is missing at the split point:

I found the cause of that: you need to apply this patch:

--- freebsd_indent/pr_comment.c~2017-05-17 14:59:31.548442801 -0400
+++ freebsd_indent/pr_comment.c 2017-05-20 20:51:16.447332977 -0400
@@ -344,8 +353,8 @@ pr_comment(void)
{
int len = strlen(t_ptr);
 
-   CHECK_SIZE_COM(len);
-   memmove(e_com, t_ptr, len);
+   CHECK_SIZE_COM(len + 1);
+   memmove(e_com, t_ptr, len + 1);
last_bl = strpbrk(e_com, " \t");
e_com += len;
}

As the code stands, the strpbrk call is being applied to a
not-null-terminated string and therefore is sometimes producing an
insane value of last_bl, messing up decisions later in the comment.
Having the memmove include the trailing \0 resolves that.

regards, tom lane


-- 
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] bumping HASH_VERSION to 3

[Bruce Momjian]

On Fri, May 19, 2017 at 04:54:27PM -0400, Robert Haas wrote:
> On Tue, May 16, 2017 at 9:25 AM, Amit Kapila  wrote:
> > On Tue, May 16, 2017 at 5:14 PM, Robert Haas  wrote:
> >> On Tue, May 16, 2017 at 7:31 AM, Amit Kapila  
> >> wrote:
> >>> I will send an updated patch once we agree on above points.
> >>
> >> Sounds good.
> >
> > Attached patch addresses all the comments as discussed.
> 
> Committed.

Just checking, but you reused some of my code from the 8.3-8.4 migration
in pg_upgrade that was removed a while back, rather than writing it from
scratch, right?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +


-- 
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] [Q] When should I use reg* types instead of oid in the system catalog?

[Tom Lane]

"MauMau"  writes:
> Both pg_aggregate.aggfnoid and pg_trigger.tgfoid references
> pg_proc.oid, but the data types of them are regproc and oid
> respectively.  Is there any criterion on when to which in the system
> catalog?  Is the regproc choice just for readability of the catalog
> query output?  Should pg_trigger.tgfoid also have been changed to
> regproc?

It's probably mostly historical accident :-(.  There have been suggestions
before to convert more system catalog columns to regfoo types, but there's
serious stumbling blocks in the way:

* Almost certainly, such conversions would break client code that's
expecting to see simple numeric OIDs in those columns.  pg_dump would
certainly be broken for example.  It'd be within our power to fix pg_dump,
but there would be more pushback about it from places like pgAdmin and
the JDBC driver.

* It's not actually that helpful, at least not without some fresh ideas
about type resolution.  For example, you'd think that if we changed
pg_attribute.attrelid to regclass then you could do something like
select attname from pg_attribute where attrelid = 'mytable'
and it'd work.  But it won't, as can be shown by trying it on one
of the existing regproc columns:

=# select * from pg_aggregate where aggfinalfn = 'interval_avg';
ERROR:  invalid input syntax for type oid: "interval_avg"
LINE 1: select * from pg_aggregate where aggfinalfn = 'interval_avg'...
  ^

The reason for that is that the "=" operator is resolved as oideq,
there not being a separate set of operators for each OID-alias type.
Now that's pretty confusing considering that the printed values for
relevant entries in that column look exactly like 'interval_avg',
but there it is.

So, pending some ideas about resolving those issues, there hasn't
been much eagerness to change catalog columns that are currently
plain "oid".

regards, tom lane


-- 
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] proposal psql \gdesc

[Pavel Stehule]

2017-05-20 22:26 GMT+02:00 Fabien COELHO :

>
> Hello Pavel,
>
>  - Maybe tests could also exercise unnamed columns, eg:
>>> SELECT 1, 2, 3 \gdesc \g
>>>
>>
>> done
>>
>
> Can't see it. No big deal, but if you put it it did not get through, and
> there is a warning with git apply on the very last line of the patch which
> may be linked to that:
>
>   psql-gdesc-05.patch:328: new blank line at EOF.
>   +
>   warning: 1 line adds whitespace errors.
>

looks like pg_regress issue - more result files has extra blank line on
end. I am able to clean it only with \r on end of sql script - not sure
what is more worst - unrelated \command or this warning



>
> ... especially as the two last tests are nearly the same now. I'm fine
> with a "one line" test, could be with some unnamed columns so that it is
> more different?


ok - look on  new version, please


>
>
> --
> Fabien.
>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 3b86612..2e46f4c 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1957,6 +1957,16 @@ Tue Oct 26 21:40:57 CEST 1999
 
 
   
+\gdesc
+
+
+Show the description of the result of the current query buffer without
+actually executing it, by considering it a prepared statement.
+
+
+  
+
+  
 \gexec
 
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b3263a9..a1c8e1d 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -88,6 +88,7 @@ static backslashResult exec_command_errverbose(PsqlScanState scan_state, bool ac
 static backslashResult exec_command_f(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_g(PsqlScanState scan_state, bool active_branch,
 			   const char *cmd);
+static backslashResult exec_command_gdesc(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gexec(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gset(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_help(PsqlScanState scan_state, bool active_branch);
@@ -337,6 +338,8 @@ exec_command(const char *cmd,
 		status = exec_command_f(scan_state, active_branch);
 	else if (strcmp(cmd, "g") == 0 || strcmp(cmd, "gx") == 0)
 		status = exec_command_g(scan_state, active_branch, cmd);
+	else if (strcmp(cmd, "gdesc") == 0)
+		status = exec_command_gdesc(scan_state, active_branch);
 	else if (strcmp(cmd, "gexec") == 0)
 		status = exec_command_gexec(scan_state, active_branch);
 	else if (strcmp(cmd, "gset") == 0)
@@ -1328,6 +1331,25 @@ exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
 }
 
 /*
+ * \gdesc -- describe query result
+ */
+static backslashResult
+exec_command_gdesc(PsqlScanState scan_state, bool active_branch)
+{
+	backslashResult status = PSQL_CMD_SKIP_LINE;
+
+	if (active_branch)
+	{
+		pset.gdesc_flag = true;
+		status = PSQL_CMD_SEND;
+	}
+	else
+		ignore_slash_filepipe(scan_state);
+
+	return status;
+}
+
+/*
  * \gexec -- send query and execute each field of result
  */
 static backslashResult
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a2f1259..d5fd8de 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1323,7 +1323,88 @@ SendQuery(const char *query)
 		}
 	}
 
-	if (pset.fetch_count <= 0 || pset.gexec_flag ||
+	if (pset.gdesc_flag)
+	{
+		/*
+		 * Unnamed prepared statement is used. Is not possible to
+		 * create any unnamed prepared statement from psql user space,
+		 * so there should not be any conflict. In this moment is not
+		 * possible to deallocate this prepared statement, so it should
+		 * to live to end of session or to another \gdesc call.
+		 */
+		results = PQprepare(pset.db, "", query, 0, NULL);
+		if (PQresultStatus(results) != PGRES_COMMAND_OK)
+		{
+			psql_error("%s", PQerrorMessage(pset.db));
+			ClearOrSaveResult(results);
+			ResetCancelConn();
+			goto sendquery_cleanup;
+		}
+		PQclear(results);
+
+		results = PQdescribePrepared(pset.db, "");
+		OK = ProcessResult();
+		if (OK && results)
+		{
+			if (PQnfields(results) > 0)
+			{
+PQExpBufferData		buf;
+int		i;
+
+initPQExpBuffer();
+
+printfPQExpBuffer(,
+	  "SELECT name AS \"%s\", pg_catalog.format_type(tp, tpm) AS \"%s\"\n"
+	  "FROM (VALUES",
+	  gettext_noop("Name"),
+	  gettext_noop("Type"));
+
+for(i = 0; i< PQnfields(results); i++)
+{
+	char	*name;
+	char	*escname;
+	size_t		name_length;
+
+	if (i > 0)
+		appendPQExpBufferStr(, ",");
+
+	name = PQfname(results, i);
+	name_length = strlen(name);
+	escname = PQescapeLiteral(pset.db, name, name_length);
+
+	if (escname == NULL)
+	{
+		psql_error("%s", PQerrorMessage(pset.db));
+		PQclear(results);
+		termPQExpBuffer();
+		goto sendquery_cleanup;
+	}
+
+	appendPQExpBuffer(, "(%s, %d, 

[HACKERS] [Q] When should I use reg* types instead of oid in the system catalog?

[MauMau]

Hello,


Both pg_aggregate.aggfnoid and pg_trigger.tgfoid references
pg_proc.oid, but the data types of them are regproc and oid
respectively.  Is there any criterion on when to which in the system
catalog?  Is the regproc choice just for readability of the catalog
query output?  Should pg_trigger.tgfoid also have been changed to
regproc?

Regards
MauMau



-- 
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] proposal psql \gdesc

[Fabien COELHO]

Hello Pavel,


 - Maybe tests could also exercise unnamed columns, eg:
SELECT 1, 2, 3 \gdesc \g


done


Can't see it. No big deal, but if you put it it did not get through, and 
there is a warning with git apply on the very last line of the patch which 
may be linked to that:


  psql-gdesc-05.patch:328: new blank line at EOF.
  +
  warning: 1 line adds whitespace errors.

... especially as the two last tests are nearly the same now. I'm fine 
with a "one line" test, could be with some unnamed columns so that it is 
more different?


--
Fabien.


--
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] Regression in join selectivity estimations when using foreign keys

[Tom Lane]

David Rowley  writes:
> I've been analyzing a reported regression case between a 9.5 plan and
> a 9.6 plan. I tracked this down to the foreign key join selectivity
> code, specifically the use_smallest_selectivity code which is applied
> to outer joins where the referenced table is on the outer side of the
> join.
> ...
> I've attached fixes, based on master, for both of these cases.

I'm entirely unconvinced by this patch --- it seems to simply be throwing
away a lot of logic.  Notably it lobotomizes the FK code altogether for
semi/antijoin cases, but you've not shown any example that even involves
such joins, so what's the argument for doing that?  Also, the reason
we had the use_smallest_selectivity code in the first place was that we
didn't believe the FK-based selectivity could be applied safely to
outer-join cases, so simply deciding that it's OK to apply it after all
seems insufficiently justified.

Or in short, exhibiting one counterexample to the existing code is not
a sufficient argument for changing things this much.  You need to give
an argument why this is the right thing to do instead.

Stepping back a bit, it seems like the core thing the FK selectivity code
was meant to do was to prevent us from underestimating selectivity of
multiple-clause join conditions through a false assumption of clause
independence.  The problem with the use_smallest_selectivity code is that
it's overestimating selectivity, but that doesn't mean that we want to go
all the way back to the old way of doing things.  I wonder if we could get
any traction in these dubious cases by computing the product, instead of
minimum, of the clause selectivities (thus matching the old estimate) and
then taking the greater of that and the FK-based selectivity.

regards, tom lane


-- 
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] Allowing dash character in LTREE

[David G. Johnston]

On Sat, May 20, 2017 at 11:26 AM, Cyril Auburtin 
wrote:

> Ah sorry, first time, I thought it didn't pass
>

​You should check our excellent online mailing list archives before
re-sending.

https://www.postgresql.org/list/

David J.
​

Re: [HACKERS] Allowing dash character in LTREE

[Cyril Auburtin]

Ah sorry, first time, I thought it didn't pass

It's to support multiple ids (base64 ones are frequent) (or uuid's) in a
path. I think it's more practical than an array, for searching and sorting

But I understand that LTREE's main purpose is probably more for words
'breadcrumbs' etc..

2017-05-20 18:31 GMT+02:00 David G. Johnston :

> On Sat, May 20, 2017 at 1:27 AM, Cyril Auburtin 
> wrote:
>
>> It could be useful to allow the `-` char in allowed LTREE label
>> characters  (currently a-zA-Z0-9_ https://www.postgre
>> sql.org/docs/9.6/static/ltree.html)
>>
>> The reason is to allow to use more easily base64 ids, (like
>> https://github.com/dylang/shortid, ...), uuid's too in labels
>>
>> `-` is also not used as a query operator as a plus
>>
>>
>> source: https://doxygen.postgresql.org/ltree_8h.html#a1cb5d5
>> dd0c364d720854e8a250967dd1
>>
>
> ​This is the third time you've posted this suggestion.  Its been seen by
> the people you are sending it to.
>
> I don't follow how this helps for UUID - they are not usually generated
> hierarchically​...
>
> I'll +1 the thought, but I won't be providing a patch.
>
> David J.
>
>

Re: [HACKERS] proposal psql \gdesc

[Pavel Stehule]

Hi

2017-05-20 9:15 GMT+02:00 Fabien COELHO :

>
> Hello Pavel,
>
> I am sending a variant with message instead empty result.
>>
>
> Thanks. Patch looks good, applies, make check ok, code is neat.
>
> Personally I prefer empty result instead message.
>>
>
> Hmm. I think that this version is less likely to raise questions from
> users, especially compared to having a somehow correct but strangely
> looking description.
>
> It is hard to choose some good text of this message. Empty result is just
>> empty result for all cases.
>>
>
we will see


>
> I'd suggest a very minor change: "No columns or command has no result"
> (not -> no). If some English native speaker has a better suggestion, fine
> with me.
>

changed


>
> Another good point of this version is that the type name query is
> simplified because it does not need to handle an empty result, thus the
> code is easier to understand.
>
> A few other suggestions:
>
>  - could you update the comment on the type name query?
>Maybe the comment can be simply removed?
>
>
removed


>  - I'm wondering whether the Name & Type columns names should be
>translatable. What do you think?
>

good idea - done


>
>  - Maybe tests could also exercise unnamed columns, eg:
> SELECT 1, 2, 3 \gdesc \g


done

Regards

Pavel


>
>
> --
> Fabien.
>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 3b86612..2e46f4c 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1957,6 +1957,16 @@ Tue Oct 26 21:40:57 CEST 1999
 
 
   
+\gdesc
+
+
+Show the description of the result of the current query buffer without
+actually executing it, by considering it a prepared statement.
+
+
+  
+
+  
 \gexec
 
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b3263a9..a1c8e1d 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -88,6 +88,7 @@ static backslashResult exec_command_errverbose(PsqlScanState scan_state, bool ac
 static backslashResult exec_command_f(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_g(PsqlScanState scan_state, bool active_branch,
 			   const char *cmd);
+static backslashResult exec_command_gdesc(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gexec(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gset(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_help(PsqlScanState scan_state, bool active_branch);
@@ -337,6 +338,8 @@ exec_command(const char *cmd,
 		status = exec_command_f(scan_state, active_branch);
 	else if (strcmp(cmd, "g") == 0 || strcmp(cmd, "gx") == 0)
 		status = exec_command_g(scan_state, active_branch, cmd);
+	else if (strcmp(cmd, "gdesc") == 0)
+		status = exec_command_gdesc(scan_state, active_branch);
 	else if (strcmp(cmd, "gexec") == 0)
 		status = exec_command_gexec(scan_state, active_branch);
 	else if (strcmp(cmd, "gset") == 0)
@@ -1328,6 +1331,25 @@ exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
 }
 
 /*
+ * \gdesc -- describe query result
+ */
+static backslashResult
+exec_command_gdesc(PsqlScanState scan_state, bool active_branch)
+{
+	backslashResult status = PSQL_CMD_SKIP_LINE;
+
+	if (active_branch)
+	{
+		pset.gdesc_flag = true;
+		status = PSQL_CMD_SEND;
+	}
+	else
+		ignore_slash_filepipe(scan_state);
+
+	return status;
+}
+
+/*
  * \gexec -- send query and execute each field of result
  */
 static backslashResult
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a2f1259..d5fd8de 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1323,7 +1323,88 @@ SendQuery(const char *query)
 		}
 	}
 
-	if (pset.fetch_count <= 0 || pset.gexec_flag ||
+	if (pset.gdesc_flag)
+	{
+		/*
+		 * Unnamed prepared statement is used. Is not possible to
+		 * create any unnamed prepared statement from psql user space,
+		 * so there should not be any conflict. In this moment is not
+		 * possible to deallocate this prepared statement, so it should
+		 * to live to end of session or to another \gdesc call.
+		 */
+		results = PQprepare(pset.db, "", query, 0, NULL);
+		if (PQresultStatus(results) != PGRES_COMMAND_OK)
+		{
+			psql_error("%s", PQerrorMessage(pset.db));
+			ClearOrSaveResult(results);
+			ResetCancelConn();
+			goto sendquery_cleanup;
+		}
+		PQclear(results);
+
+		results = PQdescribePrepared(pset.db, "");
+		OK = ProcessResult();
+		if (OK && results)
+		{
+			if (PQnfields(results) > 0)
+			{
+PQExpBufferData		buf;
+int		i;
+
+initPQExpBuffer();
+
+printfPQExpBuffer(,
+	  "SELECT name AS \"%s\", pg_catalog.format_type(tp, tpm) AS \"%s\"\n"
+	  "FROM (VALUES",
+	  gettext_noop("Name"),
+	  gettext_noop("Type"));
+
+for(i = 0; i< PQnfields(results); i++)
+{
+	char	*name;
+	char	*escname;
+		

[HACKERS] Tightening isspace() tests where behavior should match SQL parser

[Tom Lane]

The proximate cause of bug #14662,
https://www.postgresql.org/message-id/flat/20170519162316.29945.5021%40wrigleys.postgresql.org
appears to be that SplitIdentifierString's isspace() checks are
identifying some bytes of a multibyte character as spaces.  It's not
quite clear to me whether there's an encoding configuration problem
involved in this specific report, but in any case feeding individual
bytes of a multibyte character to  functions is highly dubious.
The best you can say about that is that the behavior will be
platform-dependent.

I think that the easiest way to resolve this is to replace the isspace()
calls with scanner_isspace(), on the grounds that we are trying to parse
identifiers the same way that the core SQL lexer would, and therefore we
should use its definition of whitespace.

I went looking through all our other calls of isspace() to see if we
have the same problem anywhere else, and identified these functions
as being at risk:

parse_ident()
regproc.c's parseNameAndArgTypes (for regprocedurein and siblings)
SplitIdentifierString()
SplitDirectoriesString()

In all four cases, we must allow for non-ASCII input and it's
arguable that correct behavior is to match the core lexer,
so I propose replacing isspace() with scanner_isspace() in
these places.

There are quite a lot more isspace() calls than that of course,
but the rest of them seem probably all right to me.  As an
example, I don't see a need to make float8in() stricter about
what it will allow as trailing whitespace.  We're not expecting
any non-ASCII input really, and if we do see multibyte characters
and all the bytes manage to pass isspace(), not much harm is done.

Attached is a proposed patch.  I'm vacillating on whether to
back-patch this --- it will fix a reported bug, but it seems
at least somewhat conceivable that it will also break cases
that were working acceptably before.  Thoughts?

regards, tom lane

diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 7dcecb2..9cc0b08 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -770,7 +770,7 @@ parse_ident(PG_FUNCTION_ARGS)
 	nextp = qualname_str;
 
 	/* skip leading whitespace */
-	while (isspace((unsigned char) *nextp))
+	while (scanner_isspace(*nextp))
 		nextp++;
 
 	for (;;)
@@ -858,14 +858,14 @@ parse_ident(PG_FUNCTION_ARGS)
 text_to_cstring(qualname;
 		}
 
-		while (isspace((unsigned char) *nextp))
+		while (scanner_isspace(*nextp))
 			nextp++;
 
 		if (*nextp == '.')
 		{
 			after_dot = true;
 			nextp++;
-			while (isspace((unsigned char) *nextp))
+			while (scanner_isspace(*nextp))
 nextp++;
 		}
 		else if (*nextp == '\0')
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index a90452c..ba879e8 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -32,6 +32,7 @@
 #include "lib/stringinfo.h"
 #include "miscadmin.h"
 #include "parser/parse_type.h"
+#include "parser/scansup.h"
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/syscache.h"
@@ -1769,7 +1770,7 @@ parseNameAndArgTypes(const char *string, bool allowNone, List **names,
 	ptr2 = ptr + strlen(ptr);
 	while (--ptr2 > ptr)
 	{
-		if (!isspace((unsigned char) *ptr2))
+		if (!scanner_isspace(*ptr2))
 			break;
 	}
 	if (*ptr2 != ')')
@@ -1786,7 +1787,7 @@ parseNameAndArgTypes(const char *string, bool allowNone, List **names,
 	for (;;)
 	{
 		/* allow leading whitespace */
-		while (isspace((unsigned char) *ptr))
+		while (scanner_isspace(*ptr))
 			ptr++;
 		if (*ptr == '\0')
 		{
@@ -1842,7 +1843,7 @@ parseNameAndArgTypes(const char *string, bool allowNone, List **names,
 		/* Lop off trailing whitespace */
 		while (--ptr2 >= typename)
 		{
-			if (!isspace((unsigned char) *ptr2))
+			if (!scanner_isspace(*ptr2))
 break;
 			*ptr2 = '\0';
 		}
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index be399f4..a0dd391 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -3252,7 +3252,7 @@ SplitIdentifierString(char *rawstring, char separator,
 
 	*namelist = NIL;
 
-	while (isspace((unsigned char) *nextp))
+	while (scanner_isspace(*nextp))
 		nextp++;/* skip leading whitespace */
 
 	if (*nextp == '\0')
@@ -3290,7 +3290,7 @@ SplitIdentifierString(char *rawstring, char separator,
 
 			curname = nextp;
 			while (*nextp && *nextp != separator &&
-   !isspace((unsigned char) *nextp))
+   !scanner_isspace(*nextp))
 nextp++;
 			endp = nextp;
 			if (curname == nextp)
@@ -3312,13 +3312,13 @@ SplitIdentifierString(char *rawstring, char separator,
 			pfree(downname);
 		}
 
-		while (isspace((unsigned char) *nextp))
+		while (scanner_isspace(*nextp))
 			nextp++;			/* skip trailing whitespace */
 
 		if (*nextp == separator)
 		{
 			nextp++;
-			while (isspace((unsigned char) *nextp))
+			while (scanner_isspace(*nextp))
 nextp++;		/* skip leading 

Re: [HACKERS] Making replication commands case-insensitive

[Andres Freund]

Hi,

On 2017-05-20 21:19:10 +0900, Michael Paquier wrote:
> $subject has been raised in a recent thread here:
> https://www.postgresql.org/message-id/CAB7nPqTmym5t-X6hvMF_P-KRc=ndXtbQCTiU=nhs_jvl7x1...@mail.gmail.com
> 
> The idea is to make the replication protocol a bit more flexible, in a
> way similar to what 5c837dd has done, but for repl_scanner.l. This
> will also allow to avoid any problems like what has been fixed in
> aa41bc7 where the SHOW commands used in libpq have to be capitalized.
> Personally, I have pested about the lack of flexibility a couple of
> times when running tests using psql..

-1.  This solution is too bad for maintainability, and given the
replication protocol is not for interactive use I don't see a
corresponding benefit.

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] Making replication commands case-insensitive

[Magnus Hagander]

On Sat, May 20, 2017 at 2:19 PM, Michael Paquier 
wrote:

> Hi all,
>
> $subject has been raised in a recent thread here:
> https://www.postgresql.org/message-id/CAB7nPqTmym5t-
> X6hvMF_P-KRc=ndXtbQCTiU=nhs_jvl7x1...@mail.gmail.com
>
> The idea is to make the replication protocol a bit more flexible, in a
> way similar to what 5c837dd has done, but for repl_scanner.l. This
> will also allow to avoid any problems like what has been fixed in
> aa41bc7 where the SHOW commands used in libpq have to be capitalized.
> Personally, I have pested about the lack of flexibility a couple of
> times when running tests using psql..
>
> I am parking that in the next CF.
>

Given that the protocol really isn't intended for "manual consumption", do
we really want that? Not that it adds a lot of complexity, but still. It
certainly makes the code completely unreadable. And since any program using
it should figure out pretty quickly that it's not working if they us the
wrong casing...

-- 
 Magnus Hagander
 Me: https://www.hagander.net/ 
 Work: https://www.redpill-linpro.com/ 

Re: [HACKERS] Allowing dash character in LTREE

[David G. Johnston]

On Sat, May 20, 2017 at 1:27 AM, Cyril Auburtin 
wrote:

> It could be useful to allow the `-` char in allowed LTREE label characters  
> (currently
> a-zA-Z0-9_ https://www.postgresql.org/docs/9.6/static/ltree.html)
>
> The reason is to allow to use more easily base64 ids, (like
> https://github.com/dylang/shortid, ...), uuid's too in labels
>
> `-` is also not used as a query operator as a plus
>
>
> source: https://doxygen.postgresql.org/ltree_8h.html#a
> 1cb5d5dd0c364d720854e8a250967dd1
>

​This is the third time you've posted this suggestion.  Its been seen by
the people you are sending it to.

I don't follow how this helps for UUID - they are not usually generated
hierarchically​...

I'll +1 the thought, but I won't be providing a patch.

David J.

Re: [HACKERS] Race conditions with WAL sender PID lookups

[Michael Paquier]

On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada  wrote:
> Also, as Horiguchi-san pointed out earlier, walreceiver seems need the
> similar fix.

Actually, now that I look at it, ready_to_display should as well be
protected by the lock of the WAL receiver, so it is incorrectly placed
in walreceiver.h. As you are pointing out, pg_stat_get_wal_receiver()
is lazy as well, and that's new in 10, so we have an open item here
for both of them. And I am the author for both things. No issues
spotted in walreceiverfuncs.c after review.

I am adding an open item so as both issues are fixed in PG10. With the
WAL sender part, I think that this should be a group shot.

So what do you think about the attached?
-- 
Michael
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index ad213fc454..5ab5e95fa9 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -409,15 +409,23 @@ void
 SyncRepReleaseWaiters(void)
 {
 	volatile WalSndCtlData *walsndctl = WalSndCtl;
+	WalSnd	   *walsnd = MyWalSnd;
 	XLogRecPtr	writePtr;
 	XLogRecPtr	flushPtr;
 	XLogRecPtr	applyPtr;
+	XLogRecPtr	flush;
+	WalSndState	state;
 	bool		got_recptr;
 	bool		am_sync;
 	int			numwrite = 0;
 	int			numflush = 0;
 	int			numapply = 0;
 
+	SpinLockAcquire(>mutex);
+	flush = walsnd->flush;
+	state = walsnd->state;
+	SpinLockRelease(>mutex);
+
 	/*
 	 * If this WALSender is serving a standby that is not on the list of
 	 * potential sync standbys then we have nothing to do. If we are still
@@ -425,8 +433,8 @@ SyncRepReleaseWaiters(void)
 	 * still invalid, then leave quickly also.
 	 */
 	if (MyWalSnd->sync_standby_priority == 0 ||
-		MyWalSnd->state < WALSNDSTATE_STREAMING ||
-		XLogRecPtrIsInvalid(MyWalSnd->flush))
+		state < WALSNDSTATE_STREAMING ||
+		XLogRecPtrIsInvalid(flush))
 	{
 		announce_next_takeover = true;
 		return;
@@ -711,14 +719,24 @@ SyncRepGetSyncStandbysQuorum(bool *am_sync)
 
 	for (i = 0; i < max_wal_senders; i++)
 	{
+		XLogRecPtr	flush;
+		WalSndState	state;
+		int			pid;
+
 		walsnd = >walsnds[i];
 
+		SpinLockAcquire(>mutex);
+		pid = walsnd->pid;
+		flush = walsnd->flush;
+		state = walsnd->state;
+		SpinLockRelease(>mutex);
+
 		/* Must be active */
-		if (walsnd->pid == 0)
+		if (pid == 0)
 			continue;
 
 		/* Must be streaming */
-		if (walsnd->state != WALSNDSTATE_STREAMING)
+		if (state != WALSNDSTATE_STREAMING)
 			continue;
 
 		/* Must be synchronous */
@@ -726,7 +744,7 @@ SyncRepGetSyncStandbysQuorum(bool *am_sync)
 			continue;
 
 		/* Must have a valid flush position */
-		if (XLogRecPtrIsInvalid(walsnd->flush))
+		if (XLogRecPtrIsInvalid(flush))
 			continue;
 
 		/*
@@ -780,14 +798,24 @@ SyncRepGetSyncStandbysPriority(bool *am_sync)
 	 */
 	for (i = 0; i < max_wal_senders; i++)
 	{
+		XLogRecPtr	flush;
+		WalSndState	state;
+		int			pid;
+
 		walsnd = >walsnds[i];
 
+		SpinLockAcquire(>mutex);
+		pid = walsnd->pid;
+		flush = walsnd->flush;
+		state = walsnd->state;
+		SpinLockRelease(>mutex);
+
 		/* Must be active */
-		if (walsnd->pid == 0)
+		if (pid == 0)
 			continue;
 
 		/* Must be streaming */
-		if (walsnd->state != WALSNDSTATE_STREAMING)
+		if (state != WALSNDSTATE_STREAMING)
 			continue;
 
 		/* Must be synchronous */
@@ -796,7 +824,7 @@ SyncRepGetSyncStandbysPriority(bool *am_sync)
 			continue;
 
 		/* Must have a valid flush position */
-		if (XLogRecPtrIsInvalid(walsnd->flush))
+		if (XLogRecPtrIsInvalid(flush))
 			continue;
 
 		/*
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 2723612718..25f12e0706 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1392,23 +1392,13 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
 	TimestampTz latest_end_time;
 	char	   *slotname;
 	char	   *conninfo;
-
-	/*
-	 * No WAL receiver (or not ready yet), just return a tuple with NULL
-	 * values
-	 */
-	if (walrcv->pid == 0 || !walrcv->ready_to_display)
-		PG_RETURN_NULL();
-
-	/* determine result type */
-	if (get_call_result_type(fcinfo, NULL, ) != TYPEFUNC_COMPOSITE)
-		elog(ERROR, "return type must be a row type");
-
-	values = palloc0(sizeof(Datum) * tupdesc->natts);
-	nulls = palloc0(sizeof(bool) * tupdesc->natts);
+	int			pid;
+	bool		ready_to_display;
 
 	/* Take a lock to ensure value consistency */
 	SpinLockAcquire(>mutex);
+	pid = walrcv->pid;
+	ready_to_display = walrcv->ready_to_display;
 	state = walrcv->walRcvState;
 	receive_start_lsn = walrcv->receiveStart;
 	receive_start_tli = walrcv->receiveStartTLI;
@@ -1422,8 +1412,22 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
 	conninfo = pstrdup(walrcv->conninfo);
 	SpinLockRelease(>mutex);
 
+	/*
+	 * No WAL receiver (or not ready yet), just return a tuple with NULL
+	 * values
+	 */
+	if (pid == 0 || !ready_to_display)
+		PG_RETURN_NULL();
+
+	/* determine result type */
+	if (get_call_result_type(fcinfo, NULL, ) != TYPEFUNC_COMPOSITE)
+		elog(ERROR, "return type must be a row 

Re: [HACKERS] OK, so culicidae is *still* broken

[Noah Misch]

On Sat, Apr 15, 2017 at 02:30:18PM -0700, Andres Freund wrote:
> On 2017-04-15 17:24:54 -0400, Tom Lane wrote:
> > Andres Freund  writes:
> > > On 2017-04-15 17:09:38 -0400, Tom Lane wrote:
> > >> Why doesn't Windows' ability to map the segment into the new process
> > >> before it executes take care of that?
> > 
> > > Because of ASLR of the main executable (i.e. something like PIE).
> > 
> > Not following.  Are you saying that the main executable gets mapped into
> > the process address space immediately, but shared libraries are not?

At the time of the pgwin32_ReserveSharedMemoryRegion() call, the child process
contains only ntdll.dll and the executable.

> Without PIE/ASLR we can somewhat rely on pgwin32_ReserveSharedMemoryRegion
> to find the space that PGSharedMemoryCreate allocated still unoccupied.

I've never had access to a Windows system that can reproduce the fork
failures.  My best theory is that antivirus or similar software injects an
additional DLL at that early stage.

> > I wonder whether we could work around that by just destroying the created
> > process and trying again if we get a collision.  It'd be a tad
> > inefficient, but hopefully collisions wouldn't happen often enough to be a
> > big problem.
> 
> That might work, although it's obviously not pretty.

I didn't like that idea when Michael proposed it in 2015.  Since disabling
ASLR on the exe proved insufficient, I do like it now.  It degrades nicely; if
something raises the collision rate from 1% to 10%, that just looks like fork
latency degrading.

> We could also just
> default to some out-of-the-way address for MapViewOfFileEx, that might
> also work.

Dave Vitek proposed that:
https://www.postgresql.org/message-id/flat/5046CAEB.4010600%40grammatech.com

I estimate more risk than retrying forks.  There's no guarantee that a fixed
address helpful today won't make the collisions worse after the next Windows
security patch.


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

[HACKERS] Making replication commands case-insensitive

[Michael Paquier]

Hi all,

$subject has been raised in a recent thread here:
https://www.postgresql.org/message-id/CAB7nPqTmym5t-X6hvMF_P-KRc=ndXtbQCTiU=nhs_jvl7x1...@mail.gmail.com

The idea is to make the replication protocol a bit more flexible, in a
way similar to what 5c837dd has done, but for repl_scanner.l. This
will also allow to avoid any problems like what has been fixed in
aa41bc7 where the SHOW commands used in libpq have to be capitalized.
Personally, I have pested about the lack of flexibility a couple of
times when running tests using psql..

I am parking that in the next CF.
Thanks,
-- 
Michael
diff --git a/src/backend/replication/repl_scanner.l b/src/backend/replication/repl_scanner.l
index 52ae7b343f..2bceff0b55 100644
--- a/src/backend/replication/repl_scanner.l
+++ b/src/backend/replication/repl_scanner.l
@@ -80,29 +80,31 @@ identifier		{ident_start}{ident_cont}*
 
 %%
 
-BASE_BACKUP			{ return K_BASE_BACKUP; }
-FAST			{ return K_FAST; }
-IDENTIFY_SYSTEM		{ return K_IDENTIFY_SYSTEM; }
-SHOW		{ return K_SHOW; }
-LABEL			{ return K_LABEL; }
-NOWAIT			{ return K_NOWAIT; }
-PROGRESS			{ return K_PROGRESS; }
-MAX_RATE		{ return K_MAX_RATE; }
-WAL			{ return K_WAL; }
-TABLESPACE_MAP			{ return K_TABLESPACE_MAP; }
-TIMELINE			{ return K_TIMELINE; }
-START_REPLICATION	{ return K_START_REPLICATION; }
-CREATE_REPLICATION_SLOT		{ return K_CREATE_REPLICATION_SLOT; }
-DROP_REPLICATION_SLOT		{ return K_DROP_REPLICATION_SLOT; }
-TIMELINE_HISTORY	{ return K_TIMELINE_HISTORY; }
-PHYSICAL			{ return K_PHYSICAL; }
-RESERVE_WAL			{ return K_RESERVE_WAL; }
-LOGICAL{ return K_LOGICAL; }
-SLOT{ return K_SLOT; }
-TEMPORARY			{ return K_TEMPORARY; }
-EXPORT_SNAPSHOT		{ return K_EXPORT_SNAPSHOT; }
-NOEXPORT_SNAPSHOT	{ return K_NOEXPORT_SNAPSHOT; }
-USE_SNAPSHOT		{ return K_USE_SNAPSHOT; }
+	/* brute-force case insensitivity is safer than relying on flex -i */
+
+[Bb][Aa][Ss][Ee]_[Bb][Aa][Cc][Kk][Uu][Pp]	{ return K_BASE_BACKUP; }
+[Ff][Aa][Ss][Tt]	{ return K_FAST; }
+[Ii][Dd][Ee][Nn][Tt][Ii][Ff][Yy]_[Ss][Yy][Ss][Tt][Ee][Mm]	{ return K_IDENTIFY_SYSTEM; }
+[Ss][Hh][Oo][Ww]	{ return K_SHOW; }
+[Ll][Aa][Bb][Ee][Ll]	{ return K_LABEL; }
+[Nn][Oo][Ww][Aa][Ii][Tt]	{ return K_NOWAIT; }
+[Pp][Rr][Oo][Gg][Rr][Ee][Ss][Ss]	{ return K_PROGRESS; }
+[Mm][Aa][Xx]_[Rr][Aa][Tt][Ee]		{ return K_MAX_RATE; }
+[Ww][Aa][Ll]		{ return K_WAL; }
+[Tt][Aa][Bb][Ll][Ee][Ss][Pp][Aa][Cc][Ee]_[Mm][Aa][Pp]	{ return K_TABLESPACE_MAP; }
+[Tt][Ii][Mm][Ee][Ll][Ii][Nn][Ee]	{ return K_TIMELINE; }
+[Ss][Tt][Aa][Rr][Tt]_[Rr][Ee][Pp][Ll][Ii][Cc][Aa][Tt][Ii][Oo][Nn]	{ return K_START_REPLICATION; }
+[Cc][Rr][Ee][Aa][Tt][Ee]_[Rr][Ee][Pp][Ll][Ii][Cc][Aa][Tt][Ii][Oo][Nn]_[Ss][Ll][Oo][Tt]	{ return K_CREATE_REPLICATION_SLOT; }
+[Dd][Rr][Oo][Pp]_[Rr][Ee][Pp][Ll][Ii][Cc][Aa][Tt][Ii][Oo][Nn]_[Ss][Ll][Oo][Tt]	{ return K_DROP_REPLICATION_SLOT; }
+[Tt][Ii][Mm][Ee][Ll][Ii][Nn][Ee]_[Hh][Ii][Ss][Tt][Oo][Rr][Yy]	{ return K_TIMELINE_HISTORY; }
+[Pp][Hh][Yy][Ss][Ii][Cc][Aa][Ll]	{ return K_PHYSICAL; }
+[Rr][Ee][Ss][Ee][Rr][Vv][Ee]_[Ww][Aa][Ll]	{ return K_RESERVE_WAL; }
+[Ll][Oo][Gg][Ii][Cc][Aa][Ll]	{ return K_LOGICAL; }
+[Ss][Ll][Oo][Tt]	{ return K_SLOT; }
+[Tt][Ee][Mm][Pp][Oo][Rr][Aa][Rr][Yy]	{ return K_TEMPORARY; }
+[Ee][Xx][Pp][Oo][Rr][Tt]_[Ss][Nn][Aa][Pp][Ss][Hh][Oo][Tt]	{ return K_EXPORT_SNAPSHOT; }
+[Nn][Oo][Ee][Xx][Pp][Oo][Rr][Tt]_[Ss][Nn][Aa][Pp][Ss][Hh][Oo][Tt]	{ return K_NOEXPORT_SNAPSHOT; }
+[Uu][Ss][Ee]_[Ss][Nn][Aa][Pp][Ss][Hh][Oo][Tt]	{ return K_USE_SNAPSHOT; }
 
 ","{ return ','; }
 ";"{ return ';'; }

-- 
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] [PATCH v2] Progress command to monitor progression of long running SQL queries

[Amit Kapila]

On Wed, May 17, 2017 at 9:43 PM, Remi Colinet  wrote:
>
> 2017-05-13 14:38 GMT+02:00 Amit Kapila :
>>
>> On Wed, May 10, 2017 at 10:10 PM, Remi Colinet 
>> wrote:
>> >
>> > Parallel queries can also be monitored. The same mecanism is used to
>> > monitor
>> > child workers with a slight difference: the main worker requests the
>> > child
>> > progression directly in order to dump the whole progress tree in shared
>> > memory.
>> >
>>
>> What if there is any error in the worker (like "out of memory") while
>> gathering the statistics?  It seems both for workers as well as for
>> the main backend it will just error out.  I am not sure if it is a
>> good idea to error out the backend or parallel worker as it will just
>> end the query execution.
>
>
> The handling of progress report starts by the creation of a MemoryContext
> attached to CurrentMemoryContext. Then, few memory (few KB) is allocated.
> Meanwhile, the handling of progress report could indeed exhaust memory and
> fail the backend request. But, in such situation, the backend could also
> have fail even without any progress request.
>
>>
>> Also, even if it is okay, there doesn't seem
>> to be a way by which a parallel worker can communicate the error back
>> to master backend, rather it will just exit silently which is not
>> right.
>
>
> If a child worker fails, it will not respond to the main backend request.
> The main backend will follow up it execution after a 5 seconds timeout (GUC
> param to be added may be). In which case, the report would be partially
> filled.
>
> If the main backend fails, the requesting backend will have a response such
> as:
>
> test=# PROGRESS 14611;
>  PLAN PROGRESS
> 
>  
> (1 row)
>
> test=#
>
> and the child workers will log their response to the shared memory. This
> response will not be collected by the main backend which has failed.
>

If the worker errors out due to any reason, we should end the main
query as well, otherwise, it can produce wrong results.  See the error
handling of workers in HandleParallelMessage


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
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] statement_timeout is not working as expected with postgres_fdw

[Amit Kapila]

On Thu, May 18, 2017 at 7:56 PM, Robert Haas  wrote:
> On Wed, May 17, 2017 at 6:57 AM, Amit Kapila  wrote:
>> +1.  Why not similar behavior for any other statements executed in
>> this module by do_sql_command?
>
> The other cases are not quite the same situation.  It would be good to
> accept interrupts in all cases,
>

Yes, that's one of the main things I was indicating for making the
behavior same, but again that could be done as a separate patch as
well.

> but there's no problem with a session
> continuing to be used after a failure in configure_remote_session()
> because the connection hasn't been entered in the hash table at that
> point yet, and the TRY/CATCH block in connect_pg_server() ensures that
> the connection also gets closed.  So we don't need to worry about
> those statements leaving behind messed-up sessions; they won't; only
> the transaction control commands have that part of the problem.
>

Agreed.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


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

[HACKERS] Allowing dash character in LTREE

[Cyril Auburtin]

It could be useful to allow the `-` char in allowed LTREE label
characters  (currently
a-zA-Z0-9_ https://www.postgresql.org/docs/9.6/static/ltree.html)

The reason is to allow to use more easily base64 ids, (like
https://github.com/dylang/shortid, ...), uuid's too in labels

`-` is also not used as a query operator as a plus


source: https://doxygen.postgresql.org/ltree_8h.html#
a1cb5d5dd0c364d720854e8a250967dd1

[HACKERS] type cache for concat functions

[Pavel Stehule]

Hi

Now concat is 2x times slower than || operator. With cached FmgrInfo for
output function it will be only 5%.

Regards

Pavel
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index be399f4..d3f04f8 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4718,6 +4718,28 @@ string_agg_finalfn(PG_FUNCTION_ARGS)
 		PG_RETURN_NULL();
 }
 
+static void
+rebuildConcatCache(FmgrInfo *typcache, FunctionCallInfo fcinfo, int argidx)
+{
+	int		i;
+
+	Assert(typcache != NULL);
+
+	for (i = argidx; i < PG_NARGS(); i++)
+	{
+		Oid			valtype;
+		Oid			typOutput;
+		bool		typIsVarlena;
+
+		valtype = get_fn_expr_argtype(fcinfo->flinfo, i);
+		if (!OidIsValid(valtype))
+			elog(ERROR, "could not determine data type of concat() input");
+
+		getTypeOutputInfo(valtype, , );
+		fmgr_info_cxt(typOutput, [i-argidx], fcinfo->flinfo->fn_mcxt);
+	}
+}
+
 /*
  * Implementation of both concat() and concat_ws().
  *
@@ -4733,6 +4755,7 @@ concat_internal(const char *sepstr, int argidx,
 	StringInfoData str;
 	bool		first_arg = true;
 	int			i;
+	FmgrInfo		*typcache;
 
 	/*
 	 * concat(VARIADIC some-array) is essentially equivalent to
@@ -4772,14 +4795,20 @@ concat_internal(const char *sepstr, int argidx,
 	/* Normal case without explicit VARIADIC marker */
 	initStringInfo();
 
+	typcache = (FmgrInfo *) fcinfo->flinfo->fn_extra;
+	if (typcache == NULL)
+	{
+		fcinfo->flinfo->fn_extra = MemoryContextAlloc(fcinfo->flinfo->fn_mcxt,
+		PG_NARGS() * sizeof(FmgrInfo));
+		typcache = (FmgrInfo *) fcinfo->flinfo->fn_extra;
+		rebuildConcatCache(typcache, fcinfo, argidx);
+	}
+
 	for (i = argidx; i < PG_NARGS(); i++)
 	{
 		if (!PG_ARGISNULL(i))
 		{
 			Datum		value = PG_GETARG_DATUM(i);
-			Oid			valtype;
-			Oid			typOutput;
-			bool		typIsVarlena;
 
 			/* add separator if appropriate */
 			if (first_arg)
@@ -4787,13 +4816,8 @@ concat_internal(const char *sepstr, int argidx,
 			else
 appendStringInfoString(, sepstr);
 
-			/* call the appropriate type output function, append the result */
-			valtype = get_fn_expr_argtype(fcinfo->flinfo, i);
-			if (!OidIsValid(valtype))
-elog(ERROR, "could not determine data type of concat() input");
-			getTypeOutputInfo(valtype, , );
 			appendStringInfoString(,
-   OidOutputFunctionCall(typOutput, value));
+	OutputFunctionCall([i-argidx], value));
 		}
 	}
 

-- 
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] Removal of plaintext password type references

[Heikki Linnakangas]

On 05/20/2017 05:41 AM, Tom Lane wrote:

Robert Haas  writes:

I guess it does seem likely that most users of the hook would need to
do the same, but it seems confusing to pass the same function both x
and f(x), so my vote is to not do that.


Right, that was my thinking. (Except that my vote is to nevertheless 
keep it unchanged.)



I guess what's in the back of my mind is that the password type might
someday not be just a function of the password, but require other
inputs.  That is, if we change the hook signature as proposed, then
the signature of get_password_type() also becomes part of that API.
If someday f(x) needs to become f(x,y), that becomes either more API
breakage for users of the hook, or no change at all because it's the
callers' problem.

Maybe there's no reason to believe that that will ever happen.


I don't see that happening. It's too convenient that a password verifier 
is self-identifying, i.e. that you can tell what kind of a verifier it 
is, just by looking at the value, without any out-of-band information.


Although when we talked about the representation of password verifiers 
in pg_authid.rolpassword, there was discussion on having a separate 
"password type" field. I was against it, but some thought it would be a 
good idea.



But I'm not disposed to spend
a lot of energy arguing about it, so if other people feel differently,
that's cool.


TBH, I'm not that hot about it either.  But I'm thinking this
is an API break we don't need.


I'm going to just remove this from the open items list. But if some 
other committer disagrees strongly and wants to commit this, I won't object.


- 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] proposal psql \gdesc

[Fabien COELHO]

Hello Pavel,


I am sending a variant with message instead empty result.


Thanks. Patch looks good, applies, make check ok, code is neat.


Personally I prefer empty result instead message.


Hmm. I think that this version is less likely to raise questions from 
users, especially compared to having a somehow correct but strangely 
looking description.


It is hard to choose some good text of this message. Empty result is 
just empty result for all cases.


I'd suggest a very minor change: "No columns or command has no result" 
(not -> no). If some English native speaker has a better suggestion, 
fine with me.


Another good point of this version is that the type name query is 
simplified because it does not need to handle an empty result, thus the 
code is easier to understand.


A few other suggestions:

 - could you update the comment on the type name query?
   Maybe the comment can be simply removed?

 - I'm wondering whether the Name & Type columns names should be
   translatable. What do you think?

 - Maybe tests could also exercise unnamed columns, eg:
SELECT 1, 2, 3 \gdesc \g

--
Fabien.


--
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] Re: proposal - using names as primary names of plpgsql function parameters instead $ based names

[Pavel Stehule]

2017-05-19 5:48 GMT+02:00 Pavel Stehule :

>
>
> 2017-05-19 3:14 GMT+02:00 Peter Eisentraut  com>:
>
>> On 5/15/17 14:34, Pavel Stehule wrote:
>> > Now, I when I working on plpgsql_check, I have to check function
>> > parameters. I can use fn_vargargnos and out_param_varno for list of
>> > arguments and related varno(s). when I detect some issue, I am using
>> > refname. It is not too nice now, because these refnames are $ based.
>> > Long names are alias only. There are not a possibility to find
>> > related alias.
>> >
>> > So, my proposal. Now, we can use names as refname of parameter
>> > variable. $ based name can be used as alias. From user perspective
>> > there are not any change.
>> >
>> > Comments, notes?
>> >
>> > here is a patch
>>
>> I don't understand what this is changing.  There are not documentation
>> or test changes.
>>
>
> This change is visible only for tools like plpgsql_check probably and
> similar tools. Now, this info is not available from user space (maybe only
> from some error message, I have to recheck it)
>
> What is changed.
>
> PLpgSQL variables has field refname - it can be used if you iterate over
> variables or it is used for some error messages. When some variables is
> searching, then namespace aliases are used. Now for any function argument
> is created variable with refname "$x" and namespace aliases "$x" and "name"
> if name exists. There are not any way, how to get a aliases related to
> variable. When I raise a warning in plpgsql about function arguments I have
> to print $x based messages, what is not too readable if function has lot of
> parameters.
>
> The proposal is the change of refname "$x" to "name" for all variables
> created for function arguments.
>
> There are another possibilities - maintain list of all aliases for
> variables or dynamically search all related aliases in namespace tree. Both
> little bit more code.
>

I wrote small regression test, where expected behave is visible

master:

postgres=# create or replace function fx(x ct)
returns void as $$
begin
  perform 1;
  get diagnostics x = row_count;
end;
$$ language plpgsql;
ERROR:  "$1" is not a scalar variable
LINE 5:   get diagnostics x = row_count;

patched:

 postgres=# create or replace function fx(x ct)
returns void as $$
begin
  perform 1;
  get diagnostics x = row_count;
end;
$$ language plpgsql;
ERROR:  "x" is not a scalar variable
LINE 5:   get diagnostics x = row_count;



> Regards
>
> Pavel
>
>
>>
>> --
>> Peter Eisentraut  http://www.2ndQuadrant.com/
>> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>>
>
>
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index a637551..49327b2 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -409,6 +409,7 @@ do_compile(FunctionCallInfo fcinfo,
 			for (i = 0; i < numargs; i++)
 			{
 char		buf[32];
+char	   *argname = NULL;
 Oid			argtypeid = argtypes[i];
 char		argmode = argmodes ? argmodes[i] : PROARGMODE_IN;
 PLpgSQL_type *argdtype;
@@ -433,9 +434,12 @@ do_compile(FunctionCallInfo fcinfo,
 		   errmsg("PL/pgSQL functions cannot accept type %s",
   format_type_be(argtypeid;
 
+argname = (argnames && argnames[i][0] != 0) ? argnames[i] : NULL;
+
 /* Build variable and add to datum list */
-argvariable = plpgsql_build_variable(buf, 0,
-	 argdtype, false);
+argvariable = plpgsql_build_variable(argname != NULL ?
+		   argname : buf,
+		   0, argdtype, false);
 
 if (argvariable->dtype == PLPGSQL_DTYPE_VAR)
 {
@@ -461,9 +465,9 @@ do_compile(FunctionCallInfo fcinfo,
 add_parameter_name(argitemtype, argvariable->dno, buf);
 
 /* If there's a name for the argument, make an alias */
-if (argnames && argnames[i][0] != '\0')
+if (argname != '\0')
 	add_parameter_name(argitemtype, argvariable->dno,
-	   argnames[i]);
+	   argname);
 			}
 
 			/*
diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out
index 7ebbde6..a9fe736 100644
--- a/src/test/regress/expected/plpgsql.out
+++ b/src/test/regress/expected/plpgsql.out
@@ -5979,3 +5979,15 @@ LINE 1: SELECT (SELECT string_agg(id || '=' || name, ',') FROM d)
^
 QUERY:  SELECT (SELECT string_agg(id || '=' || name, ',') FROM d)
 CONTEXT:  PL/pgSQL function alter_table_under_transition_tables_upd_func() line 3 at RAISE
+-- should fail -- message should to contain argument name
+CREATE TYPE ct AS (a int, b int);
+CREATE OR REPLACE FUNCTION fx(x ct)
+RETURNS void AS $$
+BEGIN
+  GET DIAGNOSTICS x = ROW_COUNT;
+end;
+$$ language plpgsql;
+ERROR:  "x" is not a scalar variable
+LINE 4:   GET DIAGNOSTICS x = ROW_COUNT;
+  ^
+DROP TYPE ct;
diff --git a/src/test/regress/sql/plpgsql.sql