[HACKERS] Typo in drop_publication.sgml

2017-06-17 Thread Julien Rouhaud
Patch attached.

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/ref/drop_publication.sgml 
b/doc/src/sgml/ref/drop_publication.sgml
index 517d142251..8e45a43982 100644
--- a/doc/src/sgml/ref/drop_publication.sgml
+++ b/doc/src/sgml/ref/drop_publication.sgml
@@ -46,8 +46,8 @@ DROP PUBLICATION [ IF EXISTS ] name
 IF EXISTS
 
  
-  Do not throw an error if the extension does not exist. A notice is issued
-  in this case.
+  Do not throw an error if the publication does not exist. A notice is
+  issued in this case.
  
 


-- 
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] Typo in planstats.sgml

2017-06-17 Thread Julien Rouhaud
On 18/06/2017 01:03, Peter Eisentraut wrote:
> On 6/17/17 05:00, Julien Rouhaud wrote:
>> A "condition" is missing, patch attached.
> 
> fixed
> 

Thanks.

-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org


-- 
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] logical replication - still unstable after all these months

2017-06-17 Thread Erik Rijkers

On 2017-06-18 00:27, Peter Eisentraut wrote:

On 6/17/17 06:48, Erik Rijkers wrote:

On 2017-05-28 12:44, Erik Rijkers wrote:

re: srsubstate in pg_subscription_rel:


No idea what it means.  At the very least this value 'w' is missing
from the documentation, which only mentions:
  i = initalize
  d = data copy
  s = synchronized
  r = (normal replication)


Shouldn't we add this to that table (51.53) in the documentation?

After all, the value 'w' does show up when you monitor
pg_subscription_rel.


It's not supposed to.  Have you seen it after
e3a815d2faa5be28551e71d5db44fb2c78133433?


Ah no, I haven't seen that 'w'-value after that (and 1000s of tests ran 
without error since then).


I just hadn't realized that that w-value I had reported was indeed a 
erroneous state.


thanks, this is OK then.

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] type cache for concat functions

2017-06-17 Thread Pavel Stehule
2017-06-18 0:50 GMT+02:00 Dmitry Dolgov <9erthali...@gmail.com>:

> > On 20 May 2017 at 10:03, Pavel Stehule  wrote:
> >
> > Now concat is 2x times slower than || operator. With cached FmgrInfo for
> > output function it will be only 5%.
>
> Looks nice and does what's expected (what else one may need from a Saturday
> evening). I just can mention that from what I see the previous version of
> `concat` is slower the more arguments are getting involved, so looks like
> it
> can be more than 2x.
>
> Also, it was a little bit confusing to see that the function `concat`
> behaves
> differently from operator `||` in terms of performance. When you're
> looking at
> the code it's becoming obvious, but I couldn't find any mention about that
> in
> the documentation.
>

There was few steps how to evaluate expressions more faster, but no step
how to evaluate faster polymorphic "any" functions. A detecting of used
argument type and looking to type cache are expensive. These functions was
designed for string processing in error messages, and the performance was
not too important. Now the concat function can be used often as replacement
of || operator when you do migration from Oracle due similar behave. And
the performance is more interesting now.

I think so can be interesting to allow some activity in analyzing/planning
time and prexec stage to user function developers. Some is possible via
planning time hooks, but for controlling few functions it is not user
friendly - and there are no space for data from this stages.

Regards

Pavel


Re: [HACKERS] Decimal64 and Decimal128

2017-06-17 Thread Thomas Munro
On Sun, Jun 18, 2017 at 2:31 PM, Robert Haas  wrote:
> On Sat, Jun 17, 2017 at 3:50 PM, Thomas Munro
>  wrote:
>> On Sun, Jun 18, 2017 at 5:38 AM, Robert Haas  wrote:
>>> I feel like these would logically just be different types, like int4
>>> and int8 are.  We don't have integer(9) and integer(18).
>>
>> Hmm.  Perhaps format_type.c could render decfloat16 as decfloat(16)
>> and decfloat34 as decfloat(34), and gram.y could have a production
>> that selects the right one when you write DECFLOAT(x) and rejects
>> values of x other than 16 and 34.
>
> What would be the point of that?

We'd accept and display the new SQL:2016 standard type name with
length, but by mapping it onto different internal types we could use a
pass-by-value type when it fits in a Datum.

-- 
Thomas Munro
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] Decimal64 and Decimal128

2017-06-17 Thread Robert Haas
On Sat, Jun 17, 2017 at 3:50 PM, Thomas Munro
 wrote:
> On Sun, Jun 18, 2017 at 5:38 AM, Robert Haas  wrote:
>> On Thu, Jun 15, 2017 at 10:27 PM, Thomas Munro
>>  wrote:
>>> 1.  They are fixed size, and DECFLOAT(9) [= 32 bit] and DECFLOAT(17)
>>> [= 64 bit] could in theory be passed by value.  Of course we don't
>>> have a way to make those pass-by-value and yet pass DECFLOAT(34) [=
>>> 128 bit] by reference!  That is where I got stuck last time I was
>>> interested in this subject, because that seems like the place where we
>>> would stand to gain a bunch of performance, and yet the limited
>>> technical factors seems to be very well baked into Postgres.
>>
>> I feel like these would logically just be different types, like int4
>> and int8 are.  We don't have integer(9) and integer(18).
>
> Hmm.  Perhaps format_type.c could render decfloat16 as decfloat(16)
> and decfloat34 as decfloat(34), and gram.y could have a production
> that selects the right one when you write DECFLOAT(x) and rejects
> values of x other than 16 and 34.

What would be the point of that?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] outfuncs.c utility statement support

2017-06-17 Thread Peter Eisentraut
On 6/14/17 12:05, Tom Lane wrote:
> Peter Eisentraut  writes:
>> So this seems to be a pretty basic bug.  Some node fields of type char
>> may be zero, and so printing them as a zero byte just truncates the
>> whole output string.  This could be fixed by printing chars like strings
>> with the full escaping mechanism.  See attached patch.
> 
> +1 for fixing this, but you have not handled the read side correctly.
> pg_strtok doesn't promise to return a null-terminated string, so without
> changes, readfuncs.c would not successfully decode a zero-byte char field.
> Also it would do the wrong thing for any character code that outToken had
> decided to prefix with a backslash.  I think you could fix both problems
> like this:
> 
>  /* Read a char field (ie, one ascii character) */
>  #define READ_CHAR_FIELD(fldname) \
>   token = pg_strtok(&length); /* skip :fldname */ \
>   token = pg_strtok(&length); /* get field value */ \
> - local_node->fldname = token[0]
> + local_node->fldname = debackslash(token, length)[0]

An empty token produces "<>", so just debackslashing wouldn't work.  Maybe

local_node->fldname = length ? token[0] : '\0';

?

-- 
Peter Eisentraut  http://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] Preliminary results for proposed new pgindent implementation

2017-06-17 Thread Piotr Stefaniak
On 2017-06-17 21:55, Tom Lane wrote:
> I spent some time looking into this.  I reverted your commits
> 198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with
> standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1
> (Remove inclusion of sys/cdefs.h) locally and tried to build without
> those.

I wanted to mirror that move, but forgot to not rebase the repository,
so I removed those two commits instead of committing their negatives.
Sorry about that.

> I've successfully worked around the err.h change by adding
> cut-down versions of FreeBSD 11's err.h and err.c to the fileset
> (see attached).

I thought about something like:
#ifdef __FreeBSD__
#include 
#define ERR(...) err(__VA_ARGS__)
#define ERRX(...) errx(__VA_ARGS__)
#else
#include "err.h"
#endif

and then call ERR() and ERRX() instead of err() and errx(). But that
requires C99. And I would have a very hard time convincing anyone that
it makes any sense from FreeBSD's perspective, since indent is part of
the base system, where  is guaranteed to exist.

Perhaps it would be best for everyone if indent was moved out of FreeBSD
base, so that portability arguments would make more sense. But that
would take time and some debate.

> However, it's proving impossible to work around having
> "#include " as the first live code in the files.  I thought
> maybe we could provide a dummy cdefs.h file, but that breaks things on
> platforms where cdefs.h is a real thing and is relied on by other system
> headers --- which includes both Linux and BSD.  It seems we would have
> to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a
> departure from FreeBSD practice.

I was thinking if I could get away with putting those into #ifdef
__FreeBSD__ ... #endif. I think that it might be feasible unlike the
idea above. I could be wrong.

> So what I'm currently thinking is that we have to diverge from the
> FreeBSD sources to the extent of removing #include 
> and the __FBSDID() calls, and instead inserting #include "c.h" to
> pick up PG's own portability definitions.  The thing that forced me
> into the latter is that there seems no way to avoid compiler warnings
> if we don't decorate the declarations of err() and errx() with noreturn
> and printf-format attributes --- and we need c.h to provide portable
> ways of writing those.  But there are probably other portability things
> that we'll need c.h for, anyway, especially if we want to make it work
> on Windows.  So I'm thinking this is a small and easily maintainable
> difference from the upstream FreeBSD files.

That works for me.

> When I inserted #include "c.h", I got duplicate-macro-definition warnings
> about "true" and "false", so I would ask you to add this:

Done.



-- 
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] Typo in planstats.sgml

2017-06-17 Thread Peter Eisentraut
On 6/17/17 05:00, Julien Rouhaud wrote:
> A "condition" is missing, patch attached.

fixed

-- 
Peter Eisentraut  http://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] type cache for concat functions

2017-06-17 Thread Dmitry Dolgov
> On 20 May 2017 at 10:03, Pavel Stehule  wrote:
>
> Now concat is 2x times slower than || operator. With cached FmgrInfo for
> output function it will be only 5%.

Looks nice and does what's expected (what else one may need from a Saturday
evening). I just can mention that from what I see the previous version of
`concat` is slower the more arguments are getting involved, so looks like it
can be more than 2x.

Also, it was a little bit confusing to see that the function `concat`
behaves
differently from operator `||` in terms of performance. When you're looking
at
the code it's becoming obvious, but I couldn't find any mention about that
in
the documentation.


Re: [HACKERS] logical replication - still unstable after all these months

2017-06-17 Thread Peter Eisentraut
On 6/17/17 06:48, Erik Rijkers wrote:
> On 2017-05-28 12:44, Erik Rijkers wrote:
> 
> re: srsubstate in pg_subscription_rel:
> 
>> No idea what it means.  At the very least this value 'w' is missing
>> from the documentation, which only mentions:
>>   i = initalize
>>   d = data copy
>>   s = synchronized
>>   r = (normal replication)
> 
> Shouldn't we add this to that table (51.53) in the documentation?
> 
> After all, the value 'w' does show up when you monitor 
> pg_subscription_rel.

It's not supposed to.  Have you seen it after
e3a815d2faa5be28551e71d5db44fb2c78133433?

-- 
Peter Eisentraut  http://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


[HACKERS] initdb initalization failure for collation "ja_JP"

2017-06-17 Thread Marco Atzeri

Building on Cygwin latest 10  beta1 or head sourece,
make check fails as:

-initdb.log -
The database cluster will be initialized with locales
  COLLATE:  en_US.UTF-8
  CTYPE:en_US.UTF-8
  MESSAGES: C
  MONETARY: en_US.UTF-8
  NUMERIC:  en_US.UTF-8
  TIME: en_US.UTF-8
The default database encoding has accordingly been set to "UTF8".
The default text search configuration will be set to "english".

Data page checksums are disabled.

creating directory 
/cygdrive/e/cyg_pub/devel/postgresql/prova/postgresql-10.0-0.1.x86_64/build/src/test/regress/./tmp_check/data 
... ok

creating subdirectories ... ok
selecting default max_connections ... 30
selecting default shared_buffers ... 128MB
selecting dynamic shared memory implementation ... sysv
creating configuration files ... ok
running bootstrap script ... ok
performing post-bootstrap initialization ... 2017-05-31 23:23:22.214 
CEST [16860] FATAL:  collation "ja_JP" for encoding "EUC_JP" already exists

-

This does not happen on 9.6.x.
Any idea what to look ?

Regards
Marco


--
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] GSoC 2017: Foreign Key Arrays

2017-06-17 Thread Mark Rofail
*Updates till now:*

   - added a record to pg_proc (src/include/catalog/pg_proc.h)
   - modified opr_sanity regression check expected results
   - implemented a  low-level function called `array_contains_elem` as an
   equivalent to `array_contain_compare` but accepts anyelement instead of
   anyarray as the right operand. This is more efficient than constructing an
   array and then immediately deconstructing it.

*Questions:*

   - I'd like to check that anyelem and anyarray have the same element
   type. but anyelem is obtained from PG_FUNCTION_ARGS as a Datum. How can
   I make such a check?

Best Regards,
Mark Rofail
diff --git a/src/backend/access/gin/ginarrayproc.c b/src/backend/access/gin/ginarrayproc.c
index cc7435e030..214aac8fba 100644
--- a/src/backend/access/gin/ginarrayproc.c
+++ b/src/backend/access/gin/ginarrayproc.c
@@ -24,6 +24,7 @@
 #define GinContainsStrategy		2
 #define GinContainedStrategy	3
 #define GinEqualStrategy		4
+#define GinContainsElemStrategy		5
 
 
 /*
@@ -43,7 +44,7 @@ ginarrayextract(PG_FUNCTION_ARGS)
 	bool	   *nulls;
 	int			nelems;
 
-	get_typlenbyvalalign(ARR_ELEMTYPE(array),
+	get_typlenbyvalalign(ARR_ELEMTYPE(array),	
 		 &elmlen, &elmbyval, &elmalign);
 
 	deconstruct_array(array,
@@ -110,7 +111,8 @@ ginqueryarrayextract(PG_FUNCTION_ARGS)
 		case GinOverlapStrategy:
 			*searchMode = GIN_SEARCH_MODE_DEFAULT;
 			break;
-		case GinContainsStrategy:
+			case GinContainsElemStrategy:
+			case GinContainsStrategy:
 			if (nelems > 0)
 *searchMode = GIN_SEARCH_MODE_DEFAULT;
 			else	/* everything contains the empty set */
@@ -171,6 +173,7 @@ ginarrayconsistent(PG_FUNCTION_ARGS)
 }
 			}
 			break;
+		case GinContainsElemStrategy:
 		case GinContainsStrategy:
 			/* result is not lossy */
 			*recheck = false;
@@ -258,7 +261,8 @@ ginarraytriconsistent(PG_FUNCTION_ARGS)
 }
 			}
 			break;
-		case GinContainsStrategy:
+			case GinContainsElemStrategy:
+			case GinContainsStrategy:
 			/* must have all elements in check[] true, and no nulls */
 			res = GIN_TRUE;
 			for (i = 0; i < nkeys; i++)
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index d9c8aa569c..8009ab5acb 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -4232,6 +4232,107 @@ arraycontained(PG_FUNCTION_ARGS)
 	PG_RETURN_BOOL(result);
 }
 
+/*
+ * array_contains_elem : checks an array for a spefific element
+ */
+static bool
+array_contains_elem(AnyArrayType *array, Datum elem, Oid collation,
+	void **fn_extra)
+{
+	Oid 		element_type = AARR_ELEMTYPE(array);
+	TypeCacheEntry *typentry;
+	int 		nelems;
+	int			typlen;
+	bool		typbyval;
+	char		typalign;
+	int			i;
+	array_iter 	it1;
+	FunctionCallInfoData locfcinfo;
+
+	/*
+	 * We arrange to look up the equality function only once per series of
+	 * calls, assuming the element type doesn't change underneath us.  The
+	 * typcache is used so that we have no memory leakage when being used as
+	 * an index support function.
+	 */
+	typentry = (TypeCacheEntry *)*fn_extra;
+	if (typentry == NULL ||
+		typentry->type_id != element_type)
+	{
+		typentry = lookup_type_cache(element_type,
+	 TYPECACHE_EQ_OPR_FINFO);
+		if (!OidIsValid(typentry->eq_opr_finfo.fn_oid))
+			ereport(ERROR,
+	(errcode(ERRCODE_UNDEFINED_FUNCTION),
+	 errmsg("could not identify an equality operator for type %s",
+			format_type_be(element_type;
+		*fn_extra = (void *)typentry;
+	}
+	typlen = typentry->typlen;
+	typbyval = typentry->typbyval;
+	typalign = typentry->typalign;
+
+	/*
+	 * Apply the comparison operator to each pair of array elements.
+	 */
+	InitFunctionCallInfoData(locfcinfo, &typentry->eq_opr_finfo, 2,
+			 collation, NULL, NULL);
+
+	/* Loop over source data */
+	nelems = ArrayGetNItems(AARR_NDIM(array), AARR_DIMS(array));
+	array_iter_setup(&it1, array);
+
+	for (i = 0; i < nelems; i++)
+	{
+		Datum elt1;
+		bool isnull;
+		bool oprresult;
+
+		/* Get element, checking for NULL */
+		elt1 = array_iter_next(&it1, &isnull, i, typlen, typbyval, typalign);
+
+		/*
+		 * We assume that the comparison operator is strict, so a NULL can't
+		 * match anything.  XXX this diverges from the "NULL=NULL" behavior of
+		 * array_eq, should we act like that?
+		 */
+		if (isnull)
+			continue;
+
+		/*
+			* Apply the operator to the element pair
+			*/
+		locfcinfo.arg[0] = elt1;
+		locfcinfo.arg[1] = elem;
+		locfcinfo.argnull[0] = false;
+		locfcinfo.argnull[1] = false;
+		locfcinfo.isnull = false;
+		oprresult = DatumGetBool(FunctionCallInvoke(&locfcinfo));
+		if (oprresult)
+			return true;
+	}
+
+	return false;
+}
+
+Datum
+arraycontainselem(PG_FUNCTION_ARGS)
+{
+	AnyArrayType *array = PG_GETARG_ANY_ARRAY(0);
+	Datum elem = PG_GETARG_DATUM(1);
+	Oid collation = PG_GET_COLLATION();
+	bool result;
+
+	result = array_contains_elem(array, elem, collation,
+ &fcinfo->flinfo->fn_extra);
+
+	/* Avoid leaking memory when handed toasted input. */
+	AARR_FREE_IF_C

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

2017-06-17 Thread Peter Eisentraut
On 6/16/17 10:51, Tom Lane wrote:
> So I'm back to the position that we ought to stick the indent
> code under src/tools/ in our main repo.  Is anyone really
> seriously against that?

I think it would be better to have it separate.

Other than for reasons of principle and general modularity of the world,
I would like this to be available separately for separate download,
packaging, etc. to it can be applied to extension projects without
having to download and build (a specific version of) PostgreSQL.  The
code formatting in extension projects is, um, questionable.  In fact, if
it's a better indent period, I would like to package it for the general
public.

If the vote is to put it into the tree, I would request not to do it in
PG10.  At this point, we should be winding things down and not open up
new areas of activity.  There is a chance that if this goes in (or
anywhere else), there will be a stream of requests along the lines of:
doesn't build on Windows, doesn't build on AIX, doesn't build on
PowerPC, doesn't build on this other Windows variant, the tests don't
run, the tests don't run on Windows, it doesn't build in vpath, it
doesn't work on the buildfarm, and so on.

-- 
Peter Eisentraut  http://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] INSERT ... ON CONFLICT () SELECT

2017-06-17 Thread Peter Geoghegan
On Sat, Jun 17, 2017 at 7:49 AM, Matt Pulver  wrote:
> With the proposed "INSERT ... ON CONFLICT () SELECT" feature, the
> get_or_create_id() function is simplified to:

Are you locking the existing rows? Because otherwise, the
determination that they're conflicting can become obsolete immediately
afterwards. (I guess you would be.)

The problem with this design and similar designs is that presumably
the user is sometimes projecting the conflicting rows with the
intention of separately updating them in a wCTE. That might not work,
because only ON CONFLICT doesn't use the MVCC snapshot, in order to
ensure that an UPDATE is guaranteed when an INSERT cannot go ahead.
That isn't what you're doing in the example you gave, but surely some
users would try to do things like that, and get very confused.

I think that what you propose to do here would likely create a lot of
confusion by mixing MVCC semantics with special UPSERT visibility
semantics ("show me the latest row version visible to any possible
snapshot for the special update") even without a separate UPDATE, in
fact. Would you be okay if "id" appeared duplicated in the rows you
project in your new syntax, even when there is a separate unique
constraint on that column? I suppose that there is some risk of things
like that today, but this would make the "sleight of hand" used by ON
CONFLICT DO UPDATE more likely to cause problems.

-- 
Peter Geoghegan


-- 
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] Preliminary results for proposed new pgindent implementation

2017-06-17 Thread Tom Lane
I wrote:
> Piotr Stefaniak  writes:
>> There are also the "portability fixes" and they're the main problem.

> Fair enough.

I spent some time looking into this.  I reverted your commits
198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with
standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1
(Remove inclusion of sys/cdefs.h) locally and tried to build without
those.  I've successfully worked around the err.h change by adding
cut-down versions of FreeBSD 11's err.h and err.c to the fileset
(see attached).  However, it's proving impossible to work around having
"#include " as the first live code in the files.  I thought
maybe we could provide a dummy cdefs.h file, but that breaks things on
platforms where cdefs.h is a real thing and is relied on by other system
headers --- which includes both Linux and BSD.  It seems we would have
to have something like #ifdef HAVE_SYS_CDEFS_H, but that is already a
departure from FreeBSD practice.

So what I'm currently thinking is that we have to diverge from the
FreeBSD sources to the extent of removing #include 
and the __FBSDID() calls, and instead inserting #include "c.h" to
pick up PG's own portability definitions.  The thing that forced me
into the latter is that there seems no way to avoid compiler warnings
if we don't decorate the declarations of err() and errx() with noreturn
and printf-format attributes --- and we need c.h to provide portable
ways of writing those.  But there are probably other portability things
that we'll need c.h for, anyway, especially if we want to make it work
on Windows.  So I'm thinking this is a small and easily maintainable
difference from the upstream FreeBSD files.

When I inserted #include "c.h", I got duplicate-macro-definition warnings
about "true" and "false", so I would ask you to add this:

--- freebsd_indent/indent_globs.h   2017-06-16 11:06:53.329712682 -0400
+++ new/indent_globs.h  2017-06-17 14:45:41.388015754 -0400
@@ -43,8 +43,12 @@
 * of code */
 
 
+#ifndef false
 #define false 0
+#endif
+#ifndef true
 #define true  1
+#endif
 
 
 FILE   *input; /* the fid for the input file */

Other than that, I think this is a workable compromise on the
portability questions.

regards, tom lane

/*-
 * Copyright (c) 1993
 *  The Regents of the University of California.  All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *notice, this list of conditions and the following disclaimer in the
 *documentation and/or other materials provided with the distribution.
 * 3. Neither the name of the University nor the names of its contributors
 *may be used to endorse or promote products derived from this software
 *without specific prior written permission.
 *
 * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 *
 *  @(#)err.h   8.1 (Berkeley) 6/2/93
 * $FreeBSD: stable/11/include/err.h 203964 2010-02-16 19:39:50Z imp $
 */

#ifndef _ERR_H_
#define _ERR_H_

/*
 * This is cut down to just the minimum that we need to build indent.
 */

voiderr(int, const char *, ...)
  pg_attribute_noreturn() pg_attribute_printf(2, 3);
voiderrx(int, const char *, ...)
  pg_attribute_noreturn() pg_attribute_printf(2, 3);

#endif /* !_ERR_H_ */
/*-
 * Copyright (c) 1993
 *  The Regents of the University of California.  All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *notice, this list of conditions and the following disclaimer in the
 *documentation and/or other materials provided with the distribution.
 * 3. Neither the name of the University 

Re: [HACKERS] Decimal64 and Decimal128

2017-06-17 Thread Thomas Munro
On Sun, Jun 18, 2017 at 5:38 AM, Robert Haas  wrote:
> On Thu, Jun 15, 2017 at 10:27 PM, Thomas Munro
>  wrote:
>> 1.  They are fixed size, and DECFLOAT(9) [= 32 bit] and DECFLOAT(17)
>> [= 64 bit] could in theory be passed by value.  Of course we don't
>> have a way to make those pass-by-value and yet pass DECFLOAT(34) [=
>> 128 bit] by reference!  That is where I got stuck last time I was
>> interested in this subject, because that seems like the place where we
>> would stand to gain a bunch of performance, and yet the limited
>> technical factors seems to be very well baked into Postgres.
>
> I feel like these would logically just be different types, like int4
> and int8 are.  We don't have integer(9) and integer(18).

Hmm.  Perhaps format_type.c could render decfloat16 as decfloat(16)
and decfloat34 as decfloat(34), and gram.y could have a production
that selects the right one when you write DECFLOAT(x) and rejects
values of x other than 16 and 34.

-- 
Thomas Munro
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] Preliminary results for proposed new pgindent implementation

2017-06-17 Thread Tom Lane
Robert Haas  writes:
> On Fri, Jun 16, 2017 at 10:51 AM, Tom Lane  wrote:
>> So I'm back to the position that we ought to stick the indent
>> code under src/tools/ in our main repo.  Is anyone really
>> seriously against that?

> Is it under the same license as everything else?

Hm, now that you mention it, interesting point.  I was about to
answer that it's under the standard BSD license, but looking closer,
I see that these files still quote the old 4-clause BSD license text
(with the "advertising" clause).  We need to get them adjusted
to be 3-clause with no advertising.  Looking into a FreeBSD source
tree locally, I see that FreeBSD has removed the advertising clause
from all their files --- sometimes without even remembering to
renumber the old clause 4 to clause 3 ;-).  So Piotr needs to do
likewise to conform with FreeBSD policy.  Once he has, it'll be
identical text to the other BSD-origin files we have in our tree,
eg src/port/getopt.c.

Piotr: if you're unclear on the rationale for this, see the bottom of
https://www.freebsd.org/copyright/license.html

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] Preliminary results for proposed new pgindent implementation

2017-06-17 Thread Robert Haas
On Fri, Jun 16, 2017 at 10:51 AM, Tom Lane  wrote:
> So I'm back to the position that we ought to stick the indent
> code under src/tools/ in our main repo.  Is anyone really
> seriously against that?

Is it under the same license as everything else?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] Decimal64 and Decimal128

2017-06-17 Thread Robert Haas
On Thu, Jun 15, 2017 at 10:27 PM, Thomas Munro
 wrote:
> 1.  They are fixed size, and DECFLOAT(9) [= 32 bit] and DECFLOAT(17)
> [= 64 bit] could in theory be passed by value.  Of course we don't
> have a way to make those pass-by-value and yet pass DECFLOAT(34) [=
> 128 bit] by reference!  That is where I got stuck last time I was
> interested in this subject, because that seems like the place where we
> would stand to gain a bunch of performance, and yet the limited
> technical factors seems to be very well baked into Postgres.

I feel like these would logically just be different types, like int4
and int8 are.  We don't have integer(9) and integer(18).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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


Re: [HACKERS] how are the rpms configured that are available in PostgreSQL RPM Building Project - Yum Repository

2017-06-17 Thread Devrim Gündüz

Hi,

On Fri, 2017-06-16 at 19:40 +, Cook, Malcolm wrote:
> 
> I am referring to the contents of https://yum.postgresql.org/ (specifically
> version 9.6 rpms for CentoOS7)
> 
> More specifically I wonder if they are configured:    --with-python --with-
> tcl --with-pam --with-ldap

/usr/pgsql-9.6/bin/pg_config --configure will give the full list (the answer is
yes, btw)

> And do they install the Additional Supplied Modules
> (https://www.postgresql.org/docs/9.1/static/contrib.html ) as provided when
> performing a
> 
>    make install world

Yes, the -contrib rpm provides those.

> Please advise if my possibly novice question should better be posed
> elsewhere.  


Please email pgsql-pkg-...@postgresql.org list for further questions.

Regards,

-- 
Devrim Gündüz
EnterpriseDB: https://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR


signature.asc
Description: This is a digitally signed message part


[HACKERS] 答复: [HACKERS] 答复: GiST API Adancement

2017-06-17 Thread Yuan Dong
2017-06-16 17:06 GMT+03:00 Tom Lane :
> Yuan Dong  writes:
>> ·¢¼þÈË: Andrew Borodin 
>>> I think there is one more aspect of development: backward
>>> compatibility: it's impossible to update all existing extensions. This
>>> is not that major feature to ignore them.
>
>> I should still maintain original API of GiST after modification.
>
> To put a little context on that: we have always felt that it's okay
> to require extension authors to make small adjustments when going to
> a new major release.


Tom's comment is very helpful, I will keep it in mind.


>Then maybe it worth to consider more general API advancement at once?
>Here's my wish list:
>1. Choose subtree function as an alternative for penalty
>2. Bulk load support
>3. Better split framework: now many extensions peek two seeds and
>spread split candidates among them, some with subtle but noisy
>mistakes
>4. Better way of key compares (this thread, actually)
>5. Split in many portions

Okay, I would try to do more advancements step by step.

I do not understand the problem of choosing subtree as an alternative for 
penalty. What do yo mean by choosing subtree function?


There will be more questions coming, hope you will be comfortable with that~


Best wishes.

--

Dong



发件人: Andrew Borodin 
发送时间: 2017年6月16日 16:48:04
收件人: Tom Lane
抄送: Yuan Dong; pgsql-hackers@postgresql.org
主题: Re: [HACKERS] 答复: GiST API Adancement

2017-06-16 17:06 GMT+03:00 Tom Lane :
> Yuan Dong  writes:
>> ·¢¼þÈË: Andrew Borodin 
>>> I think there is one more aspect of development: backward
>>> compatibility: it's impossible to update all existing extensions. This
>>> is not that major feature to ignore them.
>
>> I should still maintain original API of GiST after modification.
>
> To put a little context on that: we have always felt that it's okay
> to require extension authors to make small adjustments when going to
> a new major release.

Then maybe it worth to consider more general API advancement at once?
Here's my wish list:
1. Choose subtree function as an alternative for penalty
2. Bulk load support
3. Better split framework: now many extensions peek two seeds and
spread split candidates among them, some with subtle but noisy
mistakes
4. Better way of key compares (this thread, actually)
5. Split in many portions

Best regards, Andrey Borodin.


Re: [HACKERS] Getting server crash on Windows when using ICU collation

2017-06-17 Thread Ashutosh Sharma
Hi,

On Sat, Jun 17, 2017 at 6:38 PM, Peter Eisentraut
 wrote:
> On 6/16/17 23:46, Amit Kapila wrote:
>> I have just posted one way
>> to determine if icu library has support for ucol_strcollUTF8, see if
>> that sounds like a way forward to you.
>
> I'm not in a position to test such patches, so someone else will have to
> take that on.

Well, I have tested it from my end. I've basically tried to test it by
running multi-byte tests as Amit suggested and by verifing the things
manually through debugger. And, i had mistakenly attached wrong patch
in my earlier email. Here, i attach the correct patch. Sorry about
that.

>
> It might not be worth bothering.  ICU 50 is already about 5 years old.
> If you're packaging for Windows, I suspect you have the option of
> bundling a version of your choice.  The support for older versions is
> mainly to support building on "stable" Linux distributions, and even
> there the window of usefulness is closing.  (CentOS 7 has 50, CentOS 6
> has 4.2 which is too old for other reasons, Debian stable has 52 (and it
> will become oldstable after today)).

Yes, it's hard to find any user's having ICU version < 50 installed on
their system. But, having said that, it's always good to have such
detective checks basically considering that we already have such
configure check for Linux platform. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com


detect_ICU_version_v2.patch
Description: Binary data

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


[HACKERS] INSERT ... ON CONFLICT () SELECT

2017-06-17 Thread Matt Pulver
Hello,

I am looking to add a new language feature that returns the rows that
conflict on an INSERT, and would appreciate feedback and guidance on this.

Here is an example.

To implement a get_or_create_id() function, this is how it must currently
be done:

CREATE TABLE example (
id SERIAL PRIMARY KEY,
name TEXT NOT NULL,
value FLOAT);
CREATE UNIQUE INDEX ON example (name);

CREATE FUNCTION get_or_create_id(_name TEXT) RETURNS INT AS
$$
WITH get AS (
SELECT id FROM example WHERE name=_name
), new AS (
INSERT INTO example (name) VALUES (_name)
ON CONFLICT (name) DO NOTHING
RETURNING id
)
SELECT id FROM get
UNION ALL
SELECT id FROM new
$$
LANGUAGE sql;

SELECT get_or_create_id('foo'); -- 1
SELECT get_or_create_id('bar'); -- 2
SELECT get_or_create_id('foo'); -- 1


With the proposed "INSERT ... ON CONFLICT () SELECT" feature, the
get_or_create_id() function is simplified to:

CREATE FUNCTION get_or_create_id(_name TEXT) RETURNS INT AS
$$
INSERT INTO example (name) VALUES (_name)
ON CONFLICT (name) DO SELECT
RETURNING id
$$
LANGUAGE sql;


In the case of a CONFLICT, the selected rows are exactly those same rows
that would be operated on by an ON CONFLICT () DO UPDATE clause. These rows
are then made available to the RETURNING clause in the same manner. Just
like "DO NOTHING", the "DO SELECT" clause takes no arguments. It only makes
the conflicting rows available to the RETURNING clause.

Tom Lane has previously responded
 to a
similar request which was ill-defined, especially in the context of
exclusion constraints. I believe that by SELECTing exactly those same rows
that an UPDATE clause would on a CONFLICT, this becomes well-defined, even
with exclusion constraints.

Feedback/guidance is most welcome.

Best regards,
Matt


Re: [HACKERS] Typo in CREATE SUBSCRIPTION documentation

2017-06-17 Thread Julien Rouhaud
On Sat, Jun 17, 2017 at 10:24:32AM -0400, Peter Eisentraut wrote:
> On 6/15/17 15:19, Julien Rouhaud wrote:
> > Hi,
> > 
> > I just found $SUBJECT, patch attached.
> 
> fixed

Thanks!

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org


-- 
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] Typo in CREATE SUBSCRIPTION documentation

2017-06-17 Thread Peter Eisentraut
On 6/15/17 15:19, Julien Rouhaud wrote:
> Hi,
> 
> I just found $SUBJECT, patch attached.

fixed

-- 
Peter Eisentraut  http://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] Getting server crash on Windows when using ICU collation

2017-06-17 Thread Peter Eisentraut
On 6/16/17 23:46, Amit Kapila wrote:
> I have just posted one way
> to determine if icu library has support for ucol_strcollUTF8, see if
> that sounds like a way forward to you.

I'm not in a position to test such patches, so someone else will have to
take that on.

It might not be worth bothering.  ICU 50 is already about 5 years old.
If you're packaging for Windows, I suspect you have the option of
bundling a version of your choice.  The support for older versions is
mainly to support building on "stable" Linux distributions, and even
there the window of usefulness is closing.  (CentOS 7 has 50, CentOS 6
has 4.2 which is too old for other reasons, Debian stable has 52 (and it
will become oldstable after today)).

Then again, it might be worth putting things into place in case we have
to do similar detections in the future.

-- 
Peter Eisentraut  http://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] Getting server crash on Windows when using ICU collation

2017-06-17 Thread Ashutosh Sharma
Hi,

Attached is the patch that detects the ICU version on Windows and
based on that it decides whether 'HAVE_UCOL_STRCOLLUTF8' flag needs to
be set or not.  If this patch gets consisdered then, we may have to
revert the changes following git commit. Thanks.

commit e42645ad92687a2250ad17e1a045da73e54a5064
Author: Peter Eisentraut 
Date:   Fri Jun 16 21:23:22 2017 -0400

Define HAVE_UCOL_STRCOLLUTF8 on Windows

This should normally be determined by a configure check, but until
someone figures out how to do that on Windows, it's better that the code
uses the new function by default.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

On Sat, Jun 17, 2017 at 9:16 AM, Amit Kapila  wrote:
> On Sat, Jun 17, 2017 at 6:57 AM, Peter Eisentraut
>  wrote:
>> On 6/16/17 10:12, Peter Eisentraut wrote:
>>> On 6/16/17 06:30, Amit Kapila wrote:
 How will this compare UTF-8 strings in UTF-8 encoding?  It seems to me
 that ideally, it should use ucol_strcollUTF8 to compare the same,
 however, with patch, it will always ucol_strcoll as we never define
 HAVE_UCOL_STRCOLLUTF8 flag on Windows.
>>>
>>> We have a configure check for that, but I don't know how to replicate
>>> that on Windows.
>>>
>>> If ucol_strcollUTF8 is not available, we have code to convert to UTF-16.
>>>  This is the same code that is used for non-Windows.
>>
>> After thinking about this some more, I have committed a change to define
>> HAVE_UCOL_STRCOLLUTF8 on Windows unconditionally.  Until someone figures
>> out a different way, I think it's better that users of newish versions
>> of ICU get the newer/better behavior, and users of older versions can
>> file a bug.
>>
>
> Yeah, we can take that stand or maybe document that as well but not
> sure that is the best way to deal with it.  I have just posted one way
> to determine if icu library has support for ucol_strcollUTF8, see if
> that sounds like a way forward to you.
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com


detect_ICU_version.patch
Description: Binary data

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


Re: [HACKERS] [BUGS] BUG #14699: Statement trigger and logical replication

2017-06-17 Thread Euler Taveira
2017-06-16 22:08 GMT-03:00 Peter Eisentraut :

>
> The issue is that the logical replication initial data copy fires a
> statement trigger for INSERT, because it's implemented as a COPY
> internally.
>
> We should document such behavior. AFAICS we discuss later if we should
provide an option to fire statement triggers during initial copy.


> By contrast, the normal apply worker does not fire any statement
> triggers (because they are not "statements").
>
> +1.


> We could adjust one or the other or leave it as is.


Let's leave it as is. At least until 11.


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento



Re: [HACKERS] logical replication - still unstable after all these months

2017-06-17 Thread Erik Rijkers

On 2017-05-28 12:44, Erik Rijkers wrote:

re: srsubstate in pg_subscription_rel:


No idea what it means.  At the very least this value 'w' is missing
from the documentation, which only mentions:
  i = initalize
  d = data copy
  s = synchronized
  r = (normal replication)


Shouldn't we add this to that table (51.53) in the documentation?

After all, the value 'w' does show up when you monitor 
pg_subscription_rel.









--
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] Incorrect comment in 001_ssltests.pl

2017-06-17 Thread Michael Paquier
On Sat, Jun 17, 2017 at 5:20 PM, Magnus Hagander  wrote:
> One could argue that the first sentences should just read "the argument"
> once the second one isn't referred, but I can't get too excited about that.
> Thus, patch applied as-is - thanks!

Thanks.
-- 
Michael


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


[HACKERS] Typo in planstats.sgml

2017-06-17 Thread Julien Rouhaud
A "condition" is missing, patch attached.

--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/planstats.sgml b/doc/src/sgml/planstats.sgml
index 8caf297f85..838fcda6d2 100644
--- a/doc/src/sgml/planstats.sgml
+++ b/doc/src/sgml/planstats.sgml
@@ -501,8 +501,8 @@ EXPLAIN (ANALYZE, TIMING OFF) SELECT * FROM t WHERE a = 1;
 of this clause to be 1%.  By comparing this estimate and the actual
 number of rows, we see that the estimate is very accurate
 (in fact exact, as the table is very small).  Changing the
-WHERE to use the b column, an identical
-plan is generated.  But observe what happens if we apply the same
+WHERE condition to use the b column, an
+identical plan is generated.  But observe what happens if we apply the same
 condition on both columns, combining them with AND:
 
 

-- 
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] WIP Patch: Pgbench Serialization and deadlock errors

2017-06-17 Thread Marina Polyakova
> To be clear, part of "retrying from the beginning" means that if a> result from one statement is used to determine the content (or> whether to run) a subsequent statement, that first statement must be> run in the new transaction and the results evaluated again to> determine what to use for the later statement.  You can't simply> replay the statements that were run during the first try.  For> examples, to help get a feel of why that is, see:> > https://wiki.postgresql.org/wiki/SSIThank you again! :))-- Marina PolyakovaPostgres Professional: http://www.postgrespro.comThe Russian Postgres Company

Re: [HACKERS] Incorrect comment in 001_ssltests.pl

2017-06-17 Thread Magnus Hagander
On Sat, Jun 17, 2017 at 1:53 AM, Michael Paquier 
wrote:

> Hi all,
>
> I have noticed the following thing:
> --- a/src/test/ssl/t/001_ssltests.pl
> +++ b/src/test/ssl/t/001_ssltests.pl
> @@ -34,8 +34,6 @@ sub run_test_psql
>  # The first argument is a (part of a) connection string, and it's also
> printed
>  # out as the test case name. It is appended to $common_connstr global
> variable,
>  # which also contains a libpq connection string.
> -#
> -# The second argument is a hostname to connect to.
>  sub test_connect_ok
>  {
> my $connstr = $_[0]
>
> But test_connect_ok and test_connect_fails do not have a second
> argument as the hostname is appended directly into $common_connstr.
>

One could argue that the first sentences should just read "the argument"
once the second one isn't referred, but I can't get too excited about that.
Thus, patch applied as-is - thanks!

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


Re: [HACKERS] Typos in comments

2017-06-17 Thread Magnus Hagander
On Sat, Jun 17, 2017 at 8:55 AM, Daniel Gustafsson  wrote:

> Spotted one “paramter” typo and git grep found two more, patch attached
> with
> s/paramter/parameter/ for these.
>

Applied, thanks.

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