Re: [HACKERS] Adding support for Default partition in partitioning

[Ashutosh Bapat]

On Fri, Jun 16, 2017 at 12:48 AM, Robert Haas  wrote:
> On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat
>  wrote:
>> Some more comments on the latest set of patches.
>>
>> In heap_drop_with_catalog(), we heap_open() the parent table to get the
>> default partition OID, if any. If the relcache doesn't have an entry for the
>> parent, this means that the entry will be created, only to be invalidated at
>> the end of the function. If there is no default partition, this all is
>> completely unnecessary. We should avoid heap_open() in this case. This also
>> means that get_default_partition_oid() should not rely on the relcache entry,
>> but should growl through pg_inherit to find the default partition.
>
> I am *entirely* unconvinced by this line of argument.  I think we want
> to open the relation the first time we touch it and pass the Relation
> around thereafter.

If this would be correct, why heap_drop_with_catalog() without this
patch just locks the parent and doesn't call a heap_open(). I am
missing something.

> Anything else is prone to accidentally failing to
> have the relation locked early enough,

We are locking the parent relation even without this patch, so this
isn't an issue.

> or looking up the OID in the
> relcache multiple times.

I am not able to understand this in the context of default partition.
After that nobody else is going to change its partitions and their
bounds (since both of those require heap_open on parent which would be
stuck on the lock we hold.). So, we have to check only once if the
table has a default partition. If it doesn't, it's not going to
acquire one unless we release the lock on the parent i.e at the end of
transaction. If it has one, it's not going to get dropped till the end
of the transaction for the same reason. I don't see where we are
looking up OIDs multiple times.


>
>> +defaultPartOid = get_default_partition_oid(rel);
>> +if (OidIsValid(defaultPartOid))
>> +ereport(ERROR,
>> +(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>> + errmsg("there exists a default partition for table
>> \"%s\", cannot attach a new partition",
>> +RelationGetRelationName(rel;
>> +
>> Should be done before heap_open on the table being attached. If we are not
>> going to attach the partition, there's no point in instantiating its 
>> relcache.
>
> No, because we should take the lock before examining any properties of
> the table.

There are three tables involved here. "rel" which is the partitioned
table. "attachrel" is the table being attached as a partition to "rel"
and defaultrel, which is the default partition table. If there exists
a default partition in "rel" we are not allowing "attachrel" to be
attached to "rel". If that's the case, we don't need to examine any
properties of "attachrel" and hence we don't need to instantiate
relcache of "attachrel". That's what the comment is about.
ATExecAttachPartition() receives "rel" as an argument which has been
already locked and opened. So, we can check the existence of default
partition right at the beginning of the function.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] PATCH: Don't downcase filepath/filename while loading libraries

[QL Zhuo]

"That's fourteen years without complains", so maybe not to back-patch is
good choice, until someone complains this :)

And, the attached new patch fixes the memory leaks.

--
This email address (zhuo.devgmail.com) is only for development affairs,
e.g. mail list, please mail to zhuohexoasis.com or zhuoqlzoho.com
for other purpose.

ZHUO QL (KDr2), http://kdr2.com

On Fri, Jun 16, 2017 at 12:27 PM, Tom Lane  wrote:

> Michael Paquier  writes:
> > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> >> (2) My inclination would be not to back-patch.  This change could break
> >> configurations that worked before, and the lack of prior complaints
> >> says that not many people are having a problem with it.
>
> > That's fourteen years without complains, still I cannot imagine any
> > cases where it would be a problem as people who would have faced this
> > problem but not reported it have likely just enforced the FS to handle
> > case-insensitivity for paths.
>
> Well, it's not just about case sensitivity.  The comment for
> SplitDirectoriesString points out that it deals with embedded spaces
> differently, and it also applies canonicalize_path().  I'm too tired
> to think hard about what that part might mean for compatibility, but
> it probably isn't nothing.
>
> Anyway, I agree that this is an appropriate change for HEAD.  Just
> not convinced that we should shove it into minor releases.
>
> regards, tom lane
>


0001-Don-t-downcase-filepath-in-load_libraries.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] PATCH: Don't downcase filepath/filename while loading libraries

[David G. Johnston]

On Thursday, June 15, 2017, Tom Lane  wrote:

> Michael Paquier  writes:
> > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> >> (2) My inclination would be not to back-patch.  This change could break
> >> configurations that worked before, and the lack of prior complaints
> >> says that not many people are having a problem with it.
>
> > That's fourteen years without complains, still I cannot imagine any
> > cases where it would be a problem as people who would have faced this
> > problem but not reported it have likely just enforced the FS to handle
> > case-insensitivity for paths.
>
> Well, it's not just about case sensitivity.  The comment for
> SplitDirectoriesString points out that it deals with embedded spaces
> differently, and it also applies canonicalize_path().  I'm too tired
> to think hard about what that part might mean for compatibility, but
> it probably isn't nothing.
>
> Anyway, I agree that this is an appropriate change for HEAD.  Just
> not convinced that we should shove it into minor releases.
>
>
If it's downcasing then careless use of mixed case when the true path on a
case sensitive filesystem is all lower case (not unlikely at all given
default packaging for deb and rpm packages, iirc) would indeed be a
problem.  Those paths are accidentally working right now.  I vote for no
back-patch.

David J.

Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[Tom Lane]

Michael Paquier  writes:
> On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
>> (2) My inclination would be not to back-patch.  This change could break
>> configurations that worked before, and the lack of prior complaints
>> says that not many people are having a problem with it.

> That's fourteen years without complains, still I cannot imagine any
> cases where it would be a problem as people who would have faced this
> problem but not reported it have likely just enforced the FS to handle
> case-insensitivity for paths.

Well, it's not just about case sensitivity.  The comment for
SplitDirectoriesString points out that it deals with embedded spaces
differently, and it also applies canonicalize_path().  I'm too tired
to think hard about what that part might mean for compatibility, but
it probably isn't nothing.

Anyway, I agree that this is an appropriate change for HEAD.  Just
not convinced that we should shove it into minor releases.

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] PATCH: Don't downcase filepath/filename while loading libraries

[Michael Paquier]

On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> (2) My inclination would be not to back-patch.  This change could break
> configurations that worked before, and the lack of prior complaints
> says that not many people are having a problem with it.

That's fourteen years without complains, still I cannot imagine any
cases where it would be a problem as people who would have faced this
problem but not reported it have likely just enforced the FS to handle
case-insensitivity for paths. Or they just relied on the default: on
Windows the default is to be case-insensitive, as much as OSX. So the
proposed patch handles things better with case-sensitive paths,
without really impacting users with case-insensive FS.

I see the point of not back-patching the change though.
-- 
Michael


-- 
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: Don't downcase filepath/filename while loading libraries

[Tom Lane]

Michael Paquier  writes:
> On Fri, Jun 16, 2017 at 11:04 AM, QL Zhuo  wrote:
>> After few digging, I found there's a wrong use of `SplitIdentifierString` in
>> function `load_libraries` in /src/backend/utils/init/miscinit.c, and the
>> attached patch fixes it.

> That's a good catch. All the other callers of SplitIdentifierString()
> don't handle a list of directories. This requires a back-patch.

(1) As is, I think the patch leaks memory.  SplitDirectoriesString's
API is not identical to SplitIdentifierString's.

(2) My inclination would be not to back-patch.  This change could break
configurations that worked before, and the lack of prior complaints
says that not many people are having a problem with it.

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] Get stuck when dropping a subscription during synchronizing table

[Peter Eisentraut]

On 6/13/17 15:49, Peter Eisentraut wrote:
> On 6/13/17 02:33, Noah Misch wrote:
>>> Steps to reproduce -
>>> X cluster -> create 100 tables , publish all tables  (create publication pub
>>> for all tables);
>>> Y Cluster -> create 100 tables ,create subscription(create subscription sub
>>> connection 'user=centos host=localhost' publication pub;
>>> Y cluster ->drop subscription - drop subscription sub;
>>>
>>> check the log file on Y cluster.
>>>
>>> Sometime , i have seen this error on psql prompt and drop subscription
>>> operation got failed at first attempt.
>>>
>>> postgres=# drop subscription sub;
>>> ERROR:  tuple concurrently updated
>>> postgres=# drop subscription sub;
>>> NOTICE:  dropped replication slot "sub" on publisher
>>> DROP SUBSCRIPTION
>>
>> [Action required within three days.  This is a generic notification.]
> 
> It's being worked on.  Let's see by Thursday.

A patch has been posted, and it's being reviewed.  Next update Monday.

-- 
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] Something is rotten in publication drop

[Peter Eisentraut]

On 6/15/17 12:23, Tom Lane wrote:
> It strikes me that you could rewrite psql's query to just do its own
> catalog search and not bother with the function at all.  It would have
> to know a bit more about the catalog structure than it does now, but not
> that much:
> 
> select pub.pubname from pg_catalog.pg_publication pub
> where puballtables or
>   exists(select 1 from pg_catalog.pg_publication_rel r
>  where r.prpubid = pub.oid and r.prrelid = '%s');

We used to do something like that, but then people complained that that
was not absolutely accurate, because it did not exclude catalog tables
and related things properly.  See commit
2d460179baa8744e9e2a183a5121306596c53fba.  To do this properly, you need
to filter pg_class using is_publishable_class() (hitherto internal C
function).

The way this was originally written was for use by subscriptioncmds.c
fetch_table_list(), which generally only deals with a small number of
publications as the search key and wants to find all the relations.  The
psql use case is exactly the opposite: We start with a relation and want
to find all the publications.  The third use case is that we document
the view pg_publication_tables for general use, so depending on which
search key you start with, you might get terrible performance if you
have a lot of tables.

An academically nice way to write a general query for this would be:

CREATE VIEW pg_publication_tables AS
 SELECT
 p.pubname AS pubname,
 n.nspname AS schemaname,
 c.relname AS tablename,
 c.oid AS relid
 FROM pg_publication p
  JOIN pg_publication_rel pr ON p.oid = pr.prpubid
  JOIN pg_class c ON pr.prrelid = c.oid
  JOIN pg_namespace n ON c.relnamespace = n.oid
 UNION
 SELECT
 p.pubname AS pubname,
 n.nspname AS schemaname,
 c.relname AS tablename,
 c.oid AS relid
 FROM pg_publication p
  JOIN pg_class c ON p.puballtables AND
pg_is_relation_publishable(c.oid)
  JOIN pg_namespace n ON c.relnamespace = n.oid;

But looking at the plans this generates, it will do a sequential scan of
pg_class even if you look for a publication that is not puballtables,
which would suck for the subscriptioncmds.c use case.

We could use the above definition for the documented view and the psql
use case.

We could then create second view that uses the existing definition

CREATE VIEW pg_publication_tables AS
SELECT
P.pubname AS pubname,
N.nspname AS schemaname,
C.relname AS tablename
FROM pg_publication P, pg_class C
 JOIN pg_namespace N ON (N.oid = C.relnamespace)
WHERE C.oid IN (SELECT relid FROM pg_get_publication_tables(P.pubname));

for use in subscriptioncmds.c.  Or don't use a view for that.  But the
view is useful because we should preserve this interface across versions.

Or we throw away all the views and use custom code everywhere.

Patch for experimentation attached.

Any ideas?


-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 855aff09b862fab7bf8ca49b3b653b296a124484 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 15 Jun 2017 23:18:39 -0400
Subject: [PATCH] WIP Tweak publication fetching in psql

---
 src/backend/catalog/pg_publication.c | 16 
 src/backend/catalog/system_views.sql | 23 +--
 src/bin/psql/describe.c  |  5 ++---
 src/include/catalog/pg_proc.h|  2 ++
 4 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/backend/catalog/pg_publication.c 
b/src/backend/catalog/pg_publication.c
index 17105f4f2c..fccf642605 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -105,6 +105,22 @@ is_publishable_class(Oid relid, Form_pg_class reltuple)
relid >= FirstNormalObjectId;
 }
 
+Datum
+pg_is_relation_publishable(PG_FUNCTION_ARGS)
+{
+   Oid relid = PG_GETARG_OID(0);
+   HeapTuple tuple;
+   bool result;
+
+   tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+   if (!tuple)
+   PG_RETURN_NULL();
+   result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple));
+   ReleaseSysCache(tuple);
+   PG_RETURN_BOOL(result);
+}
+
+
 /*
  * Insert new publication / relation mapping.
  */
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 0fdad0c119..895b4001b7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -255,12 +255,23 @@ CREATE VIEW pg_stats WITH (security_barrier) AS
 
 CREATE VIEW pg_publication_tables AS
 SELECT
-P.pubname AS pubname,
-N.nspname AS schemaname,
-C.relname AS tablename
-FROM pg_publication P, pg_class C
- JOIN pg_namespace N ON (N.oid = C.relnamespace)
-WHERE C.oid IN (SELECT relid FROM 

Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[Michael Paquier]

On Fri, Jun 16, 2017 at 11:04 AM, QL Zhuo  wrote:
> I just put this line in my postgresql.conf:
>
> ```
> shared_preload_libraries = '/Path/Contains/UpCaseWords/an_ext.so'
> ```
>
> Then the server couldn't start. It tried to load the file
> "/path/contains/upcasewords/an_ext.so" and failed.
>
> After few digging, I found there's a wrong use of `SplitIdentifierString` in
> function `load_libraries` in /src/backend/utils/init/miscinit.c, and the
> attached patch fixes it.

That's a good catch. All the other callers of SplitIdentifierString()
don't handle a list of directories. This requires a back-patch.
-- 
Michael


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

[Thomas Munro]

On Fri, Jun 16, 2017 at 1:24 PM, Craig Ringer  wrote:
> On 16 June 2017 at 05:42, Thomas Munro  wrote:
>> Bumping this ancient thread to say that DECFLOAT appears to have
>> landed in the SQL standard.  I haven't looked at SQL:2016 myself by I
>> just saw this on Markus Winand's Modern SQL blog:
>>
>> "There is a new type decfloat[()] (T076)."
>>
>> http://modern-sql.com/blog/2017-06/whats-new-in-sql-2016
>>
>> So far it's supported only by DB2 (inventor) and FirebirdSQL has just
>> announced support in the next release.
>
> I was pretty excited by decimal floating point initially, but the lack
> of support for its use in hardware in commonplace CPUs makes me less
> thrilled. IIRC Intel was talking about adding it, but I can't find any
> references to that anymore. POWER6 and POWER7 has it, which is great,
> but hardly justifies a push for getting it into the core Pg.
>
> Some of the discussion on
> https://software.intel.com/en-us/articles/intel-decimal-floating-point-math-library?page=1
> suggests that doing it fully in hardware is very expensive, so a mixed
> software/microcode implementation with some hardware assistance is
> likely if/when it comes.

There are considerations other than raw arithmetic performance though:

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.

2.  They may be increasingly used as 'vocabulary' datatypes as more
languages and databases adopt them.  That's probably not a huge
semantic problem since DECIMAL can represent most useful DECFLOAT
values exactly (but not some IEEE 754 quirks like -0, -inf, +inf, NaN,
and I haven't checked what SQL:2016 says about that anyway but if it's
based directly on IEEE 754:2008 then I guess they'll be in there).

I don't understand these things but it looks like the support in C
(originally proposed as N1312 and implemented by IBM, HP, GCC and
Intel compilers) has reached the next stage and been published as
ISO/IEC TS 18661-2:2015, and now N2079 proposes that TS 18661-2 be
absorbed into C2x (!).  As glacial as ISO processes may be, it's
encouraging that there is now a TS even though I'm not allowed to
download it without paying CHF178.  Meanwhile Python and others just
did it (albeit vastly less efficiently).

-- 
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] WIP: Data at rest encryption

[Tatsuo Ishii]

> Yeah, I guess we will just have to wait to see it since other people are
> excited about it.  My concern is code complexity and usability
> challenges, vs punting the problem to the operating system, though
> admittedly there are some cases where that is not possible.

Oracle sells this feature only with the expensive enterprise
edition. And people actually buy it. I guess the feature is pretty
important for some users.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


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

[HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[QL Zhuo]

I just put this line in my postgresql.conf:

```
shared_preload_libraries = '/Path/Contains/UpCaseWords/an_ext.so'
```

Then the server couldn't start. It tried to load the file
"/path/contains/upcasewords/an_ext.so" and failed.

After few digging, I found there's a wrong use of `SplitIdentifierString`
in function `load_libraries` in /src/backend/utils/init/miscinit.c, and the
attached patch fixes it.


--
This email address (zhuo.devgmail.com) is only for development affairs,
e.g. mail list, please mail to zhuohexoasis.com or zhuoqlzoho.com
for other purpose.

ZHUO QL (KDr2), http://kdr2.com


0001-Don-t-downcase-filepath-in-load_libraries.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] Get stuck when dropping a subscription during synchronizing table

[Peter Eisentraut]

On 6/15/17 12:22, Petr Jelinek wrote:
> On 15/06/17 17:53, Peter Eisentraut wrote:
>> On 6/14/17 18:35, Petr Jelinek wrote:
>>> Attached fixes it (it was mostly about order of calls).
>> So do I understand this right that the actual fix is just moving up the
>> logicalrep_worker_stop() call in DropSubscription().
>>
> No the fix is heap_open before SearchSysCache().

The existing code already does that.

-- 
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] Broken hint bits (freeze)

[Sergey Burladyan]

Bruce Momjian  writes:

> !  against the old primary and standby clusters.  Verify that the
> !  Latest checkpoint location values match in all clusters.

For "Log-Shipping only" standby server this cannot be satisfied, because
last WAL from master (with shutdown checkpoint) never archived. For
example (git master):
 postgresql.conf ===
port = 5430
shared_buffers = 32MB
wal_level = hot_standby
archive_mode = on
archive_command = 'test ! -f "$ARH/%f" && ( echo "arch %p"; cp %p "$ARH/%f"; )'
max_wal_senders = 5
hot_standby = on
log_line_prefix = '%t '
log_checkpoints = on
lc_messages = C


 pg_control 
pg_control version number:1002
Catalog version number:   201705301
Database system identifier:   6432034080221219745
Database cluster state:   shut down
pg_control last modified: Fri Jun 16 03:57:22 2017
Latest checkpoint location:   0/D28
Prior checkpoint location:0/1604878
Latest checkpoint's REDO location:0/D28
Latest checkpoint's REDO WAL file:0001000D


 WALs archive 
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010003
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010004
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010005
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010006
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010007
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010008
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010009
-rw--- 1 sergey users 16777216 Jun 16 03:57 0001000A
-rw--- 1 sergey users 16777216 Jun 16 03:57 0001000B
-rw--- 1 sergey users 16777216 Jun 16 03:57 0001000C
==

 logfile 
arch pg_wal/0001000A
arch pg_wal/0001000B
2017-06-16 00:57:21 GMT LOG:  received fast shutdown request
2017-06-16 00:57:21 GMT LOG:  aborting any active transactions
2017-06-16 00:57:21 GMT LOG:  shutting down
arch pg_wal/0001000C
2017-06-16 00:57:21 GMT LOG:  checkpoint starting: shutdown immediate
2017-06-16 00:57:22 GMT LOG:  checkpoint complete: wrote 4058 buffers (99.1%); 
0 WAL file(s) added, 0 removed, 0 recycled; write=0.033 s, sync=0.949 s, 
total=1.144 s; sync files=32, longest=0.598 s, average=0.029 s; distance=190445 
kB, estimate=190445 kB
2017-06-16 00:57:22 GMT LOG:  database system is shut down
=

There is no 0001000D in archive and after shutdown,
standby can only be at it previous restartpoint (0/1604878) because it
does not receive latest checkpoint (0/D28) from master.

So, after shutdown master and "Log-Shipping only" standby, it always "one
checkpoint early" then master and "Latest checkpoint location" never
match for it.

I think this must be mentioned somehow in documentation. 


> ! 
> !  Also, if upgrading standby servers, change wal_level
> !  to replica in the postgresql.conf file on
> !  the new cluster.
>   
>  

I am not sure how this help.

wal_level is reset by pg_resetxlog during pg_upgrade, so it does not
depend on postgresql.conf. After pg_upgrade wal_level always is
'minimal', that is why you must start and stop new master before rsync:

 output 
$ "$bin"/pg_controldata "$ver" | grep wal_level
wal_level setting:replica

$ "$bin"/pg_resetwal "$ver"
Write-ahead log reset

$ "$bin"/pg_controldata "$ver" | grep wal_level
wal_level setting:minimal


If you rsync standby now (without start/stop new master after pg_upgrade)
you will send pg_control with wal_level=minimal into it and after that
standby abort on startup:
 standby logfile 
2017-06-16 01:22:14 GMT LOG:  entering standby mode
2017-06-16 01:22:14 GMT WARNING:  WAL was generated with wal_level=minimal, 
data may be missing
2017-06-16 01:22:14 GMT HINT:  This happens if you temporarily set 
wal_level=minimal without taking a new base backup.
2017-06-16 01:22:14 GMT FATAL:  hot standby is not possible because wal_level 
was not set to "replica" or higher on the master server
2017-06-16 01:22:14 GMT HINT:  Either set wal_level to "replica" on the master, 
or turn off hot_standby here.
2017-06-16 01:22:14 GMT LOG:  startup process (PID 27916) exited with exit code 
1
=

PS: Thank you for answer, Bruce!

-- 
Sergey Burladyan


-- 
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 read-only slave

[Craig Ringer]

On 15 June 2017 at 23:12, Maeldron T.  wrote:

> I could send an explicit command for each session to make it read-only
> I could use a read-only role (let’s ignore now I don’t use rules)

You can also set the GUC default_transaction_read_only = on.

But apps can easily clobber that with explicit read/write begin.
Setting it in combination with a role that doesn't have any write
permissions would be sufficient for most practical situations IMO.

> The DDL could be applied in a specific session as whitelisting is safer than
> blacklisting. I think the only missing part is if the subscription could
> turn on the writes for itself.
>
> If you think this would make sense, please consider it.

BDR has the option of marking a node as read-only, which is
implemented using an ExecutorStart_hook. It probably wouldn't be
overly hard to do the same thing as a standalone extension. You'd want
to detect when you were running within a logical replication apply
worker and permit changes then, but I don't expect that'd be unduly
hard.

It'd be nice to have a built-in way to do this, so maybe you could
pursue that for postgresql 11, raising a firm design idea here and
following up with a patch if you get a reasonable approximation of
consensus.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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] Decimal64 and Decimal128

[Craig Ringer]

On 16 June 2017 at 05:42, Thomas Munro  wrote:
> On Fri, Sep 25, 2015 at 5:06 PM, Pavel Stehule  
> wrote:
>> 2015-09-25 0:25 GMT+02:00 Jim Nasby :
>>>
>>> On 9/24/15 3:35 PM, Peter Geoghegan wrote:

 I would worry about the implicit casts you've added. They might cause
 problems.
>>>
>>>
>>> Given the cycle created between numeric->decimal and decimal->numeric, I
>>> can pretty much guarantee they will. In any case, I don't think implicit
>>> casting from numeric->decimal is a good idea since it can overflow. I'm not
>>> sure that the other direction is safe either... I can't remember offhand if
>>> casting correctly obeys typmod or not.
>>>
>>> BTW, have you talked to Pavel about making these changes to his code?
>>> Seems a shame to needlessly fork it. :/
>>
>>
>> yes, he talked with me, and I gave a agreement to continue/enhance/fork this
>> project how will be necessary
>
> Bumping this ancient thread to say that DECFLOAT appears to have
> landed in the SQL standard.  I haven't looked at SQL:2016 myself by I
> just saw this on Markus Winand's Modern SQL blog:
>
> "There is a new type decfloat[()] (T076)."
>
> http://modern-sql.com/blog/2017-06/whats-new-in-sql-2016
>
> So far it's supported only by DB2 (inventor) and FirebirdSQL has just
> announced support in the next release.

I was pretty excited by decimal floating point initially, but the lack
of support for its use in hardware in commonplace CPUs makes me less
thrilled. IIRC Intel was talking about adding it, but I can't find any
references to that anymore. POWER6 and POWER7 has it, which is great,
but hardly justifies a push for getting it into the core Pg.

Some of the discussion on
https://software.intel.com/en-us/articles/intel-decimal-floating-point-math-library?page=1
suggests that doing it fully in hardware is very expensive, so a mixed
software/microcode implementation with some hardware assistance is
likely if/when it comes.

--
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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] Huge pages support on Windows

[Tsunakawa, Takayuki]

Hello, Andrea

From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Andrea caldarone
> Anyone can point me in the right direction?

Thank you for your interest.

1. Download the PostgreSQL 10 source code (which is still in development), 
which is the file named postgresql-snapshot.tar.gz from here:

https://ftp.postgresql.org/pub/snapshot/dev/

2. Get the latest patch for huge page support for Windows in the mail thread 
you saw.  The file name is win_large_pages_v13.patch.

3. Decompress the source code and apply the patch.
$ tar zxf postgresql-snapshot.tar.gz
$ cd postgresql*
$ patch -p1 < win_large_pages_v13.patch

Now you can build and install the program normally.

Regards
Takayuki Tsunakawa


-- 
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: Data at rest encryption

[Bruce Momjian]

On Thu, Jun 15, 2017 at 04:56:36PM -0700, Andres Freund wrote:
> On 2017-06-15 19:44:43 -0400, Bruce Momjian wrote:
> > Understood, but now you are promoting a feature with an admittedly-poor
> > API, duplication of an OS feature, and perhaps an invasive change to the
> > code.
> 
> *Perhaps* an invasive change to the code?  To me it's pretty evident
> that this'll be a pretty costly feature from that angle. We've quite a
> few places that manipulate on-disk files, and they'll all have to be
> manipulated. Several of those are essentially critical sections, adding
> memory allocations to them wouldn't be good, so we'll need
> pre-allocation APIs.
> 
> I've only skimmed the discussion, but based on that I'm very surprised
> how few concerns about this feature's complexity / maintainability
> impact have been raised.

Yeah, I guess we will just have to wait to see it since other people are
excited about it.  My concern is code complexity and usability
challenges, vs punting the problem to the operating system, though
admittedly there are some cases where that is not possible.

-- 
  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] WIP: Data at rest encryption

[Andres Freund]

On 2017-06-15 19:44:43 -0400, Bruce Momjian wrote:
> Understood, but now you are promoting a feature with an admittedly-poor
> API, duplication of an OS feature, and perhaps an invasive change to the
> code.

*Perhaps* an invasive change to the code?  To me it's pretty evident
that this'll be a pretty costly feature from that angle. We've quite a
few places that manipulate on-disk files, and they'll all have to be
manipulated. Several of those are essentially critical sections, adding
memory allocations to them wouldn't be good, so we'll need
pre-allocation APIs.

I've only skimmed the discussion, but based on that I'm very surprised
how few concerns about this feature's complexity / maintainability
impact have been raised.

- Andres


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

Re: [HACKERS] WIP: Data at rest encryption

[Bruce Momjian]

On Thu, Jun 15, 2017 at 07:51:36PM -0400, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > On Thu, Jun 15, 2017 at 07:27:55PM -0400, Stephen Frost wrote:
> > > I expect the same would happen with the shell-command approach suggested
> > > up-thread and the prompt-on-stdin approach too, they aren't great but I
> > > expect users would still use the feature.  As Robert and I have
> > > mentioned, there is a good bit of value to having this feature simply
> > > because it avoids the need to get someone with root privileges to set up
> > > an encrypted volume and I don't think having to use a shell command or
> > > providing the password on stdin at startup really changes that very
> > > much.
> > 
> > Understood, but now you are promoting a feature with an admittedly-poor
> > API, duplication of an OS feature, and perhaps an invasive change to the
> > code.  Those are high hurdles.
> 
> I thought we called it "incremental development".  From the opposite
> point of view, would you say we should ban use of passphrase-protected
> SSL key files because the current user interface for them is bad?
> 
> I have no use for data-at-rest encryption myself, but I wouldn't stop
> development just because the initial design proposal doesn't include
> top-notch key management.

Yes, but we have to have a plan on how to improve it.  Why add a feature
that is hard to maintain, and hard to use.

-- 
  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] ASOF join

[Thomas Munro]

On Fri, Jun 16, 2017 at 4:20 AM, Konstantin Knizhnik
 wrote:
> I wonder if there were some discussion/attempts to add ASOF join to Postgres
> (sorry, may be there is better term for it, I am refereeing KDB definition:
> http://code.kx.com/wiki/Reference/aj ).

Interesting idea.  Also in Pandas:

http://pandas.pydata.org/pandas-docs/version/0.19.0/generated/pandas.merge_asof.html#pandas.merge_asof

I suppose you could write a function that pulls tuples out of a bunch
of cursors and zips them together like this, as a kind of hand-coded
special merge join "except that we match on nearest key rather than
equal keys" (as they put it).

I've written code like this before in a trading context, where we
called that 'previous tick interpolation', and in a scientific context
where other kinds of interpolation were called for (so not really
matching a tuple but synthesising one if no exact match).  If you view
the former case as a kind of degenerate case of interpolation then it
doesn't feel like a "join" as we know it, but clearly it is.  I had
never considered before that such things might belong inside the
database as a kind of join operator.

-- 
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] WIP: Data at rest encryption

[Stephen Frost]

Bruce,

* Bruce Momjian (br...@momjian.us) wrote:
> On Thu, Jun 15, 2017 at 07:27:55PM -0400, Stephen Frost wrote:
> > I expect the same would happen with the shell-command approach suggested
> > up-thread and the prompt-on-stdin approach too, they aren't great but I
> > expect users would still use the feature.  As Robert and I have
> > mentioned, there is a good bit of value to having this feature simply
> > because it avoids the need to get someone with root privileges to set up
> > an encrypted volume and I don't think having to use a shell command or
> > providing the password on stdin at startup really changes that very
> > much.
> 
> Understood, but now you are promoting a feature with an admittedly-poor
> API, duplication of an OS feature, and perhaps an invasive change to the
> code.  Those are high hurdles.

Of those, the only one that worries me, at least, is that it might be an
invasive and difficult to maintain code change.  As Robert said, and I
agree with, "duplication of an OS feature" is something we pretty
routinly, and justifiably, do.  The poor interface is unfortunate, but
if it's consistent with what we have today for a similar feature then
I'm really not too upset with it.  If we can do better, great, I'm all
for that, but if not, then I'd rather have the feature with the poor
interface than not have it at all.

If it's an invasive code change or one which ends up being difficult to
maintain, then that's a problem.  Getting some focus on that aspect
would be great and I certainly appreciate Robert's initial review and
commentary on it.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] WIP: Data at rest encryption

[Alvaro Herrera]

Bruce Momjian wrote:
> On Thu, Jun 15, 2017 at 07:27:55PM -0400, Stephen Frost wrote:
> > I expect the same would happen with the shell-command approach suggested
> > up-thread and the prompt-on-stdin approach too, they aren't great but I
> > expect users would still use the feature.  As Robert and I have
> > mentioned, there is a good bit of value to having this feature simply
> > because it avoids the need to get someone with root privileges to set up
> > an encrypted volume and I don't think having to use a shell command or
> > providing the password on stdin at startup really changes that very
> > much.
> 
> Understood, but now you are promoting a feature with an admittedly-poor
> API, duplication of an OS feature, and perhaps an invasive change to the
> code.  Those are high hurdles.

I thought we called it "incremental development".  From the opposite
point of view, would you say we should ban use of passphrase-protected
SSL key files because the current user interface for them is bad?

I have no use for data-at-rest encryption myself, but I wouldn't stop
development just because the initial design proposal doesn't include
top-notch key management.

-- 
Álvaro Herrerahttps://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] WIP: Data at rest encryption

[Bruce Momjian]

On Thu, Jun 15, 2017 at 07:27:55PM -0400, Stephen Frost wrote:
> I expect the same would happen with the shell-command approach suggested
> up-thread and the prompt-on-stdin approach too, they aren't great but I
> expect users would still use the feature.  As Robert and I have
> mentioned, there is a good bit of value to having this feature simply
> because it avoids the need to get someone with root privileges to set up
> an encrypted volume and I don't think having to use a shell command or
> providing the password on stdin at startup really changes that very
> much.

Understood, but now you are promoting a feature with an admittedly-poor
API, duplication of an OS feature, and perhaps an invasive change to the
code.  Those are high hurdles.

-- 
  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] WIP: Data at rest encryption

[Stephen Frost]

Bruce,

* Bruce Momjian (br...@momjian.us) wrote:
> On Thu, Jun 15, 2017 at 06:41:08PM -0400, Stephen Frost wrote:
> > > > > One serious difference between in-database-encryption and SSH keys is
> > > > > that the use of passwords for SSH is well understood and reasonable to
> > > > > use, while I think we all admit that use of passwords for database
> > > > > objects like SSL keys is murky.  Use of keys for OS-level encryption 
> > > > > is
> > > > > a little better handled, but not as clean as SSH keys.
> > > > 
> > > > Peter pointed out upthread that our handling of SSL passphrases leaves
> > > > a lot to be desired, and that maybe we should fix that problem first;
> > > > I agree.  But I don't think this is any kind of intrinsic limitation
> > > > of PostgreSQL vs. encrypted filesystems vs. SSH; it's just a
> > > > quality-of-implementation issue.
> > 
> > I'm not thrilled with asking Ants to implement a solution to SSL
> > passphrases, and generalizing it to work for this, to get this feature
> > accepted.  I assume that the reason for asking for that work to be done
> > now is because we decided that the current approach for SSL sucks but we
> > couldn't actually drop support for it, but we don't want to add other
> > features which work in a similar way because, well, it sucks.
> 
> My point is that if our support for db-level encryption is as bad as SSL
> key passwords, then it will be nearly useless, so we might as well not
> have it.  Isn't that obvious?

Well, no, because the reason we even have an approach at all for SSL key
passwords is because multiple people (myself and Magnus, at least, as I
recall) complained as we are aware of installations which are actively
using that approach.

That doesn't mean it's a great solution or that it doesn't suck- really,
I tend to agree that it does, but it's necessary because we need a
solution, it works, and users are using it.  Having a better solution
would be great and something agent-based might be the right answer (or
perhaps something where we have support for using hardware accellerators
through an existing library...).

I expect the same would happen with the shell-command approach suggested
up-thread and the prompt-on-stdin approach too, they aren't great but I
expect users would still use the feature.  As Robert and I have
mentioned, there is a good bit of value to having this feature simply
because it avoids the need to get someone with root privileges to set up
an encrypted volume and I don't think having to use a shell command or
providing the password on stdin at startup really changes that very
much.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] WIP: Data at rest encryption

[Bruce Momjian]

On Thu, Jun 15, 2017 at 06:41:08PM -0400, Stephen Frost wrote:
> > > > One serious difference between in-database-encryption and SSH keys is
> > > > that the use of passwords for SSH is well understood and reasonable to
> > > > use, while I think we all admit that use of passwords for database
> > > > objects like SSL keys is murky.  Use of keys for OS-level encryption is
> > > > a little better handled, but not as clean as SSH keys.
> > > 
> > > Peter pointed out upthread that our handling of SSL passphrases leaves
> > > a lot to be desired, and that maybe we should fix that problem first;
> > > I agree.  But I don't think this is any kind of intrinsic limitation
> > > of PostgreSQL vs. encrypted filesystems vs. SSH; it's just a
> > > quality-of-implementation issue.
> 
> I'm not thrilled with asking Ants to implement a solution to SSL
> passphrases, and generalizing it to work for this, to get this feature
> accepted.  I assume that the reason for asking for that work to be done
> now is because we decided that the current approach for SSL sucks but we
> couldn't actually drop support for it, but we don't want to add other
> features which work in a similar way because, well, it sucks.

My point is that if our support for db-level encryption is as bad as SSL
key passwords, then it will be nearly useless, so we might as well not
have it.  Isn't that obvious?

-- 
  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] WIP: Data at rest encryption

[Stephen Frost]

Bruce,

* Bruce Momjian (br...@momjian.us) wrote:
> On Thu, Jun 15, 2017 at 05:04:17PM -0400, Robert Haas wrote:
> > > Also, there is the sense that security requires
> > > trust of the root user, while using Postgres doesn't require the root
> > > user to also use Postgres.
> > 
> > I don't understand this.  It is certainly true that you're running
> > binaries owned by root, the root user could Trojan the binaries and
> > break any security you think you have.  But that problem is no better
> > or worse for PostgreSQL than anything else.
> 
> I couldn't find a cleaner way to see it --- it is that database use
> doesn't involve the root user using it, while database security requires
> the root user to also be security-conscious.

I tend to agree with Robert here (in general).  Further, there are
certainly environments where the administrator with root access is
absolutely security conscious, but that doesn't mean that the DBA has
easy access to the root account or to folks who have that level of
access and it's much easier for the DBA if they're able to address all
of their requirements by building PG themselves, installing it, and,
ideally, encrypting the DB.

> > > One serious difference between in-database-encryption and SSH keys is
> > > that the use of passwords for SSH is well understood and reasonable to
> > > use, while I think we all admit that use of passwords for database
> > > objects like SSL keys is murky.  Use of keys for OS-level encryption is
> > > a little better handled, but not as clean as SSH keys.
> > 
> > Peter pointed out upthread that our handling of SSL passphrases leaves
> > a lot to be desired, and that maybe we should fix that problem first;
> > I agree.  But I don't think this is any kind of intrinsic limitation
> > of PostgreSQL vs. encrypted filesystems vs. SSH; it's just a
> > quality-of-implementation issue.

I'm not thrilled with asking Ants to implement a solution to SSL
passphrases, and generalizing it to work for this, to get this feature
accepted.  I assume that the reason for asking for that work to be done
now is because we decided that the current approach for SSL sucks but we
couldn't actually drop support for it, but we don't want to add other
features which work in a similar way because, well, it sucks.

I get that.  I'm not thrilled with it, but I get it.  I'm hopeful it
ends up not being too bad, but if it ends up meaning we don't get this
feature, then I'll reconsider my position about agreeing that it's an
acceptable requirement.

> I think there are environmental issues that make password use on SSH
> easier than the other cases --- it isn't just code quality.  However, it
> would be good to research how SSH handles it to see if we can get any
> ideas.

Actually, the approach SSH uses is a really good one, imv, and one which
we might be able to leverage..  I'm not sure though.  I will say that,
in general, I like the idea of leveraging the external libraries which
handle keys and deal with encryption to make this happen as those allow
things like hardware devices to hold the key and possibly perform the
encryption/decryption, etc.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] subscription worker signalling wal writer too much

[Jeff Janes]

On Wed, Jun 14, 2017 at 4:29 PM, Andres Freund  wrote:

> On 2017-06-14 16:24:27 -0700, Jeff Janes wrote:
> > On Wed, Jun 14, 2017 at 3:20 PM, Andres Freund 
> wrote:
> >
> > > On 2017-06-14 15:08:49 -0700, Jeff Janes wrote:
> > > > On Wed, Jun 14, 2017 at 11:55 AM, Jeff Janes 
> > > wrote:
> > > >
> > > > > If I publish a pgbench workload and subscribe to it, the
> subscription
> > > > > worker is signalling the wal writer thousands of times a second,
> once
> > > for
> > > > > every async commit.  This has a noticeable performance cost.
> > > > >
> > > >
> > > > I've used a local variable to avoid waking up the wal writer more
> than
> > > once
> > > > for the same page boundary.  This reduces the number of wake-ups by
> about
> > > > 7/8.
> > >
> > > Maybe I'm missing something here, but isn't that going to reduce our
> > > guarantees about when asynchronously committed xacts are flushed out?
> > > You can easily fit a number of commits into the same page...   As this
> > > isn't specific to logical-rep, I don't think that's ok.
> > >
> >
> > The guarantee is based on wal_writer_delay not on SIGUSR1, so I don't
> think
> > this changes that. (Also, it isn't really a guarantee, the fsync can take
> > many seconds to complete once we do initiate it, and there is absolutely
> > nothing we can do about that, other than do the fsync synchronously in
> the
> > first place).
>
> Well, wal_writer_delay doesn't work if walwriter is in sleep mode, and
> this afaics would allow wal writer to go into sleep mode with half a
> page filled, and it'd not be woken up again until the page is filled.
> No?
>

If it is taking the big sleep, then we always wake it up, since
acd4c7d58baf09f.

I didn't change that part.  I only changed what we do if it not hibernating.


> > > Have you chased down why there's that many wakeups?  Normally I'd have
> > > expected that a number of the SetLatch() calls get consolidated
> > > together, but I guess walwriter is "too quick" in waking up and
> > > resetting the latch?
>
> > I'll have to dig into that some more.  The 7/8 reduction I cited was just
> > in calls to SetLatch from that part of the code, I didn't measure whether
> > the SetLatch actually called kill(owner_pid, SIGUSR1) or not when I
> > determined that reduction, so it wasn't truly wake ups I measured.
> Actual
> > wake ups were measured only indirectly via the impact on performance.
> I'll
> > need to figure out how to instrument that without distorting the
> > performance too much in the process..
>
> I'd suspect that just measuring the number of kill() calls should be
> doable, if measured via perf or something like hta.t
>


It looks like only limited consolidation was happening, the number of kills
invoked was more than half of the number of SetLatch.  I think the reason
is what when kill(owner_pid, SIGUSR1) is called, the kernel immediately
suspends the calling process and gives the signalled process a chance to
run in that time slice.  The Wal Writer gets woken up, sees that it only
has a partial page to write and decides not to do anything, but resets the
latch.  It does this fast enough that the subscription worker hasn't
migrated CPUs yet.

But, if the WAL Writer sees it has already flushed up to the highest
eligible page boundary, why doesn't the SetAsync code see the same thing?
Because it isn't flushed that high, but only written that high.  It looks
like a story of three git commits:

commit 4de82f7d7c50a81ec8e70e2cb0ab413ab9134c0b
Author: Simon Riggs 
Date:   Sun Nov 13 09:00:57 2011 +

Wakeup WALWriter as needed for asynchronous commit performance.


commit db76b1efbbab2441428a9ef21f7ac9ba43c52482
Author: Andres Freund 
Date:   Mon Feb 15 22:15:35 2016 +0100

Allow SetHintBits() to succeed if the buffer's LSN is new enough.


commit 7975c5e0a992ae9a45e03d145e0d37e2b5a707f5
Author: Andres Freund 
Date:   Mon Feb 15 23:52:38 2016 +0100

Allow the WAL writer to flush WAL at a reduced rate.


The first change made the WALWriter wake up and do a write and flush
whenever an async commit occurred and there was a completed WAL page to be
written.  This way the hint bits could be set while the heap page was still
hot, because they couldn't get set until the WAL covering the hinted-at
transaction commit is flushed.

The second change said we can set hint bits before the WAL covering the
hinted-at transaction is flushed, as long the page LSN is newer than that
(so the wal covering the hinted-at transaction commit must be flushed
before the page containing the hint bit can be written).

Then the third change makes the wal writer only write the full pages most
of the times it is woken up, not flush them.  It only flushes them once
every wal_writer_delay or wal_writer_flush_after, regardless of how often
it is woken up.

It seems like the third change rendered the first one 

Re: [HACKERS] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 5:16 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Jun 15, 2017 at 5:06 PM, Tom Lane  wrote:
>>> ... nodeGather cannot deem the query done until it's seen EOF on
>>> each tuple queue, which it cannot see until each worker has attached
>>> to and then detached from the associated shm_mq.
>
>> Oh.  That's sad.  It definitely has to wait for any tuple queues that
>> have been attached to be detached, but it would be better if it didn't
>> have to wait for processes that haven't even attached yet.
>
> Hm.  We assume they attach before they start taking any of the query
> work?  Seems reasonable, and this would give us some chance of recovering
> from worker fork failure.

Yeah, something like that.  I'm not sure exactly how to implement it,
though.  I think I intended for it to work that way all along, but the
code's not there.

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

[Thomas Munro]

On Fri, Sep 25, 2015 at 5:06 PM, Pavel Stehule  wrote:
> 2015-09-25 0:25 GMT+02:00 Jim Nasby :
>>
>> On 9/24/15 3:35 PM, Peter Geoghegan wrote:
>>>
>>> I would worry about the implicit casts you've added. They might cause
>>> problems.
>>
>>
>> Given the cycle created between numeric->decimal and decimal->numeric, I
>> can pretty much guarantee they will. In any case, I don't think implicit
>> casting from numeric->decimal is a good idea since it can overflow. I'm not
>> sure that the other direction is safe either... I can't remember offhand if
>> casting correctly obeys typmod or not.
>>
>> BTW, have you talked to Pavel about making these changes to his code?
>> Seems a shame to needlessly fork it. :/
>
>
> yes, he talked with me, and I gave a agreement to continue/enhance/fork this
> project how will be necessary

Bumping this ancient thread to say that DECFLOAT appears to have
landed in the SQL standard.  I haven't looked at SQL:2016 myself by I
just saw this on Markus Winand's Modern SQL blog:

"There is a new type decfloat[()] (T076)."

http://modern-sql.com/blog/2017-06/whats-new-in-sql-2016

So far it's supported only by DB2 (inventor) and FirebirdSQL has just
announced support in the next release.

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

[Kevin Grittner]

On Thu, Jun 15, 2017 at 4:18 PM, Alvaro Herrera
 wrote:
> Kevin Grittner wrote:

> As far as I understand her proposal, it is exactly the opposite -- if a
> transaction fails, it is discarded.  And this P.S. note is asking
> whether this is a good idea, or would we prefer that failing
> transactions are retried.
>
> I think it's pretty obvious that transactions that failed with
> some serializability problem should be retried.

Agreed all around.

-- 
Kevin Grittner
VMware vCenter Server
https://www.vmware.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] WIP Patch: Pgbench Serialization and deadlock errors

[Thomas Munro]

On Fri, Jun 16, 2017 at 9:18 AM, Alvaro Herrera
 wrote:
> Kevin Grittner wrote:
>> On Thu, Jun 15, 2017 at 2:16 PM, Andres Freund  wrote:
>> > On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:
>
>> >> P.S. Does this use case (do not retry transaction with serialization or
>> >> deadlock failure) is most interesting or failed transactions should be
>> >> retried (and how much times if there seems to be no hope of success...)?
>> >
>> > I can't quite parse that sentence, could you restate?
>>
>> The way I read it was that the most interesting solution would retry
>> a transaction from the beginning on a serialization failure or
>> deadlock failure.
>
> As far as I understand her proposal, it is exactly the opposite -- if a
> transaction fails, it is discarded.  And this P.S. note is asking
> whether this is a good idea, or would we prefer that failing
> transactions are retried.
>
> I think it's pretty obvious that transactions that failed with
> some serializability problem should be retried.

+1 for retry with reporting of retry rates

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

[Alvaro Herrera]

Kevin Grittner wrote:
> On Thu, Jun 15, 2017 at 2:16 PM, Andres Freund  wrote:
> > On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:

> >> P.S. Does this use case (do not retry transaction with serialization or
> >> deadlock failure) is most interesting or failed transactions should be
> >> retried (and how much times if there seems to be no hope of success...)?
> >
> > I can't quite parse that sentence, could you restate?
> 
> The way I read it was that the most interesting solution would retry
> a transaction from the beginning on a serialization failure or
> deadlock failure.

As far as I understand her proposal, it is exactly the opposite -- if a
transaction fails, it is discarded.  And this P.S. note is asking
whether this is a good idea, or would we prefer that failing
transactions are retried.

I think it's pretty obvious that transactions that failed with
some serializability problem should be retried.

-- 
Álvaro Herrerahttps://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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> On Thu, Jun 15, 2017 at 5:06 PM, Tom Lane  wrote:
>> ... nodeGather cannot deem the query done until it's seen EOF on
>> each tuple queue, which it cannot see until each worker has attached
>> to and then detached from the associated shm_mq.

> Oh.  That's sad.  It definitely has to wait for any tuple queues that
> have been attached to be detached, but it would be better if it didn't
> have to wait for processes that haven't even attached yet.

Hm.  We assume they attach before they start taking any of the query
work?  Seems reasonable, and this would give us some chance of recovering
from worker fork failure.

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] WIP: Data at rest encryption

[Bruce Momjian]

On Thu, Jun 15, 2017 at 05:04:17PM -0400, Robert Haas wrote:
> > Also, there is the sense that security requires
> > trust of the root user, while using Postgres doesn't require the root
> > user to also use Postgres.
> 
> I don't understand this.  It is certainly true that you're running
> binaries owned by root, the root user could Trojan the binaries and
> break any security you think you have.  But that problem is no better
> or worse for PostgreSQL than anything else.

I couldn't find a cleaner way to see it --- it is that database use
doesn't involve the root user using it, while database security requires
the root user to also be security-conscious.

> > One serious difference between in-database-encryption and SSH keys is
> > that the use of passwords for SSH is well understood and reasonable to
> > use, while I think we all admit that use of passwords for database
> > objects like SSL keys is murky.  Use of keys for OS-level encryption is
> > a little better handled, but not as clean as SSH keys.
> 
> Peter pointed out upthread that our handling of SSL passphrases leaves
> a lot to be desired, and that maybe we should fix that problem first;
> I agree.  But I don't think this is any kind of intrinsic limitation
> of PostgreSQL vs. encrypted filesystems vs. SSH; it's just a
> quality-of-implementation issue.

I think there are environmental issues that make password use on SSH
easier than the other cases --- it isn't just code quality.  However, it
would be good to research how SSH handles it to see if we can get any
ideas.

-- 
  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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 5:06 PM, Tom Lane  wrote:
> I wrote:
>> Robert Haas  writes:
>>> I think you're right.  So here's a theory:
>
>>> 1. The ERROR mapping the DSM segment is just a case of the worker the
>>> losing a race, and isn't a bug.
>
>> I concur that this is a possibility,
>
> Actually, no, it isn't.  I tried to reproduce the problem by inserting
> a sleep into ParallelWorkerMain, and could not.  After digging around
> in the code, I realize that the leader process *can not* exit the
> parallel query before the workers start, at least not without hitting
> an error first, which is not happening in these examples.  The reason
> is that nodeGather cannot deem the query done until it's seen EOF on
> each tuple queue, which it cannot see until each worker has attached
> to and then detached from the associated shm_mq.

Oh.  That's sad.  It definitely has to wait for any tuple queues that
have been attached to be detached, but it would be better if it didn't
have to wait for processes that haven't even attached yet.

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

[HACKERS] pg_waldump command line arguments

[Robert Haas]

pg_waldump --help claims that you run it like this:

Usage:
  pg_waldump [OPTION]... [STARTSEG [ENDSEG]]

And https://www.postgresql.org/docs/10/static/pgwaldump.html agrees.
Since square brackets indicate optional arguments, this sort of makes
it sound like running pg_waldump with no arguments ought to work.  But
it doesn't:

$ pg_waldump
pg_waldump: no arguments specified
Try "pg_waldump --help" for more information.

If we removed the error check that displays "pg_waldump: no arguments
specified", then it would still fail, but with a more useful error
message:

$ pg_waldump --
pg_waldump: no start WAL location given
Try "pg_waldump --help" for more information.

That message ought to perhaps be changed to say that you specified
neither the start WAL location nor the start WAL file, but even as it
stands it's certainly better than "no arguments specified".

Another problem is that if the file name you pass to pg_waldump
doesn't happen to have a name that looks like a WAL file, it fails in
a completely ridiculous fashion:

$ pg_waldump /etc/passwd
pg_waldump: FATAL:  could not find file "00017C55C16F00FF": No
such file or directory

The problem appears to be that fuzzy_open_file() successfully opens
the file and then invokes XLogFromFileName() on the filename.
XLogFromFileName() calls sscanf() on the file name without any error
checking, which I think results in leaving private.timeline
uninitialized and setting segno to whatever preexisting garbage was in
the log and segno variables declared inside XLogFromFileName(),
resulting in an attempt to find a more or less completely random file.

A slightly broader concern is whether we need to require the start
position at all.  It seems like one could locate the WAL directory
using the existing logic, then search for the earliest file.  It might
be a little unclear what "earliest" means when multiple timelines are
present, but I bet we could come up with some behavior that would be
convenient for most users.  It would be quite handy to be able to run
this without arguments (or just with -z) and have it process all the
WAL files that you've got on hand.

-- 
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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

I wrote:
> Robert Haas  writes:
>> I think you're right.  So here's a theory:

>> 1. The ERROR mapping the DSM segment is just a case of the worker the
>> losing a race, and isn't a bug.

> I concur that this is a possibility,

Actually, no, it isn't.  I tried to reproduce the problem by inserting
a sleep into ParallelWorkerMain, and could not.  After digging around
in the code, I realize that the leader process *can not* exit the
parallel query before the workers start, at least not without hitting
an error first, which is not happening in these examples.  The reason
is that nodeGather cannot deem the query done until it's seen EOF on
each tuple queue, which it cannot see until each worker has attached
to and then detached from the associated shm_mq.

(BTW, this also means that the leader is frozen solid if a worker
process fails to start, but we knew that already.)

So we still don't know why lorikeet is sometimes reporting "could not map
dynamic shared memory segment".  It's clear though that once that happens,
the current code has no prayer of recovering cleanly.  It looks from
lorikeet's logs like there is something that is forcing a timeout via
crash after ~150 seconds, but I do not know what that is.

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] WIP: Data at rest encryption

[Robert Haas]

On Thu, Jun 15, 2017 at 4:29 PM, Bruce Momjian  wrote:
> I think the big win for having OS features in the database is
> selectivity --- the ability to selectively apply a feature to part of
> the database.  This is what you are doing by putting a password on your
> SSH key, and my idea about row encryption.

I agree. I think we will eventually want to be able to apply
encryption selectively, although I don't think we need to have that
feature in a first patch.  One problem is that if you don't encrypt
the WAL, there's not much point in encrypting the table data, so it
becomes tricky to encrypt some things and not others.  However, I am
sure we can eventually solve those problems, given enough time and
development effort.

> It is also a question of convenience.  If SSH told users they have to
> create an encrypted volume to store their SSH keys with a password, it
> would be silly, since the files are so small compared to a file system.
> I think the assumption is that any security-concerned deployment of
> Postgres will already have Postgres on its own partition and have the
> root administrator involved.  I think it is this assumption that drives
> the idea that requiring root to run Postgres doesn't make sense, but it
> does to do encryption.

I don't think that's a particularly good assumption, though,
especially with the proliferation of virtual and containerized
environments where access to root privileges tends to be more
circumscribed.  Also, there are a lot of small databases out there
that you might want to be able to encrypt without encrypting
everything on the filesystem.  For example, there are products that
embed PostgreSQL.  If a particular PostgreSQL configuration requires
root access, then using that configuration means that the installing
the product which contains it also requires root access.  Installing
the product means changing /etc/fstab, and uninstalling it means
reversing those changes.  That's very awkward.  I agree that if you've
got a terabyte of sensitive data, you probably want to encrypt the
filesystem and involve the DBA, but there are still people who have a
gigabyte of sensitive data.  For those people, a separate filesystem
likely doesn't make sense, but they may still want encryption.

> Also, there is the sense that security requires
> trust of the root user, while using Postgres doesn't require the root
> user to also use Postgres.

I don't understand this.  It is certainly true that you're running
binaries owned by root, the root user could Trojan the binaries and
break any security you think you have.  But that problem is no better
or worse for PostgreSQL than anything else.

> One serious difference between in-database-encryption and SSH keys is
> that the use of passwords for SSH is well understood and reasonable to
> use, while I think we all admit that use of passwords for database
> objects like SSL keys is murky.  Use of keys for OS-level encryption is
> a little better handled, but not as clean as SSH keys.

Peter pointed out upthread that our handling of SSL passphrases leaves
a lot to be desired, and that maybe we should fix that problem first;
I agree.  But I don't think this is any kind of intrinsic limitation
of PostgreSQL vs. encrypted filesystems vs. SSH; it's just a
quality-of-implementation issue.

-- 
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] Detection of IPC::Run presence in SSL TAP tests

[Tom Lane]

Peter Eisentraut  writes:
> On 6/15/17 15:57, Tom Lane wrote:
>> Pushed, thanks.  I grabbed the very latest copy of ax_prog_perl_modules
>> out of the GNU archives --- it's only cosmetically different, but we
>> might as well be au courant.

> Um, this patch was previously rejected.  Shouldn't we at least discuss
> it, or have it go through the commit fest?  This is not a regression
> against 9.6.

It was rejected only because we didn't want to move the goalposts for
the then-active CMake effort.  That argument no longer applies, I'd say.

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] WIP: Data at rest encryption

[Bruce Momjian]

On Thu, Jun 15, 2017 at 03:09:32PM -0400, Robert Haas wrote:
> To be honest, I find the hostility toward this feature a bit baffling.
> The argument seems to be essentially that we shouldn't have this
> feature because we'd have to maintain the code and many of the same
> goals could be accomplished by using facilities that already exist
> outside the database server.  But that's also true for parallel query
> (cf. Stado), logical replication (cf. Slony, Bucardo, Londiste),
> physical replication (cf. DRBD), partitioning (cf. pg_partman), RLS
> (cf. veil), and anything that could be written as application logic
> (eg. psql's \if ... \endif, every procedural language we have,
> user-defined functions themselves, database-enforced constraints,
> FDWs).  Yet, in every one of those cases, we find it worthwhile to
> have the feature because it works better and is easier to use when
> it's built in.  I don't think that a patch for this feature is likely
> to be bigger than (or even as large as) the patches for logical
> replication or parallel query, and it will probably be less work to
> maintain going forward than either.

I think the big win for having OS features in the database is
selectivity --- the ability to selectively apply a feature to part of
the database.  This is what you are doing by putting a password on your
SSH key, and my idea about row encryption.

It is also a question of convenience.  If SSH told users they have to
create an encrypted volume to store their SSH keys with a password, it
would be silly, since the files are so small compared to a file system. 
I think the assumption is that any security-concerned deployment of
Postgres will already have Postgres on its own partition and have the
root administrator involved.  I think it is this assumption that drives
the idea that requiring root to run Postgres doesn't make sense, but it
does to do encryption.  Also, there is the sense that security requires
trust of the root user, while using Postgres doesn't require the root
user to also use Postgres.

One serious difference between in-database-encryption and SSH keys is
that the use of passwords for SSH is well understood and reasonable to
use, while I think we all admit that use of passwords for database
objects like SSL keys is murky.  Use of keys for OS-level encryption is
a little better handled, but not as clean as SSH keys.

I admit there is no hard line here, so I guess we will have to see what
the final patch looks like.  I am basing my statements on what I guess
the complexity will be.  Complexity has a cost so we will have to weigh
it when we see it.  When SSH added password access, it was probably an
easy decision because the use-case was high and the complexity was low.

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

[Kevin Grittner]

On Thu, Jun 15, 2017 at 2:16 PM, Andres Freund  wrote:
> On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:

>> I suggest a patch where pgbench client sessions are not disconnected because
>> of serialization or deadlock failures and these failures are mentioned in
>> reports.
>
> I think that's a good idea and sorely needed.

+1

>> P.S. Does this use case (do not retry transaction with serialization or
>> deadlock failure) is most interesting or failed transactions should be
>> retried (and how much times if there seems to be no hope of success...)?
>
> I can't quite parse that sentence, could you restate?

The way I read it was that the most interesting solution would retry
a transaction from the beginning on a serialization failure or
deadlock failure.  Most people who use serializable transactions (at
least in my experience) run though a framework that does that
automatically, regardless of what client code initiated the
transaction.  These retries are generally hidden from the client
code -- it just looks like the transaction took a bit longer.
Sometimes people will have a limit on the number of retries.  I
never used such a limit and never had a problem, because our
implementation of serializable transactions will not throw a
serialization failure error until one of the transactions involved
in causing it has successfully committed -- meaning that the retry
can only hit this again on a *new* set of transactions.

Essentially, the transaction should only count toward the TPS rate
when it eventually completes without a serialization failure.

Marina, did I understand you correctly?

-- 
Kevin Grittner
VMware vCenter Server
https://www.vmware.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] Detection of IPC::Run presence in SSL TAP tests

[Peter Eisentraut]

On 6/15/17 15:57, Tom Lane wrote:
> Pushed, thanks.  I grabbed the very latest copy of ax_prog_perl_modules
> out of the GNU archives --- it's only cosmetically different, but we
> might as well be au courant.

Um, this patch was previously rejected.  Shouldn't we at least discuss
it, or have it go through the commit fest?  This is not a regression
against 9.6.

-- 
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] Detection of IPC::Run presence in SSL TAP tests

[Tom Lane]

Michael Paquier  writes:
> On Tue, Jun 13, 2017 at 11:14 PM, Tom Lane  wrote:
>> I'd vote for removing this and adding a configure-time check that
>> insists on IPC::Run when --enable-tap-tests is given.

> There was a patch last year to do something like that:
> https://www.postgresql.org/message-id/56bddc20.9020...@postgrespro.ru
> While Test::More is part of any standard installation, IPC::Run is
> not. FWIW, I still think that this is worth checking for. That's way
> better than having the TAP tests explode with a weird fatal failure
> where one needs to dig into the logs just to find out that the module
> is missing.

> Attached is an updated patch, with credit mainly going to Eugene
> Kazakov. I have run autoconf myself, I know that usually committers do
> that... Hope you don't mind.

Pushed, thanks.  I grabbed the very latest copy of ax_prog_perl_modules
out of the GNU archives --- it's only cosmetically different, but we
might as well be au courant.

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

[HACKERS] Приглашение: Re: [HACKERS] intermittent failures in Cygwin from select... - пт, 16 июнь 2017 09:00 - 10:00 (MSK) (pgsql-hackers@postgresql.org)

[Vladimir Sitnikov]

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:REQUEST
BEGIN:VEVENT
DTSTART:20170616T06Z
DTEND:20170616T07Z
DTSTAMP:20170615T193848Z
ORGANIZER;CN=Vladimir Sitnikov:mailto:sitnikov.vladi...@gmail.com
UID:2a6cc082-5202-11e7-978f-bfaa91f79ea0
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=pgsql-hackers@postgresql.org;X-NUM-GUESTS=0:mailto:pgsql-hackers@po
 stgresql.org
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=andrew.duns...@2ndquadrant.com;X-NUM-GUESTS=0:mailto:andrew.dunstan
 @2ndquadrant.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Robert Haas;X-NUM-GUESTS=0:mailto:robertmh...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=Amit Kapila;X-NUM-GUESTS=0:mailto:amit.kapil...@gmail.com
ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
 TRUE;CN=t...@sss.pgh.pa.us;X-NUM-GUESTS=0:mailto:t...@sss.pgh.pa.us
CREATED:20170615T193847Z
DESCRIPTION:Просмотрите свое мероприятие на странице https://www.google.com
 /calendar/event?action=VIEW=XzY5Z2pjb3IzNjBzMzRiOWw2OG8zNGI5aDY1aWplYjl
 wNnNzNmNiYjJjcGdtMmU5aGNvcmppcGIxNjAgcGdzcWwtaGFja2Vyc0Bwb3N0Z3Jlc3FsLm9yZw
 =Mjcjc2l0bmlrb3YudmxhZGltaXJAZ21haWwuY29tYTk1NGFmNjg4YzY4MGU2OWE4Y2VkYj
 IxMDczMzA0ZWE0ZjA4YTNlMA=Europe/Moscow=ru.
LAST-MODIFIED:20170615T193848Z
LOCATION:
SEQUENCE:0
STATUS:CONFIRMED
SUMMARY:Re: [HACKERS] intermittent failures in Cygwin from select_parallel 
 testsэх"
TRANSP:OPAQUE
BEGIN:VALARM
ACTION:DISPLAY
DESCRIPTION:This is an event reminder
TRIGGER:-P0DT0H15M0S
END:VALARM
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

-- 
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 CREATE SUBSCRIPTION documentation

[Julien Rouhaud]

Hi,

I just found $SUBJECT, patch attached.
--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
diff --git a/doc/src/sgml/ref/create_subscription.sgml 
b/doc/src/sgml/ref/create_subscription.sgml
index 77bf87681b..3ca06fb3c3 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -32,7 +32,7 @@ CREATE SUBSCRIPTION subscription_nameDescription
 
   
-   CREATE SUBSCRIPTION adds a new subscription for a
+   CREATE SUBSCRIPTION adds a new subscription for the
current database.  The subscription name must be distinct from the name of
any existing subscription in the database.
   

-- 
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] Adding support for Default partition in partitioning

[Robert Haas]

On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat
 wrote:
> Some more comments on the latest set of patches.
>
> In heap_drop_with_catalog(), we heap_open() the parent table to get the
> default partition OID, if any. If the relcache doesn't have an entry for the
> parent, this means that the entry will be created, only to be invalidated at
> the end of the function. If there is no default partition, this all is
> completely unnecessary. We should avoid heap_open() in this case. This also
> means that get_default_partition_oid() should not rely on the relcache entry,
> but should growl through pg_inherit to find the default partition.

I am *entirely* unconvinced by this line of argument.  I think we want
to open the relation the first time we touch it and pass the Relation
around thereafter.  Anything else is prone to accidentally failing to
have the relation locked early enough, or looking up the OID in the
relcache multiple times.

> In get_qual_for_list(), if the table has only default partition, it won't have
> any boundinfo. In such a case the default partition's constraint would come 
> out
> as (NOT ((a IS NOT NULL) AND (a = ANY (ARRAY[]::integer[]. The empty array
> looks odd and may be we spend a few CPU cycles executing ANY on an empty 
> array.
> We have the same problem with a partition containing only NULL value. So, may
> be this one is not that bad.

I think that one is probably worth fixing.

> Please add a testcase to test addition of default partition as the first
> partition.

That seems like a good idea, too.

> get_qual_for_list() allocates the constant expressions corresponding to the
> datums in CacheMemoryContext while constructing constraints for a default
> partition. We do not do this for other partitions. We may not be constructing
> the constraints for saving in the cache. For example, ATExecAttachPartition
> constructs the constraints for validation. In such a case, this code will
> unnecessarily clobber the cache memory. generate_partition_qual() copies the
> partition constraint in the CacheMemoryContext.
>
> +   if (spec->is_default)
> +   {
> +   result = list_make1(make_ands_explicit(result));
> +   result = list_make1(makeBoolExpr(NOT_EXPR, result, -1));
> +   }

Clearly we do not want things to end up across multiple contexts.  We
should ensure that anything linked from the relcache entry ends up in
CacheMemoryContext, but we must be careful not to allocate anything
else in there, because CacheMemoryContext is never reset.

> If the "result" is an OR expression, calling make_ands_explicit() on it would
> create AND(OR(...)) expression, with an unnecessary AND. We want to avoid 
> that?

I'm not sure it's worth the trouble.

> +defaultPartOid = get_default_partition_oid(rel);
> +if (OidIsValid(defaultPartOid))
> +ereport(ERROR,
> +(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> + errmsg("there exists a default partition for table
> \"%s\", cannot attach a new partition",
> +RelationGetRelationName(rel;
> +
> Should be done before heap_open on the table being attached. If we are not
> going to attach the partition, there's no point in instantiating its relcache.

No, because we should take the lock before examining any properties of
the table.

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

[Andres Freund]

Hi,

On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:
> Now in pgbench we can test only transactions with Read Committed isolation
> level because client sessions are disconnected forever on serialization
> failures. There were some proposals and discussions about it (see message
> here [1] and thread here [2]).

> I suggest a patch where pgbench client sessions are not disconnected because
> of serialization or deadlock failures and these failures are mentioned in
> reports.

I think that's a good idea and sorely needed.


In details:


> - if there were these failures during script execution this "transaction" is
> marked
> appropriately in logs;
> - numbers of "transactions" with these failures are printed in progress, in
> aggregation logs and in the end with other results (all and for each
> script);

I guess that'll include a "rolled-back %' or 'retried %' somewhere?


> Advanced options:
> - mostly for testing built-in scripts: you can set the default transaction
> isolation level by the appropriate benchmarking option (-I);

I'm less convinced of the need of htat, you can already set arbitrary
connection options with
PGOPTIONS='-c default_transaction_isolation=serializable' pgbench


> P.S. Does this use case (do not retry transaction with serialization or
> deadlock failure) is most interesting or failed transactions should be
> retried (and how much times if there seems to be no hope of success...)?

I can't quite parse that sentence, could you restate?

- Andres


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

Re: [HACKERS] pgbench tap tests & minor fixes

[Fabien COELHO]

As for me, I would do expr_scanner_chomp_substring(PsqlScanState, int, int&);
that changes end_offset as desired...


Why not.


And use it instead of end_offset = expr_scanner_offset(sstate) - 1;


I removed these?


The second issue: you are removing all trailing \n and \r. I think you should
remove only one \n at the end of the string, and one \r before \n if there was
one.


So chomp one eol.

Is the attached version better to your test?

--
Fabien.diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index dc1367b..0c802af 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -360,6 +360,23 @@ expr_scanner_get_substring(PsqlScanState state,
 }
 
 /*
+ * get current expression line without one ending newline
+ */
+char *
+expr_scanner_chomp_substring(PsqlScanState state, int start_offset, int end_offset)
+{
+	const char *p = state->scanbuf;
+	/* chomp eol */
+	if (end_offset > start_offset && p[end_offset] == '\n')
+	{
+		end_offset--;
+		if (end_offset > start_offset && p[end_offset] == '\r')
+			end_offset--;
+	}
+	return expr_scanner_get_substring(state, start_offset, end_offset + 1);
+}
+
+/*
  * Get the line number associated with the given string offset
  * (which must not be past the end of where we've lexed to).
  */
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 14aa587..0f4e125 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -229,7 +229,7 @@ typedef struct SimpleStats
 typedef struct StatsData
 {
 	time_t		start_time;		/* interval start time, for aggregates */
-	int64		cnt;			/* number of transactions */
+	int64		cnt;			/* number of transactions, including skipped */
 	int64		skipped;		/* number of transactions skipped under --rate
  * and --latency-limit */
 	SimpleStats latency;
@@ -329,7 +329,7 @@ typedef struct
 	bool		prepared[MAX_SCRIPTS];	/* whether client prepared the script */
 
 	/* per client collected stats */
-	int64		cnt;			/* transaction count */
+	int64		cnt;			/* client transaction count, for -t */
 	int			ecnt;			/* error count */
 } CState;
 
@@ -2045,7 +2045,8 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 	if (INSTR_TIME_IS_ZERO(now))
 		INSTR_TIME_SET_CURRENT(now);
 	now_us = INSTR_TIME_GET_MICROSEC(now);
-	while (thread->throttle_trigger < now_us - latency_limit)
+	while (thread->throttle_trigger < now_us - latency_limit &&
+		   (nxacts <= 0 || st->cnt < nxacts)) /* with -t, do not overshoot */
 	{
 		processXactStats(thread, st, , true, agg);
 		/* next rendez-vous */
@@ -2053,6 +2054,12 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 		thread->throttle_trigger += wait;
 		st->txn_scheduled = thread->throttle_trigger;
 	}
+
+	if (nxacts > 0 && st->cnt >= nxacts)
+	{
+		st->state = CSTATE_FINISHED;
+		break;
+	}
 }
 
 st->state = CSTATE_THROTTLE;
@@ -2364,15 +2371,8 @@ doCustom(TState *thread, CState *st, StatsData *agg)
  */
 			case CSTATE_END_TX:
 
-/*
- * transaction finished: calculate latency and log the
- * transaction
- */
-if (progress || throttle_delay || latency_limit ||
-	per_script_stats || use_log)
-	processXactStats(thread, st, , false, agg);
-else
-	thread->stats.cnt++;
+/* transaction finished: calculate latency and do log */
+processXactStats(thread, st, , false, agg);
 
 if (is_connect)
 {
@@ -2381,7 +2381,6 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 	INSTR_TIME_SET_ZERO(now);
 }
 
-++st->cnt;
 if ((st->cnt >= nxacts && duration <= 0) || timer_exceeded)
 {
 	/* exit success */
@@ -2519,17 +2518,20 @@ processXactStats(TState *thread, CState *st, instr_time *now,
 {
 	double		latency = 0.0,
 lag = 0.0;
+	bool		detailed = progress || throttle_delay || latency_limit ||
+		per_script_stats || use_log;
 
-	if ((!skipped) && INSTR_TIME_IS_ZERO(*now))
-		INSTR_TIME_SET_CURRENT(*now);
-
-	if (!skipped)
+	if (detailed && !skipped)
 	{
+		if (INSTR_TIME_IS_ZERO(*now))
+			INSTR_TIME_SET_CURRENT(*now);
+
 		/* compute latency & lag */
 		latency = INSTR_TIME_GET_MICROSEC(*now) - st->txn_scheduled;
 		lag = INSTR_TIME_GET_MICROSEC(st->txn_begin) - st->txn_scheduled;
 	}
 
+	/* detailed thread stats */
 	if (progress || throttle_delay || latency_limit)
 	{
 		accumStats(>stats, skipped, latency, lag);
@@ -2539,7 +2541,13 @@ processXactStats(TState *thread, CState *st, instr_time *now,
 			thread->latency_late++;
 	}
 	else
+	{
+		/* no detailed stats, just count */
 		thread->stats.cnt++;
+	}
+
+	/* client stat is just counting */
+	st->cnt ++;
 
 	if (use_log)
 		doLog(thread, st, agg, skipped, latency, lag);
@@ -3030,8 +3038,7 @@ process_backslash_command(PsqlScanState sstate, const char *source)
 	PQExpBufferData word_buf;
 	int			word_offset;
 	int			offsets[MAX_ARGS];		/* offsets of argument words */
-	int			start_offset,
-end_offset;
+	int			start_offset;
 	int		

Re: [HACKERS] WIP: Data at rest encryption

[Robert Haas]

On Thu, Jun 15, 2017 at 12:06 PM, Peter Eisentraut
 wrote:
> Making this work well would be a major part of the usability story that
> this is being sold on.  If the proposed solution is that you can cobble
> together a few bits of shell, then not only is that not very
> user-friendly, it also won't work consistently across platforms, won't
> work under systemd (launchd? Windows service?), and might behave
> awkwardly under restricted environments where there is no terminal or
> only a limited OS environment.  Moreover, it leaves the security aspects
> of that part of the solution (keys lingering in memory or in swap) up to
> the user.
>
> There was a discussion a while ago about how to handle passphrase entry
> for SSL keys.  The conclusion was that it works pretty crappily right
> now, and several suggestions for improvement were discussed.  I suggest
> that fixing that properly and with flexibility could also yield a
> solution for encryption key entry.

That sounds like a good idea to me.

However, I'd like to disagree with the idea that key management is the
primary way in which this feature would improve usability.  To me, the
big advantage is that you don't need to be root (also, we can have
more consistency in behavior across operating systems).  I disagree
vigorously with the idea that anyone who wants to encrypt their
PostgreSQL database should just get root privileges on the system and
use an encrypted filesystem.  In some environments, "just get root
privileges" is not something that is very easy to do; but even if you
*have* root privileges, you don't necessarily want to have to use them
just to install and configure your database.

Right now, I can compile and install PostgreSQL from my own user
account, run initdb, and start it up.  Surely everyone PostgreSQL
developer in the world - and a good number of users - would agree that
if I suddenly needed to run initdb as root, that would be a huge
usability regression.  You wouldn't even be able to run 'make check'
without root privileges, which would suck.  In the same way, when we
removed (for most users) the need to tune System V shared memory
parameters (b0fc0df9364d2d2d17c0162cf3b8b59f6cb09f67), a lot of users
were very happy precisely because it eliminated a setup step that
formerly had to be done as root.  It is very reasonable to suppose
that users who need encryption will similarly be happy if they no
longer need to be root to get an encrypted PostgreSQL working.  Sure,
that's only a subset of users rather than all of them, but it's the
same kind of issue.

Also, I don't think we should be presenting filesystem encryption and
built-in encryption as if they were two methods of solving the exact
same problem.  In some scenarios, they are in fact solving the same
problem.  However, it's entirely reasonable to want to use both.  For
example, the hard disk in my laptop is encrypted, because that's a
thing Apple does.  If somebody steals the hard disk out of my laptop,
they may have some difficulty recovering the contents. However, as
Stephen also mentioned, that has not deterred me from putting a
passphrase on my SSH keys.  There are situations in which the
passphrase provides protection that the whole-drive encryption won't.
For example, if I copy my home directory onto a USB stick and then
copy it from there to a new laptop, somebody might steal the USB
stick.  If they manage to do that, they will get most of my files, but
they won't get my SSH keys, or at least not without guessing my
passphrase.  Similarly, if I foolishly walk away from my laptop in the
presence of some nefarious person (say, Magnus) without locking it,
that person can't steal my keys.  That person might be able to
impersonate me for as long as I'm away from the laptop, if the keys
are loaded into my SSH agent, but not afterwards.  The issues for the
database are similar.  You might want one of these things or the other
or both depending on the exact situation.

To be honest, I find the hostility toward this feature a bit baffling.
The argument seems to be essentially that we shouldn't have this
feature because we'd have to maintain the code and many of the same
goals could be accomplished by using facilities that already exist
outside the database server.  But that's also true for parallel query
(cf. Stado), logical replication (cf. Slony, Bucardo, Londiste),
physical replication (cf. DRBD), partitioning (cf. pg_partman), RLS
(cf. veil), and anything that could be written as application logic
(eg. psql's \if ... \endif, every procedural language we have,
user-defined functions themselves, database-enforced constraints,
FDWs).  Yet, in every one of those cases, we find it worthwhile to
have the feature because it works better and is easier to use when
it's built in.  I don't think that a patch for this feature is likely
to be bigger than (or even as large as) the patches for logical
replication or parallel query, and it will probably be 

Re: [HACKERS] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> I think you're right.  So here's a theory:

> 1. The ERROR mapping the DSM segment is just a case of the worker the
> losing a race, and isn't a bug.

I concur that this is a possibility, but if we expect this to happen,
seems like there should be other occurrences in the buildfarm logs.
I trolled the last three months worth of check/installcheck logs (all runs
not just failures), and could find exactly two cases of "could not map
dynamic shared memory segment":

 sysname  |branch |  snapshot   | stage  |  
   l
 
--+---+-++---
 lorikeet | REL9_6_STABLE | 2017-05-03 10:21:31 | Check  | 2017-05-03 
06:27:32.626 EDT [5909b094.1e28:1] ERROR:  could not map dynamic shared memory 
segment
 lorikeet | HEAD  | 2017-06-13 20:28:33 | InstallCheck-C | 2017-06-13 
16:44:57.247 EDT [59404ec9.2e78:1] ERROR:  could not map dynamic shared memory 
segment
(2 rows)

Now maybe this can be explained away by saying that the worker never loses
the race unless it's subject to cygwin's unusually slow fork() emulation,
but somehow I doubt that.  For one thing, it's not clear why that path
would be slower than EXEC_BACKEND, which would also involve populating
a new process image from scratch.

BTW, that 9.6 failure is worth studying because it looks quite a bit
different from the one on HEAD.  It looks like the worker failed to
launch and then the leader got hung up waiting for the worker.
Eventually other stuff started failing because the select_parallel
test is holding an exclusive lock on tenk1 throughout its session.
(Does it really need to do that ALTER TABLE?)

> 2. But when that happens, parallel_terminate_count is getting bumped
> twice for some reason.
> 3. So then the leader process fails that assertion when it tries to
> launch the parallel workers for the next query.

It seems like this has to trace to some sort of logic error in the
postmaster that's allowing it to mess up parallel_terminate_count,
but I'm not managing to construct a plausible flow of control that
would cause 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] Get stuck when dropping a subscription during synchronizing table

[Petr Jelinek]

On 15/06/17 18:36, Peter Eisentraut wrote:
> On 6/15/17 12:22, Petr Jelinek wrote:
>> On 15/06/17 17:53, Peter Eisentraut wrote:
>>> On 6/14/17 18:35, Petr Jelinek wrote:
 Attached fixes it (it was mostly about order of calls).
>>>
>>> So do I understand this right that the actual fix is just moving up the
>>> logicalrep_worker_stop() call in DropSubscription().
>>>
>>
>> No the fix is heap_open before SearchSysCache().
> 
> Right.  Is there a reason for moving the _stop() call then?
> 

Nothing specific, just felt it's better there when I was messing with
the function.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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

[Ashutosh Sharma]

Hi,

On Thu, Jun 15, 2017 at 8:36 PM, Peter Eisentraut
 wrote:
> On 6/12/17 00:38, Ashutosh Sharma wrote:
>> PFA patch that fixes the issue described in above thread. As mentioned
>> in the above thread, the crash is basically happening in varstr_cmp()
>> function and  it's  only happening on Windows because in varstr_cmp(),
>> if the collation provider is ICU, we don't even think of calling ICU
>> functions to compare the string. Infact, we directly attempt to call
>> the OS function wsccoll*() which is not expected. Thanks.
>
> Maybe just
>
> diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
> index a0dd391f09..2506f4eeb8 100644
> --- a/src/backend/utils/adt/varlena.c
> +++ b/src/backend/utils/adt/varlena.c
> @@ -1433,7 +1433,7 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, 
> Oid collid)
>
>  #ifdef WIN32
> /* Win32 does not have UTF-8, so we need to map to UTF-16 */
> -   if (GetDatabaseEncoding() == PG_UTF8)
> +   if (GetDatabaseEncoding() == PG_UTF8 && (!mylocale || 
> mylocale->provider == COLLPROVIDER_LIBC))
> {
> int a1len;
> int a2len;

Oh, yes, this looks like the simplest and possibly the ideal way to
fix the issue. Attached is the patch. Thanks for the inputs.

-- 
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a0dd391..d24d1c5 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1433,7 +1433,8 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid)
 
 #ifdef WIN32
 		/* Win32 does not have UTF-8, so we need to map to UTF-16 */
-		if (GetDatabaseEncoding() == PG_UTF8)
+		if (GetDatabaseEncoding() == PG_UTF8 &&
+			(!mylocale || mylocale->provider == COLLPROVIDER_LIBC))
 		{
 			int			a1len;
 			int			a2len;

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

[Ashutosh Sharma]

Hi,

On Thu, Jun 15, 2017 at 7:43 PM, Amit Kapila  wrote:
> On Mon, Jun 12, 2017 at 10:08 AM, Ashutosh Sharma  
> wrote:
>> PFA patch that fixes the issue described in above thread. As mentioned
>> in the above thread, the crash is basically happening in varstr_cmp()
>> function and  it's  only happening on Windows because in varstr_cmp(),
>> if the collation provider is ICU, we don't even think of calling ICU
>> functions to compare the string. Infact, we directly attempt to call
>> the OS function wsccoll*() which is not expected. Thanks.
>>
>
> Your analysis is right, basically, when ICU is enabled we need to use
> ICU related functions and use corresponding information in the
> pg_locale structure. However, I see few problems with your patch.
>
> + if (mylocale)
> + {
> + if (mylocale->provider == COLLPROVIDER_ICU)
> + {
> +#ifdef USE_ICU
> +#ifdef HAVE_UCOL_STRCOLLUTF8
> + if (GetDatabaseEncoding() == PG_UTF8)
> + {
> + UErrorCode status;
> +
> + status = U_ZERO_ERROR;
> + result = ucol_strcollUTF8(mylocale->info.icu.ucol,
> +  arg1, len1,
> +  arg2, len2,
> +  );
>
> On Windows, we need to map UTF-8 strings to UTF-16 as we are doing in
> this function for other Windows specific comparisons for UTF-8
> strings.  Also, we don't want to screw memcmp optimization we have in
> this function, so do this ICU specific checking after memcmp check.

Firstly, thanks for reviewing the patch. Okay, we do map UTF-8 strings
to UTF-16 on windows. But, I think, we only do that when calling OS
(wcscoll*) functions for string comparison. Please correct me if i am
wrong here. In my case, i am using ICU functions for comparing strings
and i am not sure if we need the convert UTF-8 strings to UTF-16.

-- 
With Regards,
Ashutosh Sharma
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] UPDATE of partition key

[Amit Khandekar]

On 13 June 2017 at 15:40, Amit Khandekar  wrote:
> While rebasing my patch for the below recent commit, I realized that a
> similar issue exists for the uptate-tuple-routing patch as well :
>
> commit 78a030a441966d91bc7e932ef84da39c3ea7d970
> Author: Tom Lane 
> Date:   Mon Jun 12 23:29:44 2017 -0400
>
> Fix confusion about number of subplans in partitioned INSERT setup.
>
> The above issue was about incorrectly using 'i' in
> mtstate->mt_plans[i] during handling WITH CHECK OPTIONS in
> ExecInitModifyTable(), where 'i' was actually meant to refer to the
> positions in mtstate->mt_num_partitions. Actually for INSERT, there is
> only a single plan element in mtstate->mt_plans[] array.
>
> Similarly, for update-tuple routing, we cannot use
> mtstate->mt_plans[i], because 'i' refers to position in
> mtstate->mt_partitions[] , whereas mtstate->mt_plans is not at all in
> order of mtstate->mt_partitions; in fact mt_plans has only the plans
> that are to be scanned on pruned partitions; so it can well be a small
> subset of total partitions.
>
> I am working on an updated patch to fix the above.

Attached patch v10 fixes the above. In the existing code, where it
builds WCO constraints for each leaf partition; with the patch, that
code now is applicable to row-movement-updates as well. So the
assertions in the code are now updated to allow the same. Secondly,
the mapping for each of the leaf partitions was constructed using the
root partition attributes. Now in the patch, the
mtstate->resultRelInfo[0] (i.e. the first resultRelInfo) is used as
reference. So effectively, map_partition_varattnos() now represents
not just parent-to-partition mapping, but rather, mapping between any
two partitions/partitioned_tables. It's done this way, so that we can
have a common WCO building code for inserts as well as updates. For
e.g. for inserts, the first (and only) WCO belongs to
node->nominalRelation so nominalRelation is used for
map_partition_varattnos(), whereas for updates, first WCO belongs to
the first resultRelInfo which is not same as nominalRelation. So in
the patch, in both cases, we use the first resultRelInfo and the WCO
of the first resultRelInfo for map_partition_varattnos().

Similar thing is done for Returning expressions.

-

Another change in the patch is : for ExecInitQual() for WCO quals,
mtstate->ps is used as parent, rather than first plan. For updates,
first plan does not belong to the parent partition. In fact, I think
in all cases, we should use mtstate->ps as the parent.
mtstate->mt_plans[0] don't look like they should be considered parent
of these expressions. May be it does not matter to which parent we
link these quals, because there is no ReScan for ExecModifyTable().

Note that for RETURNING projection expressions, we do use mtstate->ps.



There is another issue I discovered. The row-movement works fine if
the destination leaf partition has different attribute ordering than
the root : the existing insert-tuple-routing mapping handles that. But
if the source partition has different ordering w.r.t. the root, it has
a problem : there is no mapping in the opposite direction, i.e. from
the leaf to root. And we require that because the tuple of source leaf
partition needs to be converted to root partition tuple descriptor,
since ExecFindPartition() starts with root.

To fix this, I have introduced another mapping array
mtstate->mt_resultrel_maps[]. This corresponds to the
mtstate->resultRelInfo[]. We don't require per-leaf-partition mapping,
because the update result relations are pruned subset of the total
leaf partitions.

So in ExecInsert, before calling ExecFindPartition(), we need to
convert the leaf partition tuple to root using this reverse mapping.
Since we need to convert the tuple here, and again after
ExecFindPartition() for the found leaf partition, I have replaced the
common code by new function ConvertPartitionTupleSlot().

---

Used a new flag is_partitionkey_update in ExecInitModifyTable(), which
can be re-used in subsequent sections , rather than again calling
IsPartitionKeyUpdate() function again.

---

Some more test scenarios added that cover above changes. Basically
partitions that have different tuple descriptors than parents.


update-partition-key_v10.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] Broken hint bits (freeze)

[Bruce Momjian]

On Thu, Jun 15, 2017 at 03:00:18PM +0530, Amit Kapila wrote:
> On Wed, Jun 14, 2017 at 8:44 PM, Bruce Momjian  wrote:
> > On Wed, Jun 14, 2017 at 07:45:17PM +0530, Amit Kapila wrote:
> >> > Now, it seems we later added a doc section early on that talks about
> >> > "Verify standby servers" so I have moved the wal_level section into that
> >> > block, which should be safe.  There is now no need to start/stop the new
> >> > server since pg_upgrade will do that safely already.
> >> >
> >>
> >> ! 
> >> !  Also, if upgrading standby servers, change wal_level
> >> !  to replica in the postgresql.conf file on
> >> !  the new cluster.
> >>
> >> I think it is better to indicate that this is required for the master
> >> cluster (probably it is clear for users) /"on the new cluster."/"on
> >> the new master cluster.". Do we need something different for v10 where
> >> default wal_level is 'replica'
> >
> > You know, I thought about that and was afraid saying "new master
> > cluster" would be confusing because it isn't a master _yet_, but if you
> > feel it will help, and I considered it, let's add it.  The problem is
> > that in the old instructions, at the point we were mentioning this, it
> > was the new master, which is why I evaluated removing it in the first
> > place. (Yeah, I am amazed I considered all these cases.)
> >
> > Updated patch attached.  Thanks.
> >
> 
> Looks good to me.

Patch applied back to 9.5, where these instructions first appeared.  A
mention of this will appear in the minor release notes.  Thanks for
everyone's work on this.

-- 
  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] Shortened URLs for commit messages

[Bruce Momjian]

On Tue, May 23, 2017 at 11:25:07PM -0400, Bruce Momjian wrote:
> I have written the following sed script to convert regular Postgres
> email message URLs to their shorter form for commit messages:
> 
>  sed 
> 's;http\(s\?\)://www\.postgresql\.org/message-id/;http\1://postgr.es/m/;gi'
> 
> in case this is helpful to anyone.

Oh, here's another one.  I use an optional "Discussion:" tag in my
commit messages. This sed script converts a message-id into a proper
URL:

sed '/http/!s;^\(Discussion: *\)\(.*\)$;\1https://postgr.es/m/\2;'

For example:

-Discussion: 87wp8o506b.fsf@seb.koffice.internal
+Discussion: https://postgr.es/m/87wp8o506b.fsf@seb.koffice.internal

Yeah!

-- 
  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] Adding support for Default partition in partitioning

[Ashutosh Bapat]

Some more comments on the latest set of patches.

In heap_drop_with_catalog(), we heap_open() the parent table to get the
default partition OID, if any. If the relcache doesn't have an entry for the
parent, this means that the entry will be created, only to be invalidated at
the end of the function. If there is no default partition, this all is
completely unnecessary. We should avoid heap_open() in this case. This also
means that get_default_partition_oid() should not rely on the relcache entry,
but should growl through pg_inherit to find the default partition.

In get_qual_for_list(), if the table has only default partition, it won't have
any boundinfo. In such a case the default partition's constraint would come out
as (NOT ((a IS NOT NULL) AND (a = ANY (ARRAY[]::integer[]. The empty array
looks odd and may be we spend a few CPU cycles executing ANY on an empty array.
We have the same problem with a partition containing only NULL value. So, may
be this one is not that bad.

Please add a testcase to test addition of default partition as the first
partition.

get_qual_for_list() allocates the constant expressions corresponding to the
datums in CacheMemoryContext while constructing constraints for a default
partition. We do not do this for other partitions. We may not be constructing
the constraints for saving in the cache. For example, ATExecAttachPartition
constructs the constraints for validation. In such a case, this code will
unnecessarily clobber the cache memory. generate_partition_qual() copies the
partition constraint in the CacheMemoryContext.

+   if (spec->is_default)
+   {
+   result = list_make1(make_ands_explicit(result));
+   result = list_make1(makeBoolExpr(NOT_EXPR, result, -1));
+   }

If the "result" is an OR expression, calling make_ands_explicit() on it would
create AND(OR(...)) expression, with an unnecessary AND. We want to avoid that?

+   if (cur_index < 0 && (partition_bound_has_default(partdesc->boundinfo)))
+   cur_index = partdesc->boundinfo->default_index;
+
The partition_bound_has_default() check is unnecessary since we check for
cur_index < 0 next anyway.

+ *
+ * Given the parent relation checks if it has default partition, and if it
+ * does exist returns its oid, otherwise returns InvalidOid.
+ */
May be reworded as "If the given relation has a default partition, this
function returns the OID of the default partition. Otherwise it returns
InvalidOid."

+Oid
+get_default_partition_oid(Relation parent)
+{
+   PartitionDesc partdesc = RelationGetPartitionDesc(parent);
+
+   if (partdesc->boundinfo && partition_bound_has_default(partdesc->boundinfo))
+   return partdesc->oids[partdesc->boundinfo->default_index];
+
+   return InvalidOid;
+}
An unpartitioned table would not have partdesc set set. So, this function will
segfault if we pass an unpartitioned table. Either Assert that partdesc should
exist or check for its NULL-ness.


+defaultPartOid = get_default_partition_oid(rel);
+if (OidIsValid(defaultPartOid))
+ereport(ERROR,
+(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("there exists a default partition for table
\"%s\", cannot attach a new partition",
+RelationGetRelationName(rel;
+
Should be done before heap_open on the table being attached. If we are not
going to attach the partition, there's no point in instantiating its relcache.

The comment in heap_drop_with_catalog() should mention why we lock the default
partition before locking the table being dropped.

 extern List *preprune_pg_partitions(PlannerInfo *root, RangeTblEntry *rte,
Index rti, Node *quals, LOCKMODE lockmode);
-
 #endif   /* PARTITION_H */
Unnecessary hunk.

On Thu, Jun 15, 2017 at 12:31 PM, Amit Langote
 wrote:
> Oops, I meant to send one more comment.
>
> On 2017/06/15 15:48, Amit Langote wrote:
>> BTW, I noticed the following in 0002
> +errmsg("there exists a default 
> partition for table \"%s\", cannot
> add a new partition",
>
> This error message style seems novel to me.  I'm not sure about the best
> message text here, but maybe: "cannot add new partition to table \"%s\"
> with default partition"
>
> Note that the comment applies to both DefineRelation and
> ATExecAttachPartition.
>
> Thanks,
> Amit
>



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] Get stuck when dropping a subscription during synchronizing table

[Peter Eisentraut]

On 6/15/17 12:22, Petr Jelinek wrote:
> On 15/06/17 17:53, Peter Eisentraut wrote:
>> On 6/14/17 18:35, Petr Jelinek wrote:
>>> Attached fixes it (it was mostly about order of calls).
>>
>> So do I understand this right that the actual fix is just moving up the
>> logicalrep_worker_stop() call in DropSubscription().
>>
> 
> No the fix is heap_open before SearchSysCache().

Right.  Is there a reason for moving the _stop() call then?

-- 
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] Something is rotten in publication drop

[Tom Lane]

Peter Eisentraut  writes:
> On 6/9/17 11:45, Tom Lane wrote:
>> What we've done in many comparable situations is to allow a
>> catalog-probing function to return NULL instead of failing
>> when handed an OID or other identifier that it can't locate.
>> Here it seems like pg_get_publication_tables() needs to use
>> missing_ok = TRUE and then return zero rows for a null result.

> Why is it that dropping a publication in another session makes our local
> view of things change in middle of a single SQL statement?  Is there
> something we can change to address that?

Not easily.  The syscaches run on CatalogSnapshot time and may therefore
see changes not yet visible to the surrounding query's snapshot.  It's
possible that you could reimplement pg_get_publication_tables() to do
its own catalog access with the outer query's snapshot, but it'd be
pretty tedious and duplicative.

It strikes me that you could rewrite psql's query to just do its own
catalog search and not bother with the function at all.  It would have
to know a bit more about the catalog structure than it does now, but not
that much:

select pub.pubname from pg_catalog.pg_publication pub
where puballtables or
  exists(select 1 from pg_catalog.pg_publication_rel r
 where r.prpubid = pub.oid and r.prrelid = '%s');

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] Making server name part of the startup message

[Magnus Hagander]

On Thu, Jun 15, 2017 at 5:57 PM, Tom Lane  wrote:

> Alvaro Herrera  writes:
> > Tom Lane wrote:
> >> This makes no sense at all.  The client is telling the server what the
> >> server's name is?
>
> > I think for instance you could have one pgbouncer instance (or whatever
> > pooler) pointing to several different servers.  So the client connects
> > to the pooler and indicates which of the servers to connect to.
>
> I should think that in such cases, the end client is exactly not what
> you want to be choosing which server it gets redirected to.  You'd
> be wanting to base that on policies defined at the pooler.  There are
> already plenty of client-supplied attributes you could use as inputs
> for such policies (user name and application name, for instance).
> Why do we need to incur a protocol break to add another one?
>

The normal one to use for pgbonucer today is, well, "database name". You
can then have pgbouncer map different databases to different backend
servers. It's fairly common in my experience to have things like "dbname"
and "dbname-ro" (for example) as different database names with one mapping
to the master and one mapping to a load-balanced set of standbys, and
things like that. ISTM that using the database name is a good choice for
that.

For the original idea in this thread, using something like dbname@server
seems a more logical choice than username@server.

TBH, so maybe I'm misunderstanding the original issue?

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

Re: [HACKERS] Get stuck when dropping a subscription during synchronizing table

[Petr Jelinek]

On 15/06/17 17:53, Peter Eisentraut wrote:
> On 6/14/17 18:35, Petr Jelinek wrote:
>> Attached fixes it (it was mostly about order of calls).
> 
> So do I understand this right that the actual fix is just moving up the
> logicalrep_worker_stop() call in DropSubscription().
>

No the fix is heap_open before SearchSysCache().

>> I also split the
>> SetSubscriptionRelState into 2 separate interface while I was changing
>> it, because now that the update_only bool was added it has become quite
>> strange to have single interface for what is basically two separate
>> functions.
> 
> makes sense
> 
>> Other related problem is locking of subscriptions during operations on
>> them, especially AlterSubscription seems like it should lock the
>> subscription itself. I did that in 0002.
> 
> More detail here please.  AlterSubscription() does locking via
> heap_open().  This introduces a new locking method.  What are the
> implications?
> 

I don't think heap_open will be enough once we remove the
AccessExclusiveLock of the catalog in DropSubscription because
concurrent AlterSubscription might happily add tables to the
subscription that has been dropped if we don't lock it. But you made me
realize that even my patch is not enough because we then reread the
subscription info only from syscache without any kind of invalidation
attempt.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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] ASOF join

[Konstantin Knizhnik]

I wonder if there were some discussion/attempts to add ASOF join to 
Postgres  (sorry, may be there is better term for it, I am refereeing 
KDB definition: http://code.kx.com/wiki/Reference/aj ).
Such kind of join can be useful when we need to associate two 
timeseries. It is quite popular in trading:


//join the eq price and size for the op trade time
a::aj[`underlyingSym`time;select time, underlyingSym, sym, putorcall, 
ex, cond, price, seqnum, contracts, contractsTraded from t;eqtrades];


...and not only. Below is one example of how people now manually coding 
ASOF join:


select
count(*),
count(*)
filter (where timedelta_prev < -30),
count(*)
filter (where ride_prev = ride_next),
... -- many different aggregates
from
(
select
p.provider_id,
p.ts,
(
select extract(epoch from t.ts - p.ts)
from providers_positions t
where p.provider_id = t.provider_id and t.ts < p.ts and 
t.source = 'gps'

order by t.ts desc
limit 1
) as timedelta_prev,
(
select extract(epoch from t.ts - p.ts)
from providers_positions t
where p.provider_id = t.provider_id and t.ts > p.ts and 
t.source = 'gps'

order by t.ts
limit 1
) as timedelta,
(
select ride_id
from providers_positions t
where p.provider_id = t.provider_id and t.ts < p.ts and 
t.source = 'gps'

order by t.ts desc
limit 1
) as ride_prev,
(
select ride_id
from providers_positions t
where p.provider_id = t.provider_id and t.ts > p.ts and 
t.source = 'gps'

order by t.ts
limit 1
) as ride_next
from (
 select
 provider_id,
 ts,
 event_name
 from
 lvl2_681_parsed p
 ) p
where
p.event_name = 'GPS signal restored'
   -- offset 0
) z;

Without OFFSET 0 this query generates awful execution plans with 
hundreds (!) of subplans  corresponding to the subqueries.
Number of subplans (most of them are the same) is equal number of 
occurrences of timedelta, timedelta_prev, ... columns in target aggregates.
OFFSET 0 reduce number of subplans to 4. And I expect that using LATERAL 
join can reduce it to two and even without "OFFSET 0" trick.
But in any case - it is very complex and unnatural way of expressing 
this not so complex query.

With ASOF join is can be written much simpler.

Also Postgres is implementing this query using nested loop with index 
scan, which is definitely not the most efficient strategy.
The best way to implement ASOF join is to use something like mergejoin. 
Usually there are indexes for both timeseries, so what we need is to 
merge two ordered sets using ASOF join rules.
It will require minimal changes in SQL syntax, just adding ASOF keyword 
seems to be enough:


   select  * from Trades ASOF JOIN EqTrades USING (underlyingSym,time);

It seems to me that adding ASOF joins should not require huge amount of 
work and can be done with minimal number of changes in executor and 
optimizer.

But may be there are some problems/challenges which I do not realize now?

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[HACKERS] GiST API Adancement

[Yuan Dong]

Hi hackers,

I want to hack a little. I applied for GSoC project 
(https://wiki.postgresql.org/wiki/GSoC_2017#GiST_API_advancement), but now I'm 
going to hack on my own. With the help of Andrew Borodin, I want to start the 
project with adding a third state to collision check. The third state is that:
 subtree is totally within the query. In this case, GiST scan can scan down 
subtree without key checks.

After reading some code, I get this plan to modify the code:

1. Modify the consistent function of datatypes supported by GiST.

1.1 Start with cube, g_cube_consistent should return a third state when calling 
g_cube_internal_consistent. Inspired by Andrew Borodin, I have two solutions, 
1)modify return value, 2) modify a reference type parameter.

2. Modify the gistindex_keytest in gistget.c to return multiple states

2.1 Need declare a new enum type(or define values) in gist_private.h or 
somewhere

3. Modify the gitsScanPage in gistget.c to deal with the extra state

4. Add a state to mark the nodes under this GISTSearchItem are all with in the 
query

4.1 Two ways to do this: 1) add a state to GISTSearchItem (prefered) 2) 
Somewhere else to record all this kind of items

4.2 We may use the block number as the key

4.3 Next time when the gistScanPage met this item, just find all the leaves 
directly(need a new function)

After this, I shall start to benchmark the improvement and edit the code of 
other datatypes.

Hope you hackers can give me some suggestions~

--
Dong

Re: [HACKERS] Something is rotten in publication drop

[Peter Eisentraut]

On 6/9/17 11:45, Tom Lane wrote:
> What we've done in many comparable situations is to allow a
> catalog-probing function to return NULL instead of failing
> when handed an OID or other identifier that it can't locate.
> Here it seems like pg_get_publication_tables() needs to use
> missing_ok = TRUE and then return zero rows for a null result.

Why is it that dropping a publication in another session makes our local
view of things change in middle of a single SQL statement?  Is there
something we can change to address that?

> BTW, isn't the above command a hugely inefficient way of finding
> the publications for the target rel?  Unless you've got a rather
> small number of rather restricted publications, seems like it's
> going to take a long time.  Maybe we don't care too much about
> manual invocations of \d+, but I bet somebody will carp if there's
> not a better way to find this out.  Maybe a better answer is to
> define a more suitable function pg_publications_for_table(relid)
> and let it have the no-error-for-bad-OID behavior.

That would possibly be better (the current function was written for a
different use case), but it could have the same concurrency problem as
above.

-- 
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] WIP: Data at rest encryption

[Peter Eisentraut]

On 6/14/17 17:41, Stephen Frost wrote:
>> Relying on environment variables is clearly pretty crappy.  So if that's
>> the proposal, then I think it needs to be better.
> I don't believe that was ever intended to be the final solution, I was
> just pointing out that it's what the WIP patch did.
> 
> The discussion had moved into having a command called which provided the
> key on stdout, as I recall, allowing it to be whatever the user wished,
> including binary of any kind.
> 
> If you have other suggestions, I'm sure they would be well received.  As
> to the question of complexity, it certainly looks like it'll probably be
> quite straight-forward for users to use.

I think the passphrase entry part of the problem is actually a bit
harder than it appears.

Making this work well would be a major part of the usability story that
this is being sold on.  If the proposed solution is that you can cobble
together a few bits of shell, then not only is that not very
user-friendly, it also won't work consistently across platforms, won't
work under systemd (launchd? Windows service?), and might behave
awkwardly under restricted environments where there is no terminal or
only a limited OS environment.  Moreover, it leaves the security aspects
of that part of the solution (keys lingering in memory or in swap) up to
the user.

There was a discussion a while ago about how to handle passphrase entry
for SSL keys.  The conclusion was that it works pretty crappily right
now, and several suggestions for improvement were discussed.  I suggest
that fixing that properly and with flexibility could also yield a
solution for encryption key entry.

-- 
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] Huge pages support on Windows

[Andrea caldarone]

Hi all,

First of all bear with me since I'm new in PostgreSQL world, I'm SQL Server
DBA with 20yrs experience.
I've seen the thread started by Tsunakawa Takayuki about huge pages support
on windows and his version of pg_ctl.c in commit fest.
I see no information anywhere about how to recompile only the pg_ctl.exe in
Windows or how to apply a .patch file in Windows.
I've used Mingw64 to build the whole PostgreSQL from source code and I
succeded, then I tried to the pg_ctl.c in source files with the pg_ctl.exe
modified by Tsunakawa Takayuki, I've taken only the pg_ctl.exe generated
and put in my Windows machine, it works but when I try to enable huge pages
in the postresql.conf, the service fails to start with the message "CEST
FATAL:  huge pages not supported on this platform"

Anyone can point me in the right direction?

Re: [HACKERS] Making server name part of the startup message

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane wrote:
>> This makes no sense at all.  The client is telling the server what the
>> server's name is?

> I think for instance you could have one pgbouncer instance (or whatever
> pooler) pointing to several different servers.  So the client connects
> to the pooler and indicates which of the servers to connect to.

I should think that in such cases, the end client is exactly not what
you want to be choosing which server it gets redirected to.  You'd
be wanting to base that on policies defined at the pooler.  There are
already plenty of client-supplied attributes you could use as inputs
for such policies (user name and application name, for instance).
Why do we need to incur a protocol break to add another one?

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] Get stuck when dropping a subscription during synchronizing table

[Peter Eisentraut]

On 6/14/17 18:35, Petr Jelinek wrote:
> Attached fixes it (it was mostly about order of calls).

So do I understand this right that the actual fix is just moving up the
logicalrep_worker_stop() call in DropSubscription().

> I also split the
> SetSubscriptionRelState into 2 separate interface while I was changing
> it, because now that the update_only bool was added it has become quite
> strange to have single interface for what is basically two separate
> functions.

makes sense

> Other related problem is locking of subscriptions during operations on
> them, especially AlterSubscription seems like it should lock the
> subscription itself. I did that in 0002.

More detail here please.  AlterSubscription() does locking via
heap_open().  This introduces a new locking method.  What are the
implications?

-- 
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] WIP: Data at rest encryption

[Robert Haas]

On Wed, Jun 14, 2017 at 5:41 PM, Stephen Frost  wrote:
> I don't believe that was ever intended to be the final solution, I was
> just pointing out that it's what the WIP patch did.
>
> The discussion had moved into having a command called which provided the
> key on stdout, as I recall, allowing it to be whatever the user wished,
> including binary of any kind.
>
> If you have other suggestions, I'm sure they would be well received.  As
> to the question of complexity, it certainly looks like it'll probably be
> quite straight-forward for users to use.

To me, this reads a bit like you're still trying to shut down the
discussion here.  Perhaps I am misreading it.

Upthread, you basically said that we shouldn't talk about key
management (specifically, you said, "Key management is an entirely
independent discussion from this") which I think is a ridiculous
statement.  We have to have some kind of simple key management in
order to have the feature at all.  It does not have to be crazy
complicated, but it has to exist.

-- 
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] Misnaming of staext_dependencies_load

[Alvaro Herrera]

Kyotaro HORIGUCHI wrote:
> Hello.
> 
> It is annoying that only staext_dependencies_load is prefixed
> with "staext" (two t's) among several similar names prefixed by
> "statext"(three t's).
> 
> Should we rename it to have the same prefix?

Sure.  Pushed, thanks.

-- 
Álvaro Herrerahttps://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] Making server name part of the startup message

[Alvaro Herrera]

Tom Lane wrote:
> Satyanarayana Narlapuram  writes:

> > Change the Postgres wire protocol to include server name in the startup 
> > message. This field can be an optional field driven by the connection 
> > parameters for psql (-N, --servername).
> > We need this extra parameter for backward compatibility.
> > Make PostgreSQL server aware of the new field, and accept the startup 
> > message containing this field. Though server doesn't need this field, this 
> > change helps making the server name by default included in the startup 
> > message in future.
> 
> This makes no sense at all.  The client is telling the server what the
> server's name is?

I think for instance you could have one pgbouncer instance (or whatever
pooler) pointing to several different servers.  So the client connects
to the pooler and indicates which of the servers to connect to.

-- 
Álvaro Herrerahttps://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] logical replication read-only slave

[Peter Eisentraut]

On 6/15/17 11:12, Maeldron T. wrote:
> However, it provides me a safety net that I could not execute writes on
> the slave by accident. Not only I couldn’t do it, I would also receive a
> notification from the software about the attempt as it would throw an
> exception.
> 
> Let’s say I would switch to logical replication of all tables
> Safety net is gone

You can change the permissions on your slave so that you cannot write to
the tables.

-- 
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] logical replication read-only slave

[Maeldron T.]

Hello,

I played around a bit with the logical replication in 10.0 beta 1.

My first question was: is it possible to set the "slave" server to run in
(almost) read-only mode?

The current setup is the following:

There is a Rails application running on multiple servers
Two PostgreSQL servers, stream replication
Writes are executed on the master
Some of the reads are executed on the slave
(Nothing new here)

However, it provides me a safety net that I could not execute writes on the
slave by accident. Not only I couldn’t do it, I would also receive a
notification from the software about the attempt as it would throw an
exception.


Let’s say I would switch to logical replication of all tables
Safety net is gone

I could send an explicit command for each session to make it read-only
I could use a read-only role (let’s ignore now I don’t use rules)

But the main attribute of a safety net is the safety. As soon as there
would be a bug, and a session would not send the "set session ..." command,
or the wrong role would be used, the application could write to the
"slave", and that’s not great.

As far as I see, the only solution which provides the same safety level as
the stream replication does would be starting up the "slave" in read-only
mode. In this case, writes would be needed for:

* The replication
* DDL

The DDL could be applied in a specific session as whitelisting is safer
than blacklisting. I think the only missing part is if the subscription
could turn on the writes for itself.

If you think this would make sense, please consider it.

M

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

[Peter Eisentraut]

On 6/12/17 00:38, Ashutosh Sharma wrote:
> PFA patch that fixes the issue described in above thread. As mentioned
> in the above thread, the crash is basically happening in varstr_cmp()
> function and  it's  only happening on Windows because in varstr_cmp(),
> if the collation provider is ICU, we don't even think of calling ICU
> functions to compare the string. Infact, we directly attempt to call
> the OS function wsccoll*() which is not expected. Thanks.

Maybe just

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a0dd391f09..2506f4eeb8 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1433,7 +1433,7 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, 
Oid collid)
 
 #ifdef WIN32
/* Win32 does not have UTF-8, so we need to map to UTF-16 */
-   if (GetDatabaseEncoding() == PG_UTF8)
+   if (GetDatabaseEncoding() == PG_UTF8 && (!mylocale || 
mylocale->provider == COLLPROVIDER_LIBC))
{
int a1len;
int a2len;


-- 
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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 10:21 AM, Amit Kapila  wrote:
> Yes, I think it is for next query.  If you refer the log below from lorikeet:
>
> 2017-06-13 16:44:57.179 EDT [59404ec6.2758:63] LOG:  statement:
> EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM
> tenk1;
> 2017-06-13 16:44:57.247 EDT [59404ec9.2e78:1] ERROR:  could not map
> dynamic shared memory segment
> 2017-06-13 16:44:57.248 EDT [59404dec.2d9c:5] LOG:  worker process:
> parallel worker for PID 10072 (PID 11896) exited with exit code 1
> 2017-06-13 16:44:57.273 EDT [59404ec6.2758:64] LOG:  statement: select
> stringu1::int2 from tenk1 where unique1 = 1;
> TRAP: FailedAssertion("!(BackgroundWorkerData->parallel_register_count
> - BackgroundWorkerData->parallel_terminate_count <= 1024)", File:
> "/home/andrew/bf64/root/HEAD/pgsql.build/../pgsql/src/backend/postmaster/bgworker.c",
> Line: 974)
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:6] LOG:  server process
> (PID 10072) was terminated by signal 6: Aborted
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:7] DETAIL:  Failed process
> was running: select stringu1::int2 from tenk1 where unique1 = 1;
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:8] LOG:  terminating any
> other active server processes
>
> Error "could not map dynamic shared memory segment" is due to query
> "EXPLAIN .. SELECT * FROM tenk1" and Assertion failure is due to
> another statement "select stringu1::int2 from tenk1 where unique1 =
> 1;".

I think you're right.  So here's a theory:

1. The ERROR mapping the DSM segment is just a case of the worker the
losing a race, and isn't a bug.

2. But when that happens, parallel_terminate_count is getting bumped
twice for some reason.

3. So then the leader process fails that assertion when it tries to
launch the parallel workers for the next query.

-- 
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] memory fields from getrusage()

[Tom Lane]

Justin Pryzby  writes:
> Comments ?

I was wondering whether it'd be better to drive this off of configure
probes for the existence of the struct fields.  However, in view of
the same fields having different contents on some platforms, such
a probe wouldn't really help much --- you'd still need platform-aware
logic.

Please add to next commitfest so we remember about this when the time
is ripe.

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] Bug in ExecModifyTable function and trigger issues for foreign tables

[Ildus Kurbangaliev]

On Mon, 5 Jun 2017 17:25:27 +0900
Etsuro Fujita  wrote:

> On 2017/06/02 18:10, Etsuro Fujita wrote:
> > On 2017/05/16 21:36, Etsuro Fujita wrote:  
> >> One approach I came up with to fix this issue is to rewrite the 
> >> targetList entries of an inherited UPDATE/DELETE properly using 
> >> rewriteTargetListUD, when generating a plan for each child table
> >> in inheritance_planner.  Attached is a WIP patch for that.  Maybe
> >> I am missing something, though.  
> > 
> > While updating the patch, I noticed the patch rewrites the UPDATE 
> > targetList incorrectly in some cases; rewrite_inherited_tlist I
> > added to adjust_appendrel_attrs (1) removes all junk items from the
> > targetList and (2) adds junk items for the child table using
> > rewriteTargetListUD, but it's wrong to drop all junk items in cases
> > where there are junk items for some other reasons than
> > rewriteTargetListUD.  Consider junk items containing MULTIEXPR
> > SubLink.  One way I came up with to fix this is to change (1) to
> > only remove junk items with resname; since junk items added by
> > rewriteTargetListUD should have resname (note: we would need
> > resname to call ExecFindJunkAttributeInTlist at execution time!)
> > while other junk items wouldn't have resname (see
> > transformUpdateTargetList), we could correctly replace junk items
> > added by rewriteTargetListUD for the parent with ones for the
> > child, by that change.  I might be missing something, though.
> > Comments welcome.  
> 
> I updated the patch that way.  Please find attached an updated
> version.
> 
> Other changes:
> * Moved the initialization for "tupleid" I added in ExecModifyTable
> as discussed before, which I think is essentially the same as
> proposed by Ildus in [1], since I think that would be more consistent
> with "oldtuple".
> * Added regression tests.
> 
> Anyway I'll add this to the next commitfest.
> 
> Best regards,
> Etsuro Fujita
> 
> [1] 
> https://www.postgresql.org/message-id/20170514150525.0346ba72%40postgrespro.ru

Checked the latest patch. Looks good.
Shouldn't this patch be backported to 9.6 and 10beta? The bug
affects them too.

-- 
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres 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] memory fields from getrusage()

[Justin Pryzby]

On Thu, Jun 15, 2017 at 10:29:21AM -0400, Robert Haas wrote:
> On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby  wrote:
> > On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote:
> >> It might be worth adding platform-specific code for common platforms.
> >
> > All I care (which linux happily/happens to support) is maxrss; I was 
> > probably
> > originally interested in this while digging into an issue with hash agg.
> 
> I don't think it needs to go in a separate file.  I'd just patch ShowUsage().

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f99dd0a..7f57a84 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4467,6 +4467,21 @@ ShowUsage(const char *title)
 r.ru_nvcsw - Save_r.ru_nvcsw,
 r.ru_nivcsw - Save_r.ru_nivcsw,
 r.ru_nvcsw, r.ru_nivcsw);
+
+#if defined(__linux__)
+   appendStringInfo(,
+"!\t%ld max resident (kB)\n",
+r.ru_maxrss);
+#elif defined(BSD)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss, r.ru_ixrss, r.ru_idrss, 
r.ru_isrss);
+#elif defined(__darwin__)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss/1024, r.ru_ixrss/1024, 
r.ru_idrss/1024, r.ru_isrss/1024);
+#endif /* __linux__ */
+
 #endif   /* HAVE_GETRUSAGE */
 
/* remove trailing newline */

Comments ?

Testing or suggestions on !linux would be useful.

Justin
Add memory fields (sometimes) available from getrusage()

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f99dd0a..7f57a84 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4467,6 +4467,21 @@ ShowUsage(const char *title)
 r.ru_nvcsw - Save_r.ru_nvcsw,
 r.ru_nivcsw - Save_r.ru_nivcsw,
 r.ru_nvcsw, r.ru_nivcsw);
+
+#if defined(__linux__)
+   appendStringInfo(,
+"!\t%ld max resident (kB)\n",
+r.ru_maxrss);
+#elif defined(BSD)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss, r.ru_ixrss, r.ru_idrss, 
r.ru_isrss);
+#elif defined(__darwin__)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss/1024, r.ru_ixrss/1024, 
r.ru_idrss/1024, r.ru_isrss/1024);
+#endif /* __linux__ */
+
 #endif   /* HAVE_GETRUSAGE */
 
/* remove trailing newline */

-- 
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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> On Thu, Jun 15, 2017 at 10:38 AM, Tom Lane  wrote:
>> ... er, -ENOCAFFEINE.  Nonetheless, there are no checks of
>> EXEC_FLAG_EXPLAIN_ONLY in any parallel-query code, so I think
>> a bet is being missed somewhere.

> ExecGather() is where workers get launched, and that ain't happening
> if EXEC_FLAG_EXPLAIN_ONLY is set, unless I am *very* confused about
> how this works.

Ah, you're right, it's done during first execution not during InitPlan.
Nevermind ...

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] Refreshing subscription relation state inside a transaction block

[Petr Jelinek]

On 15/06/17 15:52, Peter Eisentraut wrote:
> On 6/15/17 02:41, Petr Jelinek wrote:
>> Hmm, forcibly stopping currently running table sync is not what was
>> intended, I'll have to look into it. We should not be forcibly stopping
>> anything except the main apply worker during drop subscription (and we
>> do that only because we can't drop the remote replication slot otherwise).
> 
> The change being complained about was specifically to address the
> problem described in the commit message:
> 
> Stop table sync workers when subscription relation entry is removed
> 
> When a table sync worker is in waiting state and the subscription table
> entry is removed because of a concurrent subscription refresh, the
> worker could be left orphaned.  To avoid that, explicitly stop the
> worker when the pg_subscription_rel entry is removed.
> 
> 
> Maybe that wasn't the best solution.  Alternatively, the tablesync
> worker has to check itself whether the subscription relation entry has
> disappeared, or we need a post-commit check to remove orphaned workers.
> 

Ah I missed that commit/discussion, otherwise I would have probably
complained. I don't like killing workers in the DDL command, as I said
the only reason we do it in DropSubscription is to free the slot for
dropping.

I think that either of the options you suggested now would be better.
We'll need that for stopping the tablesync which is in progress during
DropSubscription as well as those will currently still keep running. I
guess we could simply just register syscache callback, the only problem
with that is we'd need to AcceptInvalidationMessages regularly while we
do the COPY which is not exactly free so maybe killing at the end of
transaction would be better (both for refresh and drop)?

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, 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] logical replication: \dRp+ and "for all tables"

[Peter Eisentraut]

On 6/10/17 02:02, Jeff Janes wrote:
> On Fri, Jun 9, 2017 at 10:20 PM, Masahiko Sawada  > wrote:
> 
> On Sat, Jun 10, 2017 at 7:29 AM, Jeff Janes  > wrote:
> > If I create a publication "for all tables", \dRp+ doesn't indicate it 
> is for
> > all tables, it just gives a list of the tables.
> >
> > So it doesn't distinguish between a publication specified to be for all
> > tables (which will be dynamic regarding future additions), and one which
> > just happens to include all the table which currently exist.
> >
> > That seems unfortunate.  Should the "for all tables" be included as 
> another
> > column in \dRp and \dRp+, or at least as a footnote tag in \dRp+ ?
> >
> 
> +1. I was thinking the same. Attached patch adds "All Tables" column
> to both \dRp and \dRp+.
> 
> 
> Looks good to me.  Attached with regression test expected output  changes.

I have committed your patch and removed the "Tables" footer for
all-tables publications, as was discussed later in the thread.

-- 
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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 10:38 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Jun 15, 2017 at 10:32 AM, Tom Lane  wrote:
>>> It's fairly hard to read this other than as telling us that the worker was
>>> launched for the EXPLAIN (although really? why aren't we skipping that if
>>> EXEC_FLAG_EXPLAIN_ONLY?), ...
>
>> Uh, because ANALYZE was used?
>
> ... er, -ENOCAFFEINE.  Nonetheless, there are no checks of
> EXEC_FLAG_EXPLAIN_ONLY in any parallel-query code, so I think
> a bet is being missed somewhere.

ExecGather() is where workers get launched, and that ain't happening
if EXEC_FLAG_EXPLAIN_ONLY is set, unless I am *very* confused about
how this works.

-- 
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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> On Thu, Jun 15, 2017 at 10:32 AM, Tom Lane  wrote:
>> It's fairly hard to read this other than as telling us that the worker was
>> launched for the EXPLAIN (although really? why aren't we skipping that if
>> EXEC_FLAG_EXPLAIN_ONLY?), ...

> Uh, because ANALYZE was used?

... er, -ENOCAFFEINE.  Nonetheless, there are no checks of
EXEC_FLAG_EXPLAIN_ONLY in any parallel-query code, so I think
a bet is being missed somewhere.

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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 10:32 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> On Thu, Jun 15, 2017 at 10:05 AM, Tom Lane  wrote:
>>> But we know, from the subsequent failed assertion, that the leader was
>>> still trying to launch parallel workers.  So that particular theory
>>> doesn't hold water.
>
>> Is there any chance that it's already trying to launch parallel
>> workers for the *next* query?
>
> Oh!  Yeah, you might be right, because the trace includes a statement
> LOG entry from the leader in between:
>
> 2017-06-13 16:44:57.179 EDT [59404ec6.2758:63] LOG:  statement: EXPLAIN 
> (analyze, timing off, summary off, costs off) SELECT * FROM tenk1;
> 2017-06-13 16:44:57.247 EDT [59404ec9.2e78:1] ERROR:  could not map dynamic 
> shared memory segment
> 2017-06-13 16:44:57.248 EDT [59404dec.2d9c:5] LOG:  worker process: parallel 
> worker for PID 10072 (PID 11896) exited with exit code 1
> 2017-06-13 16:44:57.273 EDT [59404ec6.2758:64] LOG:  statement: select 
> stringu1::int2 from tenk1 where unique1 = 1;
> TRAP: FailedAssertion("!(BackgroundWorkerData->parallel_register_count - 
> BackgroundWorkerData->parallel_terminate_count <= 1024)", File: 
> "/home/andrew/bf64/root/HEAD/pgsql.build/../pgsql/src/backend/postmaster/bgworker.c",
>  Line: 974)
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:6] LOG:  server process (PID 
> 10072) was terminated by signal 6: Aborted
>
> It's fairly hard to read this other than as telling us that the worker was
> launched for the EXPLAIN (although really? why aren't we skipping that if
> EXEC_FLAG_EXPLAIN_ONLY?), ...

Uh, because ANALYZE was used?

-- 
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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> On Thu, Jun 15, 2017 at 10:05 AM, Tom Lane  wrote:
>> But we know, from the subsequent failed assertion, that the leader was
>> still trying to launch parallel workers.  So that particular theory
>> doesn't hold water.

> Is there any chance that it's already trying to launch parallel
> workers for the *next* query?

Oh!  Yeah, you might be right, because the trace includes a statement
LOG entry from the leader in between:

2017-06-13 16:44:57.179 EDT [59404ec6.2758:63] LOG:  statement: EXPLAIN 
(analyze, timing off, summary off, costs off) SELECT * FROM tenk1;
2017-06-13 16:44:57.247 EDT [59404ec9.2e78:1] ERROR:  could not map dynamic 
shared memory segment
2017-06-13 16:44:57.248 EDT [59404dec.2d9c:5] LOG:  worker process: parallel 
worker for PID 10072 (PID 11896) exited with exit code 1
2017-06-13 16:44:57.273 EDT [59404ec6.2758:64] LOG:  statement: select 
stringu1::int2 from tenk1 where unique1 = 1;
TRAP: FailedAssertion("!(BackgroundWorkerData->parallel_register_count - 
BackgroundWorkerData->parallel_terminate_count <= 1024)", File: 
"/home/andrew/bf64/root/HEAD/pgsql.build/../pgsql/src/backend/postmaster/bgworker.c",
 Line: 974)
2017-06-13 16:45:02.652 EDT [59404dec.2d9c:6] LOG:  server process (PID 10072) 
was terminated by signal 6: Aborted

It's fairly hard to read this other than as telling us that the worker was
launched for the EXPLAIN (although really? why aren't we skipping that if
EXEC_FLAG_EXPLAIN_ONLY?), and then we see a new LOG entry for the next
statement before the leader hits its assertion failure.

> Could be -- but it could also be timing-related.  If we are in fact
> using cygwin's fork emulation, the documentation for it explains that
> it's slow: https://www.cygwin.com/faq.html#faq.api.fork
> Interestingly, it also mentions that making it work requires
> suspending the parent while the child is starting up, which probably
> does not happen on any other platform.  Of course it also makes my
> theory that the child doesn't reach dsm_attach() before the parent
> finishes the query pretty unlikely.

Well, if this was a worker launched during InitPlan() for an EXPLAIN,
the leader would have shut down the query almost immediately after
launching the worker.  So it does fit pretty well as long as you're
willing to believe that the leader got to run before the child.

But what this theory doesn't explain is: why haven't we seen this before?
It now seems like it ought to come up often, since there are several
EXPLAINs for parallel queries in that test.

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] memory fields from getrusage()

[Robert Haas]

On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby  wrote:
> On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote:
>> It might be worth adding platform-specific code for common platforms.
>
> All I care (which linux happily/happens to support) is maxrss; I was probably
> originally interested in this while digging into an issue with hash agg.
>
> I think it's fine to show zeros for unsupported fields; that's what 
> getusage(2)
> and time(1) do after all.

Meh.  I think if we're going to go to the trouble of customizing this
code by platform, we ought to get rid of values that are meaningless
on that platform.

>> it would be a good idea to install code specific to Linux that
>> displays all and only those values that are meaningful on Linux, and
>> (less importantly) similarly for macOS.  Linux is such a common
>> platform that reporting bogus zero values and omitting other fields
>> that are actually meaningful does not seem like a very good plan.
>
> That has the issue that it varies not just by OS but also by OS version.  For
> example PG already shows context switches and FS in/out puts, but they're
> nonzero only since linux 2.6 (yes, 2.4 is ancient and unsupported but still).
>
>ru_nvcsw (since Linux 2.6)
>ru_inblock (since Linux 2.6.22)

If people who are still running 10 or 15 year old versions of Linux
see some zeroes for fields that might've been populated on a newer
release, I'm OK with that.  Maybe it will encourage them to upgrade.
We've completely removed platform support for systems newer than that.

> ..and other fields are "currently unused", but maybe supported in the past or
> future(?)
>ru_ixrss (unmaintained)
>   This field is currently unused on Linux.

Could be worth a bit of research here.

> Are you thinking of something like this, maybe hidden away in a separate file
> somewhere?
>
> #if defined(__linux__) || defined(BSD)
> appendStringInfo(, "!\t%ld max resident, %ld shared, %ld unshared 
> data, %ld unshared stack (kB)\n", r.ru_maxrss, r.ru_ixrss, r.ru_idrss, 
> r.ru_isrss);
> #elif defined(__darwin__)
> appendStringInfo(, "!\t%ld max resident, %ld shared, %ld unshared 
> data, %ld unshared stack (kB)\n", r.ru_maxrss/1024, r.ru_ixrss/1024, 
> r.ru_idrss/1024, r.ru_isrss/1024);
> #endif /* __linux__ */

I don't think it needs to go in a separate file.  I'd just patch ShowUsage().

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

[Marina Polyakova]

Sounds like a good idea.


Thank you!


Please add to the next CommitFest


Done: https://commitfest.postgresql.org/14/1170/


and review
somebody else's patch in exchange for having your own patch reviewed.


Of course, I remember about it.

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres 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] intermittent failures in Cygwin from select_parallel tests

[Amit Kapila]

On Thu, Jun 15, 2017 at 7:42 PM, Robert Haas  wrote:
> On Thu, Jun 15, 2017 at 10:05 AM, Tom Lane  wrote:
>>> Well, as Amit points out, there are entirely legitimate ways for that
>>> to happen.  If the leader finishes the whole query itself before the
>>> worker reaches the dsm_attach() call, it will call dsm_detach(),
>>> destroying the segment, and the worker will hit this ERROR.  That
>>> shouldn't happen very often in the real world, because we ought not to
>>> select a parallel plan in the first place unless the query is going to
>>> take a while to run, but the select_parallel test quite deliberately
>>> disarms all of the guards that would tend to discourage such plans.
>>
>> But we know, from the subsequent failed assertion, that the leader was
>> still trying to launch parallel workers.  So that particular theory
>> doesn't hold water.
>
> Is there any chance that it's already trying to launch parallel
> workers for the *next* query?
>

Yes, I think it is for next query.  If you refer the log below from lorikeet:

2017-06-13 16:44:57.179 EDT [59404ec6.2758:63] LOG:  statement:
EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM
tenk1;
2017-06-13 16:44:57.247 EDT [59404ec9.2e78:1] ERROR:  could not map
dynamic shared memory segment
2017-06-13 16:44:57.248 EDT [59404dec.2d9c:5] LOG:  worker process:
parallel worker for PID 10072 (PID 11896) exited with exit code 1
2017-06-13 16:44:57.273 EDT [59404ec6.2758:64] LOG:  statement: select
stringu1::int2 from tenk1 where unique1 = 1;
TRAP: FailedAssertion("!(BackgroundWorkerData->parallel_register_count
- BackgroundWorkerData->parallel_terminate_count <= 1024)", File:
"/home/andrew/bf64/root/HEAD/pgsql.build/../pgsql/src/backend/postmaster/bgworker.c",
Line: 974)
2017-06-13 16:45:02.652 EDT [59404dec.2d9c:6] LOG:  server process
(PID 10072) was terminated by signal 6: Aborted
2017-06-13 16:45:02.652 EDT [59404dec.2d9c:7] DETAIL:  Failed process
was running: select stringu1::int2 from tenk1 where unique1 = 1;
2017-06-13 16:45:02.652 EDT [59404dec.2d9c:8] LOG:  terminating any
other active server processes

Error "could not map dynamic shared memory segment" is due to query
"EXPLAIN .. SELECT * FROM tenk1" and Assertion failure is due to
another statement "select stringu1::int2 from tenk1 where unique1 =
1;".


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

[Robert Haas]

On Wed, Jun 14, 2017 at 4:48 AM, Marina Polyakova
 wrote:
> Now in pgbench we can test only transactions with Read Committed isolation
> level because client sessions are disconnected forever on serialization
> failures. There were some proposals and discussions about it (see message
> here [1] and thread here [2]).
>
> I suggest a patch where pgbench client sessions are not disconnected because
> of serialization or deadlock failures and these failures are mentioned in
> reports. In details:
> - transaction with one of these failures continue run normally, but its
> result is rolled back;
> - if there were these failures during script execution this "transaction" is
> marked
> appropriately in logs;
> - numbers of "transactions" with these failures are printed in progress, in
> aggregation logs and in the end with other results (all and for each
> script);
>
> Advanced options:
> - mostly for testing built-in scripts: you can set the default transaction
> isolation level by the appropriate benchmarking option (-I);
> - for more detailed reports: to know per-statement serialization and
> deadlock failures you can use the appropriate benchmarking option
> (--report-failures).
>
> Also: TAP tests for new functionality and changed documentation with new
> examples.
>
> Patches are attached. Any suggestions are welcome!

Sounds like a good idea.  Please add to the next CommitFest and review
somebody else's patch in exchange for having your own patch reviewed.

-- 
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] Getting server crash on Windows when using ICU collation

[Amit Kapila]

On Mon, Jun 12, 2017 at 10:08 AM, Ashutosh Sharma  wrote:
> PFA patch that fixes the issue described in above thread. As mentioned
> in the above thread, the crash is basically happening in varstr_cmp()
> function and  it's  only happening on Windows because in varstr_cmp(),
> if the collation provider is ICU, we don't even think of calling ICU
> functions to compare the string. Infact, we directly attempt to call
> the OS function wsccoll*() which is not expected. Thanks.
>

Your analysis is right, basically, when ICU is enabled we need to use
ICU related functions and use corresponding information in the
pg_locale structure. However, I see few problems with your patch.

+ if (mylocale)
+ {
+ if (mylocale->provider == COLLPROVIDER_ICU)
+ {
+#ifdef USE_ICU
+#ifdef HAVE_UCOL_STRCOLLUTF8
+ if (GetDatabaseEncoding() == PG_UTF8)
+ {
+ UErrorCode status;
+
+ status = U_ZERO_ERROR;
+ result = ucol_strcollUTF8(mylocale->info.icu.ucol,
+  arg1, len1,
+  arg2, len2,
+  );

On Windows, we need to map UTF-8 strings to UTF-16 as we are doing in
this function for other Windows specific comparisons for UTF-8
strings.  Also, we don't want to screw memcmp optimization we have in
this function, so do this ICU specific checking after memcmp check.


-- 
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] Disallowing multiple queries per PQexec()

[Daniel Verite]

Andres Freund wrote:

> Since it's an application writer's choice whether to use it,
> it seems to make not that much sense to have a
> serverside guc - it can't really be sensible set.

The application writers who are concerned by this wouldn't
know that they have a choice. If there were informed, 
supposedly they would grok the SQL syntax to begin with,
understanding the necessity and workings of proper quoting, and
the problem would not exist.

What is proposed AFAIU is an optional policy to be set on already
developed client apps, not a setting that is meant to be played with
by them.

An analogy I can find in existing GUCs, and that incidentally is
actually relevant to me as an app writer, is lo_compat_privileges
It's SUSET, it's not GUC_REPORT. Either it's on and the app
is not subject to permission checking for large objects,
or it's off and it is subject to them.
It's something that is relevant at deployment time, and not really besides
that, and it's the DBA's problem to set the policy depending on the app
requirements and the security requirements, rather than the app's problem
to adjust to whatever value there is in there.
As an example of app requirement, if the app has to let a user create a
large object and a different user to delete it, this GUC must be on,
otherwise such a scenario is not allowed, as unlinking is not a grantable
privilege, just like drop table.


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


-- 
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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 10:05 AM, Tom Lane  wrote:
>> Well, as Amit points out, there are entirely legitimate ways for that
>> to happen.  If the leader finishes the whole query itself before the
>> worker reaches the dsm_attach() call, it will call dsm_detach(),
>> destroying the segment, and the worker will hit this ERROR.  That
>> shouldn't happen very often in the real world, because we ought not to
>> select a parallel plan in the first place unless the query is going to
>> take a while to run, but the select_parallel test quite deliberately
>> disarms all of the guards that would tend to discourage such plans.
>
> But we know, from the subsequent failed assertion, that the leader was
> still trying to launch parallel workers.  So that particular theory
> doesn't hold water.

Is there any chance that it's already trying to launch parallel
workers for the *next* query?

>> Of course, as Amit also points out, it could also be the result of
>> some bug, but I'm not sure we have any reason to think so.
>
> The fact that we've only seen this on cygwin leads the mind in the
> direction of platform-specific problems.  Both this case and lorikeet's
> earlier symptoms could be explained if the parameters passed from leader
> to workers somehow got corrupted occasionally; so that's what I've been
> thinking about, but I'm not seeing anything.

Could be -- but it could also be timing-related.  If we are in fact
using cygwin's fork emulation, the documentation for it explains that
it's slow: https://www.cygwin.com/faq.html#faq.api.fork

Interestingly, it also mentions that making it work requires
suspending the parent while the child is starting up, which probably
does not happen on any other platform.  Of course it also makes my
theory that the child doesn't reach dsm_attach() before the parent
finishes the query pretty unlikely.

-- 
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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> On Wed, Jun 14, 2017 at 6:01 PM, Tom Lane  wrote:
>> The lack of any other message before the 'could not map' failure must,
>> then, mean that dsm_attach() couldn't find an entry in shared memory
>> that it wanted to attach to.  But how could that happen?

> Well, as Amit points out, there are entirely legitimate ways for that
> to happen.  If the leader finishes the whole query itself before the
> worker reaches the dsm_attach() call, it will call dsm_detach(),
> destroying the segment, and the worker will hit this ERROR.  That
> shouldn't happen very often in the real world, because we ought not to
> select a parallel plan in the first place unless the query is going to
> take a while to run, but the select_parallel test quite deliberately
> disarms all of the guards that would tend to discourage such plans.

But we know, from the subsequent failed assertion, that the leader was
still trying to launch parallel workers.  So that particular theory
doesn't hold water.

> Of course, as Amit also points out, it could also be the result of
> some bug, but I'm not sure we have any reason to think so.

The fact that we've only seen this on cygwin leads the mind in the
direction of platform-specific problems.  Both this case and lorikeet's
earlier symptoms could be explained if the parameters passed from leader
to workers somehow got corrupted occasionally; so that's what I've been
thinking about, but I'm not seeing anything.

Would someone confirm my recollection that the cygwin build does *not*
use EXEC_BACKEND, but relies on a cygwin-provided emulation of fork()?
How does that emulation work, anyway?

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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Wed, Jun 14, 2017 at 6:01 PM, Tom Lane  wrote:
> I wrote:
>> But surely the silent treatment should only apply to DSM_OP_CREATE?
>
> Oh ... scratch that, it *does* only apply to DSM_OP_CREATE.
>
> The lack of any other message before the 'could not map' failure must,
> then, mean that dsm_attach() couldn't find an entry in shared memory
> that it wanted to attach to.  But how could that happen?

Well, as Amit points out, there are entirely legitimate ways for that
to happen.  If the leader finishes the whole query itself before the
worker reaches the dsm_attach() call, it will call dsm_detach(),
destroying the segment, and the worker will hit this ERROR.  That
shouldn't happen very often in the real world, because we ought not to
select a parallel plan in the first place unless the query is going to
take a while to run, but the select_parallel test quite deliberately
disarms all of the guards that would tend to discourage such plans.

Of course, as Amit also points out, it could also be the result of
some bug, but I'm not sure we have any reason to think so.

-- 
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] Refreshing subscription relation state inside a transaction block

[Peter Eisentraut]

On 6/15/17 02:41, Petr Jelinek wrote:
> Hmm, forcibly stopping currently running table sync is not what was
> intended, I'll have to look into it. We should not be forcibly stopping
> anything except the main apply worker during drop subscription (and we
> do that only because we can't drop the remote replication slot otherwise).

The change being complained about was specifically to address the
problem described in the commit message:

Stop table sync workers when subscription relation entry is removed

When a table sync worker is in waiting state and the subscription table
entry is removed because of a concurrent subscription refresh, the
worker could be left orphaned.  To avoid that, explicitly stop the
worker when the pg_subscription_rel entry is removed.


Maybe that wasn't the best solution.  Alternatively, the tablesync
worker has to check itself whether the subscription relation entry has
disappeared, or we need a post-commit check to remove orphaned workers.

-- 
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] Adding connection id in the startup message

[Tom Lane]

Satyanarayana Narlapuram  writes:
> As a cloud service, Azure Database for PostgreSQL uses a gateway proxy to 
> route connections to a node hosting the actual server. Potentially there 
> could be multiple hops (for example client, optional proxy at the client like 
> pgbouncer for connection pooling, Azure gateway proxy, backend server) in 
> between the client, and the server. For various reasons (client firewall 
> rules, network issues etc.), the connection can be dropped before it is fully 
> authenticated at one of these hops, and it becomes extremely difficult to say 
> where and why the connection is dropped.
> The proposal is to tweak the connectivity wire protocol, and add a connection 
> id (GUID) filed in the startup message. We can trace the connection using 
> this GUID and investigate further on where the connection failed.
> Client adds a connection id in the startup message and send it to the server 
> it is trying to connect to. Proxy logs the connection id information in its 
> logs, and passes it to the server. Server logs the connection Id in the 
> server log, and set it in the GUC variable (ConnectionId).

> When an attempt to connection to the server fails, the connection failed 
> message must include the connection id in the message. This Id can be used to 
> trace the connection end to end.
> Customers can provide this Id to the support team to investigate the 
> connectivity issues to the server, along with the server information.

This seems like a lot of added mechanism for not very much gain.
In particular, it wouldn't help at all unless the client side were
also on board with generating a connection UUID and making it visible
to the end user, and then you'd have to get proxy authors on board,
etc etc, so you have to sell the idea to a lot more people than just the
server hackers.  Can you give a concrete example where this would have
helped above and beyond knowing, eg, the source and time of the connection
attempt?

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] Typo in comment in ecpg datetime.c

[Peter Eisentraut]

On 6/15/17 04:54, Daniel Gustafsson wrote:
> Spotted s/fiedls/fields/ in src/interfaces/ecpg/pgtypeslib/datetime.c per the
> attached patch.

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] Making server name part of the startup message

[Peter Eisentraut]

On 6/15/17 03:20, Satyanarayana Narlapuram wrote:
> As a cloud service, Azure Database for PostgreSQL uses a gateway proxy
> to route connections to a node hosting the actual server. To do that,
> the proxy needs to know the name of the server it tries to locate. As a
> work-around we currently overload the username parameter to pass in the
> server name using username@servername convention. It is purely a
> convention that our customers need to follow and understand. We would
> like to extend the PgSQL connection protocol to add an optional
> parameter for the server name to help with this scenario.

I think this could be useful if it's something like what HTTP uses.

-- 
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] Making server name part of the startup message

[Tom Lane]

Satyanarayana Narlapuram  writes:
> As a cloud service, Azure Database for PostgreSQL uses a gateway proxy to 
> route connections to a node hosting the actual server. To do that, the proxy 
> needs to know the name of the server it tries to locate. As a work-around we 
> currently overload the username parameter to pass in the server name using 
> username@servername convention. It is purely a convention that our customers 
> need to follow and understand. We would like to extend the PgSQL connection 
> protocol to add an optional parameter for the server name to help with this 
> scenario.

We don't actually have any concept of a server name at the moment,
and it isn't very clear what introducing that concept would buy.
Please explain.

> Proposed changes:
> Change the Postgres wire protocol to include server name in the startup 
> message. This field can be an optional field driven by the connection 
> parameters for psql (-N, --servername).
> We need this extra parameter for backward compatibility.
> Make PostgreSQL server aware of the new field, and accept the startup message 
> containing this field. Though server doesn't need this field, this change 
> helps making the server name by default included in the startup message in 
> future.

This makes no sense at all.  The client is telling the server what the
server's name is?

You're going to need a very substantially more well-reasoned proposal
to have any chance of getting us to make a protocol-level change.

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