Re: [HACKERS] DROP OWNED BY ... CACADE & "could not open relation with OID" error

2016-07-20 Thread Satoshi Nagayasu
2016-07-21 13:53 GMT+09:00 Alvaro Herrera :
> Satoshi Nagayasu wrote:
>> Hi,
>>
>> I have been trying MADlib [1], a machine-learning library for PostgreSQL,
>> and when I was tying it on 9.5 and 9.6beta2, I often got following
>> error on my box.
>>
>> 
>> madpack.py : ERROR : SQL command failed:
>> SQL: DROP OWNED BY madlib_19_installcheck CASCADE;
>> ERROR:  could not open relation with OID 147056
>> 
>>
>> I wonder it could be a PostgreSQL bug or not.
>> AFAIK, this kind of error should not occur during regular SQL command 
>> execution,
>> so I guess this is a PostgreSQL bug.
>
> Agreed.
>
>> Should I dig it down deeper to reproduce in a simple procedure?
>
> Yes, please.

Ok. I will try it.

Regards,
-- 
Satoshi Nagayasu 


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


[HACKERS] PoC: Make it possible to disallow WHERE-less UPDATE and DELETE

2016-07-20 Thread David Fetter
Folks,

Please find attached a patch which makes it possible to disallow
UPDATEs and DELETEs which lack a WHERE clause.  As this changes query
behavior, I've made the new GUCs PGC_SUSET.

What say?

Thanks to Gurjeet Singh for the idea and Andrew Gierth for the tips
implementing.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c9e0ec2..c01db8d 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -7019,6 +7019,32 @@ dynamic_library_path = 
'C:\tools\postgresql;H:\my_project\lib;$libdir'
   
  
 
+ 
+  allow_empty_updates (boolean)
+  
+   allow_empty_updates configuration 
parameter
+  
+  
+  
+   
+Allow UPDATE statements that lack a WHERE clause.
+   
+  
+ 
+
+ 
+  allow_empty_deletes (boolean)
+  
+   allow_empty_deletes configuration 
parameter
+  
+  
+  
+   
+Allow DELETE statements that lack a WHERE clause.
+   
+  
+ 
+
  
 

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 29c8c4e..0c8786e 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -48,6 +48,9 @@
 /* Hook for plugins to get control at end of parse analysis */
 post_parse_analyze_hook_type post_parse_analyze_hook = NULL;
 
+bool allow_empty_deletes = true;
+bool allow_empty_updates = true;
+
 static Query *transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt);
 static Query *transformInsertStmt(ParseState *pstate, InsertStmt *stmt);
 static List *transformInsertRow(ParseState *pstate, List *exprlist,
@@ -408,6 +411,12 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
qual = transformWhereClause(pstate, stmt->whereClause,

EXPR_KIND_WHERE, "WHERE");
 
+   /* Check for allow_empty_deletes */
+   if (!allow_empty_deletes && qual == NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_STATEMENT_HAS_NO_WHERE),
+errmsg("DELETE without a WHERE clause is 
disallowed")));
+
qry->returningList = transformReturningList(pstate, 
stmt->returningList);
 
/* done building the range table and jointree */
@@ -2110,6 +2119,12 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
qual = transformWhereClause(pstate, stmt->whereClause,

EXPR_KIND_WHERE, "WHERE");
 
+   /* Check for allow_empty_updates */
+   if (!allow_empty_updates && qual == NULL)
+   ereport(ERROR,
+   (errcode(ERRCODE_STATEMENT_HAS_NO_WHERE),
+errmsg("UPDATE without a WHERE clause is 
disallowed")));
+
qry->returningList = transformReturningList(pstate, 
stmt->returningList);
 
/*
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index be924d5..254cf10 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -388,6 +388,7 @@ Section: Class 54 - Program Limit Exceeded
 # this is for wired-in limits, not resource exhaustion problems (class 
borrowed from DB2)
 54000EERRCODE_PROGRAM_LIMIT_EXCEEDED 
program_limit_exceeded
 54001EERRCODE_STATEMENT_TOO_COMPLEX  
statement_too_complex
+54002EERRCODE_STATEMENT_HAS_NO_WHERE 
statement_has_no_where
 54011EERRCODE_TOO_MANY_COLUMNS   
too_many_columns
 54023EERRCODE_TOO_MANY_ARGUMENTS 
too_many_arguments
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6ac5184..fca6b71 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -47,6 +47,7 @@
 #include "optimizer/geqo.h"
 #include "optimizer/paths.h"
 #include "optimizer/planmain.h"
+#include "parser/analyze.h"
 #include "parser/parse_expr.h"
 #include "parser/parse_type.h"
 #include "parser/parser.h"
@@ -1653,6 +1654,26 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
 
+   {
+   {"allow_empty_deletes", PGC_SUSET, QUERY_TUNING,
+   gettext_noop("Allow DELETE without a WHERE clause"),
+   NULL
+   },
+   &allow_empty_deletes,
+   true,
+   NULL, NULL, NULL
+   },
+
+   {
+   {"allow_empty_updates", PGC_SUSET, QUERY_TUNING,
+   gettext_noop("Allow UPDATE without a WHERE clause"),
+

Re: [HACKERS] DROP OWNED BY ... CACADE & "could not open relation with OID" error

2016-07-20 Thread Alvaro Herrera
Satoshi Nagayasu wrote:
> Hi,
> 
> I have been trying MADlib [1], a machine-learning library for PostgreSQL,
> and when I was tying it on 9.5 and 9.6beta2, I often got following
> error on my box.
> 
> 
> madpack.py : ERROR : SQL command failed:
> SQL: DROP OWNED BY madlib_19_installcheck CASCADE;
> ERROR:  could not open relation with OID 147056
> 
> 
> I wonder it could be a PostgreSQL bug or not.
> AFAIK, this kind of error should not occur during regular SQL command 
> execution,
> so I guess this is a PostgreSQL bug.

Agreed.

> Should I dig it down deeper to reproduce in a simple procedure?

Yes, please.

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


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


Re: [HACKERS] Declarative partitioning

2016-07-20 Thread Amit Langote
On 2016/07/19 22:53, Ashutosh Bapat wrote:
> I am seeing following warning with this set of patches.
> gram.y:4734:24: warning: assignment from incompatible pointer type [enabled
> by default]


Thanks, will fix.  Was a copy-paste error.

Thanks,
Amit




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


[HACKERS] DROP OWNED BY ... CACADE & "could not open relation with OID" error

2016-07-20 Thread Satoshi Nagayasu
Hi,

I have been trying MADlib [1], a machine-learning library for PostgreSQL,
and when I was tying it on 9.5 and 9.6beta2, I often got following
error on my box.


madpack.py : ERROR : SQL command failed:
SQL: DROP OWNED BY madlib_19_installcheck CASCADE;
ERROR:  could not open relation with OID 147056


I wonder it could be a PostgreSQL bug or not.
AFAIK, this kind of error should not occur during regular SQL command execution,
so I guess this is a PostgreSQL bug.

Should I dig it down deeper to reproduce in a simple procedure?

Regards,

[1] http://madlib.incubator.apache.org/
-- 
Satoshi Nagayasu 


-- 
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] Design for In-Core Logical Replication

2016-07-20 Thread Craig Ringer
On 21 July 2016 at 11:05, Joshua D. Drake  wrote:

> On 07/20/2016 06:35 PM, Craig Ringer wrote:
>
> First, I'd like to emphasise that logical replication has been stalled
>> for ages now because we can no longer make forward progress on core
>> features needed until we have in-core logical replication (they're
>> dismissed as irrelevant, no in core users, etc) - but we have also had
>> difficulty getting logical replication into core. To break this impasse
>> we really need logical replication in core and need to focus on getting
>> the minimum viable feature in place, not trying to make it do everything
>> all at once. Point-to-point replication with no forwarding should be
>> just fine for the first release. Lets not bog this in extra "must have"
>> features that aren't actually crucial.
>>
>
> I don't think any person who actually works on postgresql with customers
> and clearly deals with "competition" can state with any sanity that we
> don't need logical replication in core.


No, and while people used to, we're past that now.

However, infrastructure improvements to make out-of-tree logical
replication that we can get into user hands *now* rather than two+ years
from now have been getting knocked back because there's no in-tree user,
and tools like pglogical dismissed as irrelevant. Once we have logical
replication in core hopefully we can start making infrastructure progress
again as well.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


Re: [HACKERS] [BUG] pg_basebackup from disconnected standby fails

2016-07-20 Thread Michael Paquier
On Thu, Jul 21, 2016 at 11:56 AM, Amit Kapila  wrote:
> On Thu, Jul 21, 2016 at 7:28 AM, Michael Paquier
>  wrote:
>> On Wed, Jul 20, 2016 at 8:56 PM, Michael Paquier
>>  wrote:

 Yeah, I think that is totally different angle to fix this issue, so
 don't you think it is better to start a separate thread to discuss
 about it for 10.0 and mark this patch as ready for committer.
>>>
>>> I'd like to tackle this problem in 10.0, but that will strongly depend
>>> on how my patches move on in CF1 and CF2.
>>
>> By the way, thank you for taking the time to provide input. I think
>> we're in good shape here now.
>>
>
> So, if I understand correctly, then we can mark the version posted by
> you upthread [1] which includes a test along with Kyotaro's fix can be
> marked as Ready for committer.  If so, then please change the status
> of patch accordingly.

Oops. I thought you did it already. So done.
-- 
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] Design for In-Core Logical Replication

2016-07-20 Thread Joshua D. Drake

On 07/20/2016 06:35 PM, Craig Ringer wrote:


First, I'd like to emphasise that logical replication has been stalled
for ages now because we can no longer make forward progress on core
features needed until we have in-core logical replication (they're
dismissed as irrelevant, no in core users, etc) - but we have also had
difficulty getting logical replication into core. To break this impasse
we really need logical replication in core and need to focus on getting
the minimum viable feature in place, not trying to make it do everything
all at once. Point-to-point replication with no forwarding should be
just fine for the first release. Lets not bog this in extra "must have"
features that aren't actually crucial.


I don't think any person who actually works on postgresql with customers 
and clearly deals with "competition" can state with any sanity that we 
don't need logical replication in core.


JD


--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.


--
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] pg_basebackup from disconnected standby fails

2016-07-20 Thread Amit Kapila
On Thu, Jul 21, 2016 at 7:28 AM, Michael Paquier
 wrote:
> On Wed, Jul 20, 2016 at 8:56 PM, Michael Paquier
>  wrote:
>>>
>>> Yeah, I think that is totally different angle to fix this issue, so
>>> don't you think it is better to start a separate thread to discuss
>>> about it for 10.0 and mark this patch as ready for committer.
>>
>> I'd like to tackle this problem in 10.0, but that will strongly depend
>> on how my patches move on in CF1 and CF2.
>
> By the way, thank you for taking the time to provide input. I think
> we're in good shape here now.
>

So, if I understand correctly, then we can mark the version posted by
you upthread [1] which includes a test along with Kyotaro's fix can be
marked as Ready for committer.  If so, then please change the status
of patch accordingly.


[1] - 
https://www.postgresql.org/message-id/CAB7nPqTv5gmKQcNDoFGTGqoqXz2xLz4RRw247oqOJzZTVy6-7Q%40mail.gmail.com

-- 
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] [BUG] pg_basebackup from disconnected standby fails

2016-07-20 Thread Michael Paquier
On Wed, Jul 20, 2016 at 8:56 PM, Michael Paquier
 wrote:
> On Wed, Jul 20, 2016 at 8:40 PM, Amit Kapila  wrote:
>> On Wed, Jul 20, 2016 at 5:12 AM, Michael Paquier
>>  wrote:
>>> On Tue, Jul 19, 2016 at 9:08 PM, Amit Kapila  
>>> wrote:
 On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier
  wrote:
> On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila  
> wrote:
>> On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier
>>  wrote:
 Why only for back-branches? Do you have better solution for head?
>>>
>>> Yes, I mentioned an idea upthread to set up the minimum recovery point
>>> saved in the backup to the last replayed LSN. Though that's not
>>> acceptable for 9.6 as this requires changing the output of
>>> pg_stop_backup() with a new field containing the bytes of pg_control.
>>> I am not sure how others feel about that,
>>>
>>
>> Yeah, I think that is totally different angle to fix this issue, so
>> don't you think it is better to start a separate thread to discuss
>> about it for 10.0 and mark this patch as ready for committer.
>
> I'd like to tackle this problem in 10.0, but that will strongly depend
> on how my patches move on in CF1 and CF2.

By the way, thank you for taking the time to provide input. I think
we're in good shape here now.
-- 
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] Design for In-Core Logical Replication

2016-07-20 Thread Craig Ringer
On 21 July 2016 at 01:20, Simon Riggs  wrote:

> On 20 July 2016 at 17:52, Rod Taylor  wrote:
>
>
>> I think it's important for communication channels to be defined
>> separately from the subscriptions.
>>
>
> I agree and believe it will be that way.
>
> Craig is working on allowing Replication Slots to failover between nodes,
> to provide exactly that requested behaviour.
>
>

First, I'd like to emphasise that logical replication has been stalled for
ages now because we can no longer make forward progress on core features
needed until we have in-core logical replication (they're dismissed as
irrelevant, no in core users, etc) - but we have also had difficulty
getting logical replication into core. To break this impasse we really need
logical replication in core and need to focus on getting the minimum viable
feature in place, not trying to make it do everything all at once.
Point-to-point replication with no forwarding should be just fine for the
first release. Lets not bog this in extra "must have" features that aren't
actually crucial.

That said:

I had a patch in it for 9.6 to provide the foundations for logical
replication to follow physical failover, but it got pulled at the last
minute. It'll be submitted for 10.0 along with some other enhancements to
make it usable without hacky extensions, most notably support for using a
physical replication slot and hot standby feedback to pin a master's
catalog_xmin where it's needed by slots on a physical replica.

That's for when we're combining physical and logical replication though,
e.g. "node A" is a master/standby pair, and "node B" is also a
master/standby pair.

For non-star logical topologies, which is what I think you might've been
referring to, it's necessary to have:

- Node identity
- Which nodes we want to receive data from
- How we connect to each node

all of which are separate things. Who's out there, what we want from them,
and how to get it.

pglogical doesn't really separate the latter two much at this point.
Subscriptions identify both the node to connect to and the data we want to
receive from a node; there's no selective data forwarding from one node to
another. Though there's room for that in pglogical's hooks/filters by using
filtering by replication origin, it just doesn't do it yet.

It sounds like that's what you're getting at. Wanting to be able to say
"node A wants to get data from node B and node C" separately to "node A
connects to node B to receive data", with the replication system somehow
working out that that means data written from C to B should be forwarded to
A.

Right?

If so, it's not always easy to figure that out. If you create connections
to both B and C, we then have to automagically work out that we should stop
forwarding data from C over our connection to B.

The plan with pglogical has been to allow connections to specify forwarding
options, so the connection explicitly says what nodes it wants to get data
from. It's users' job to ensure that they don't declare connections that
overlap. This is simpler to implement, but harder to admin.

One challenge with either approach is ensuring a consistent switchover. If
you have a single connection A=>B receiving data from [B,C], then you
switch to two connections A=>B and A=>C with neither forwarding, you must
ensure that the switchover occurs in such a way as that no data is
replicated twice or skipped. That's made easier by the fact that we have
replication origins and we can actually safely receive from both at the
same time then discard from one of them, even use upstream filtering to
avoid sending it over the wire twice. But it does take care and caution.

Note that none of this is actually for logical _failover_, where we lose a
node. For that we need some extra help in the form of placeholder slots
maintained on other peers. This can be done at the application /
replication system level without the need for new core features, but it
might not be something we can support in the first iteration.

I'm not sure how Petr's current design for in-core replication addresses
this, if it does, or whether it's presently focused only on point-to-point
replication like pglogical. As far as I'm concerned so long as it does
direct point-to-point replication with no forwarding that's good enough for
a first cut feature, so long as the UI, catalog and schema design leaves
room for adding more later.



> I also suspect multiple publications will be normal even if only 2 nodes.
>> Old slow moving data almost always got different treatment than fast-moving
>> data; even if only defining which set needs to hit the other node first and
>> which set can trickle through later.
>>
>
> Agreed
>
>
Yes, especially since we can currently only stream transactions one by one
in commit order after commit.

Even once we have interleaved xact streaming, though, there will still be
plenty of times we want to receive different sets of data from the same
node at different priorities/rates. Small da

[HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Noah Misch
On Tue, Jul 19, 2016 at 09:01:05PM -0400, Noah Misch wrote:
> On Tue, Jul 19, 2016 at 06:09:59PM -0500, Kevin Grittner wrote:
> > On Mon, Jul 18, 2016 at 9:10 PM, Noah Misch  wrote:
> > > On Sat, Jul 16, 2016 at 06:48:08PM -0400, Noah Misch wrote:
> > >> This PostgreSQL 9.6 open item is past due for your status update.  
> > >> Kindly send
> > >> a status update within 24 hours, and include a date for your subsequent 
> > >> status
> > >> update.  Refer to the policy on open item ownership:
> > >> http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> > >
> > > IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 9.6 open item is long past 
> > > due
> > > for your status update.  Please reacquaint yourself with the policy on 
> > > open
> > > item ownership[1] and then reply immediately.  If I do not hear from you 
> > > by
> > > 2016-07-20 03:00 UTC, I will transfer this item to release management team
> > > ownership without further notice.
> > >
> > > [1] 
> > > http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> > 
> > As far as I can see, to do this the way that Andres and Amit
> > suggest involves tying in to indexam.c and other code in incredibly
> > ugly ways.  I think it is entirely the wrong way to go, as I can't
> > find a way to make it look remotely sane.  The question is whether
> > I should do it the way that I think is sane, or whether someone
> > else wants to show me what I'm missing by producing at least a
> > rough patch along these lines.
> 
> This does not qualify as a status update, because it does not include a date
> for your subsequent status update.

This PostgreSQL 9.6 open item now needs a permanent owner.  Would any other
committer like to take ownership?  If this role interests you, please read
this thread and the policy linked above, then send an initial status update
bearing a date for your subsequent status update.  If the item does not have a
permanent owner by 2016-07-24 02:00 UTC, I will resolve the item by reverting
commit 848ef42 and followups.

Thanks,
nm


-- 
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] skink's test_decoding failures in 9.4 branch

2016-07-20 Thread Tom Lane
Andres Freund  writes:
> I guess either using valgrind's gdb server on error, or putting some
> asserts checking the size would be best. I can look into it, but it'll
> not be today likely.

I believe the problem is that DecodeUpdate is not on the same page as the
WAL-writing routines about how much data there is for an old_key_tuple.
Specifically, I see this in 9.4's log_heap_update():

if (old_key_tuple)
{
...
xlhdr_idx.t_len = old_key_tuple->t_len;

rdata[nr].data = (char *) old_key_tuple->t_data
+ offsetof(HeapTupleHeaderData, t_bits);
rdata[nr].len = old_key_tuple->t_len
- offsetof(HeapTupleHeaderData, t_bits);
...
}

so that the amount of tuple data that's *actually* in WAL is
offsetof(HeapTupleHeaderData, t_bits) less than what t_len says.
However, over in DecodeUpdate, this is processed with

xl_heap_header_len xlhdr;

memcpy(&xlhdr, data, sizeof(xlhdr));
...
datalen = xlhdr.t_len + SizeOfHeapHeader;
...
DecodeXLogTuple(data, datalen, change->data.tp.oldtuple);

and what DecodeXLogTuple does is

intdatalen = len - SizeOfHeapHeader;
(so we're back to datalen == xlhdr.t_len)
...
memcpy(((char *) tuple->tuple.t_data) + offsetof(HeapTupleHeaderData, 
t_bits),
   data + SizeOfHeapHeader,
   datalen);

so that we are copying offsetof(HeapTupleHeaderData, t_bits) too much
data from the WAL buffer.  Most of the time this doesn't hurt but it's
making valgrind complain, and on a unlucky day we might crash entirely.

I have not looked to see if the bug also exists in > 9.4.  Also, it's
not very clear to me whether other call sites for DecodeXLogTuple might
have related bugs.

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] Password identifiers, protocol aging and SCRAM protocol

2016-07-20 Thread Michael Paquier
On Thu, Jul 21, 2016 at 5:25 AM, David Fetter  wrote:
> On Wed, Jul 20, 2016 at 02:12:57PM -0400, Alvaro Herrera wrote:
>> Michael Paquier wrote:
>> > On Wed, Jul 6, 2016 at 4:18 PM, Michael Paquier
>> >  wrote:
>> > > OK, after hacking that for a bit I have finished with option 2 and the
>> > > set of PG-like set of routines, the use of USE_SSL in the file
>> > > containing all the SHA functions of OpenBSD has proved to be really
>> > > ugly, but with a split things are really clear to the eye. The stuff I
>> > > got builds on OSX, Linux and MSVC. pgcrypto cannot link directly to
>> > > libpgcommon.a, so I am making it compile directly with the source
>> > > files, as it is doing on HEAD.
>> >
>> > Btw, attached is the patch I did for this part if there is any interest in 
>> > it.
>>
>> After quickly eyeballing your patch, I agree with the decision of going
>> with (2), even if my gut initially told me that (1) would be better
>> because it'd require less makefile trickery.

Yeah, I thought the same thing as well when putting my hands in the
dirt... But the in the end (2) is really less ugly.

>> I'm surprised that you say pgcrypto cannot link libpgcommon directly.
>> Is there some insurmountable problem there?  I notice your MSVC patch
>> uses libpgcommon while the Makefile symlinks the files.

I am running into some weird things when linking both on OSX... But I
am not done with it completely yet. I'll adjust that a bit more when
producing the set of patches that will be published. So let's see.

> People have, in the past, expressed concerns about linking in
> pgcrypto.  Apparently, in some countries, it's a legal problem.

Do you have any references? I don't see that as a problem.
-- 
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] Password identifiers, protocol aging and SCRAM protocol

2016-07-20 Thread Michael Paquier
On Thu, Jul 21, 2016 at 12:15 AM, Robert Haas  wrote:
> On Fri, Jul 15, 2016 at 9:30 AM, Michael Paquier
>  wrote:
>> OK, I am doing that at the end.
>>
>> And also while moving on...
>>
>> On another topic, here are some ideas to extend CREATE/ALTER ROLE to
>> support SCRAM password directly:
>> 1) protocol PASSWORD value, where protocol is { MD5 | PLAIN | SCRAM }, 
>> giving:
>> CREATE ROLE foorole SCRAM PASSWORD value;
>> 2) PASSWORD (protocol) value.
>> 3) Just add SCRAM PASSWORD
>> My mind is thinking about 1) as being the cleanest solution as this
>> does not touch the defaults, which may change a couple of releases
>> later. Other opinions?
>
> I can't really understand what you are saying here, but I'm going to
> be -1 on adding SCRAM as a parser keyword.  Let's pick a syntax like
> "PASSWORD SConst USING SConst" or "PASSWORD SConst ENCRYPTED WITH
> SConst".

No, I do not mean to make SCRAM or MD5 keywords. While hacking that, I
got at some point in the mood of using "PASSWORD Sconst Sconst" but
that's ugly. Sticking a keyword in between makes more sense, and USING
is a good idea. I haven't thought of this one.

By the way, the core patch does not have any grammar extension. The
grammar extension will be on top of it and the core patch can just
activate scram passwords using password_encryption. That's user
unfriendly, but as the patch is large I try to cut it in as many
pieces as necessary.
-- 
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] Design for In-Core Logical Replication

2016-07-20 Thread Petr Jelinek

On 20/07/16 19:07, Simon Riggs wrote:

On 20 July 2016 at 16:39, Joshua D. Drake mailto:j...@commandprompt.com>> wrote:

  Logical Replication uses a Publish and Subscribe model
with one or
  more Subscribers subscribing to one or more Publications on a
  Provider node. Subscribers pull data from the Publications
they
  subscribe to and may subsequently re-publish data to allow
  cascading replication or more complex configurations.


Is that somehow different than Origin/Subscriber or Master/Slave? If
not, why are we using yet more terms?


Thanks for asking, an important question that we have a chance to get
right before we go too far down the road of implementation.

I'll explain my thinking, so we can discuss the terms I've recommended,
which can be summarized as:
A Provider node has one or more Databases, each of which can publish its
data in zero, one or more PUBLICATIONs. A Subscribing node can receive
data in the form of zero, one or more SUBSCRIBERs, where each SUBSCRIBER
may bring together data from one or more PUBLICATIONs

Here's why...



Just to add to what Simon wrote. There is one more reason for not using 
term origin for this - origin of data does not necessarily have to be on 
the provider database once there is a cascading so it does not really 
map all that well.


--
  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] Odd error when using UNION and COLLATE

2016-07-20 Thread Bruce Momjian
On Wed, Jul 20, 2016 at 06:03:08PM -0400, Tom Lane wrote:
> Greg Stark  writes:
> > But I think I agree that it's surprising that the collate clause isn't
> > working in the ORDER BY on a column produced by a UNION. Certainly
> > that's where people usually want to put it.
> 
> See this ancient comment in transformSetOperationStmt:
> 
>  * For now, we don't support resjunk sort clauses on the output of a
>  * setOperation tree --- you can only use the SQL92-spec options of
>  * selecting an output column by name or number.  Enforce by checking that
>  * transformSortClause doesn't add any items to tlist.
> 
> Perhaps sometime we ought to make an effort to relax that.

Oh, I didn't see that above the error block.

-- 
  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] Odd error when using UNION and COLLATE

2016-07-20 Thread Bruce Momjian
On Wed, Jul 20, 2016 at 10:55:38PM +0100, Greg Stark wrote:
> On Wed, Jul 20, 2016 at 10:38 PM, Bruce Momjian  wrote:
> > SELECT 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY x COLLATE "C";
> 
> 
> ::***> select 'a-c' COLLATE "C" AS x UNION ALL SELECT 'ab' AS x ORDER BY x ;

Oh, collate on the string, before AS. I never thought of that.

-- 
  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] Odd error when using UNION and COLLATE

2016-07-20 Thread Tom Lane
Greg Stark  writes:
> But I think I agree that it's surprising that the collate clause isn't
> working in the ORDER BY on a column produced by a UNION. Certainly
> that's where people usually want to put it.

See this ancient comment in transformSetOperationStmt:

 * For now, we don't support resjunk sort clauses on the output of a
 * setOperation tree --- you can only use the SQL92-spec options of
 * selecting an output column by name or number.  Enforce by checking that
 * transformSortClause doesn't add any items to tlist.

Perhaps sometime we ought to make an effort to relax 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] Odd error when using UNION and COLLATE

2016-07-20 Thread Greg Stark
Actually there's nothing about UNION here. It's true for any column alias:

::***> select 'a-c' AS x ORDER BY x COLLATE "C" ;
ERROR:  42703: column "x" does not exist
LINE 2: select 'a-c' AS x ORDER BY x COLLATE "C" ;
   ^
LOCATION:  errorMissingColumn, parse_relation.c:2892
Time: 0.204 ms

Also you don't need WITH, just an old-fashioned inline view:

::***> select * from (select 'a-c'::text AS x) as subquery ORDER BY x
COLLATE "C" ;
┌─┐
│  x  │
├─┤
│ a-c │
└─┘
(1 row)


-- 
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] Odd error when using UNION and COLLATE

2016-07-20 Thread David G. Johnston
On Wed, Jul 20, 2016 at 5:38 PM, Bruce Momjian  wrote:

> I think the 'ORDER BY x COLLATE "C"' is being parsed as an a_expr, and
> we don't allow a_expr in a UNION.  Perhaps we are too strict here, but I
> can't tell.
>

​ORDER BY 1 COLLATE "C" is indeed an expression - the number no longer
refers to a column position but it is a constant.  The presence or absence
of UNION doesn't factor into things here - the expression itself is useless
on its face.​

This one is a bit different in cause but I suspect is working as well as
can be expected.

SELECT 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY x COLLATE "C";

​David J.​


Re: [HACKERS] Odd error when using UNION and COLLATE

2016-07-20 Thread Greg Stark
On Wed, Jul 20, 2016 at 10:38 PM, Bruce Momjian  wrote:
> SELECT 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY x COLLATE "C";


::***> select 'a-c' COLLATE "C" AS x UNION ALL SELECT 'ab' AS x ORDER BY x ;
┌─┐
│  x  │
├─┤
│ a-c │
│ ab  │
└─┘
(2 rows)


But I think I agree that it's surprising that the collate clause isn't
working in the ORDER BY on a column produced by a UNION. Certainly
that's where people usually want to put it.

-- 
greg


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


[HACKERS] Odd error when using UNION and COLLATE

2016-07-20 Thread Bruce Momjian
Seems you can't use UNION and COLLATE in the same SELECT statement;  you
have to put the UNION inside of WITH and then do the COLLATE outside:

 test=> SELECT 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY 1 COLLATE "C";
 ERROR:  collations are not supported by type integer
 LINE 1: ... 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY 1 COLLATE "C...
 ^


 test=> SELECT 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY x COLLATE "C";
 ERROR:  invalid UNION/INTERSECT/EXCEPT ORDER BY clause
 LINE 1: ...CT 'a-c' AS x UNION ALL SELECT 'ab' AS x ORDER BY x COLLATE ...
 ^
 DETAIL:  Only result column names can be used, not expressions or functions.
 HINT:  Add the expression/function to every SELECT, or move the UNION into a 
FROM clause.


 test=> WITH d AS (SELECT 'a-c' AS x UNION ALL SELECT 'ab' AS x) SELECT * FROM 
d ORDER BY x COLLATE "C";
   x
 -
  a-c
  ab
 (2 rows)

I think the 'ORDER BY x COLLATE "C"' is being parsed as an a_expr, and
we don't allow a_expr in a UNION.  Perhaps we are too strict here, but I
can't tell.

-- 
  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] Password identifiers, protocol aging and SCRAM protocol

2016-07-20 Thread David Fetter
On Wed, Jul 20, 2016 at 02:12:57PM -0400, Alvaro Herrera wrote:
> Michael Paquier wrote:
> > On Wed, Jul 6, 2016 at 4:18 PM, Michael Paquier
> >  wrote:
> > > OK, after hacking that for a bit I have finished with option 2 and the
> > > set of PG-like set of routines, the use of USE_SSL in the file
> > > containing all the SHA functions of OpenBSD has proved to be really
> > > ugly, but with a split things are really clear to the eye. The stuff I
> > > got builds on OSX, Linux and MSVC. pgcrypto cannot link directly to
> > > libpgcommon.a, so I am making it compile directly with the source
> > > files, as it is doing on HEAD.
> > 
> > Btw, attached is the patch I did for this part if there is any interest in 
> > it.
> 
> After quickly eyeballing your patch, I agree with the decision of going
> with (2), even if my gut initially told me that (1) would be better
> because it'd require less makefile trickery.
> 
> I'm surprised that you say pgcrypto cannot link libpgcommon directly.
> Is there some insurmountable problem there?  I notice your MSVC patch
> uses libpgcommon while the Makefile symlinks the files.

People have, in the past, expressed concerns about linking in
pgcrypto.  Apparently, in some countries, it's a legal problem.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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] Password identifiers, protocol aging and SCRAM protocol

2016-07-20 Thread Alvaro Herrera
Michael Paquier wrote:
> On Wed, Jul 6, 2016 at 4:18 PM, Michael Paquier
>  wrote:
> > OK, after hacking that for a bit I have finished with option 2 and the
> > set of PG-like set of routines, the use of USE_SSL in the file
> > containing all the SHA functions of OpenBSD has proved to be really
> > ugly, but with a split things are really clear to the eye. The stuff I
> > got builds on OSX, Linux and MSVC. pgcrypto cannot link directly to
> > libpgcommon.a, so I am making it compile directly with the source
> > files, as it is doing on HEAD.
> 
> Btw, attached is the patch I did for this part if there is any interest in it.

After quickly eyeballing your patch, I agree with the decision of going
with (2), even if my gut initially told me that (1) would be better
because it'd require less makefile trickery.

I'm surprised that you say pgcrypto cannot link libpgcommon directly.
Is there some insurmountable problem there?  I notice your MSVC patch
uses libpgcommon while the Makefile symlinks the files.

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


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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Andres Freund
On 2016-07-20 13:59:32 -0400, Robert Haas wrote:
> It's hard to believe that it's equally good to use the newest
> registered snapshot (which is, I think, what you will often get from
> GetActiveSnapshot()) and the oldest registered snapshot (which is what
> you will get from pairingheap_first()).  It seems to me that we need
> to analyze what happens if we choose a snapshot that is older than the
> one used to find the datum which contained the toast pointer, and
> conversely what happens if we use a snapshot that is newer than the
> one we used to find the toast pointer.

Yea, the oldest seems better.


> Here's an attempt:
> 
> 1. If we pick a snapshot that is older than the one that found the
> scan tuple, we might get a "snapshot too old" error that is not
> strictly necessary.

Right. Which still seems a lot better than essentially pessimizing
vacuuming for toast tables considerably.


> 2. If we pick a snapshot that is newer than the one that found the
> scan tuple, then haven't we failed to fix the problem?  I'm not so
> sure about this direction, but if it's OK to test an arbitrarily new
> snapshot, then I can't see why we need the test at all.

I think some argument could be construed why it'd possibly be safe, but
I feel a lot better with the other option.

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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Robert Haas
On Wed, Jul 20, 2016 at 12:30 PM, Andres Freund  wrote:
>> And how do you obtain that?  The functions that reference
>> SnapshotToast are toast_delete_datum, toastrel_value_exists, and
>> toast_fetch_datum, toast_fetch_datum_slice, but none of those take a
>> snapshot as an argument, nor is there any reasonable way to make them
>> do so.  Those are indirectly called by things like bttextcmp, which
>> don't know what snapshot was used to fetch the datum that they are
>> detoasting and can't reasonably be made to know.
>>
>> I mean, you could do something *approximately* correct by calling
>> GetActiveSnapshot() but that doesn't seem likely to be correct in
>> detail.
>
> GetActiveSnapshot() seems like it should work well enough in this case,
> or we could use pairingheap_first() to get the actual oldest registered
> one.

It's hard to believe that it's equally good to use the newest
registered snapshot (which is, I think, what you will often get from
GetActiveSnapshot()) and the oldest registered snapshot (which is what
you will get from pairingheap_first()).  It seems to me that we need
to analyze what happens if we choose a snapshot that is older than the
one used to find the datum which contained the toast pointer, and
conversely what happens if we use a snapshot that is newer than the
one we used to find the toast pointer.

Here's an attempt:

1. If we pick a snapshot that is older than the one that found the
scan tuple, we might get a "snapshot too old" error that is not
strictly necessary.

2. If we pick a snapshot that is newer than the one that found the
scan tuple, then haven't we failed to fix the problem?  I'm not so
sure about this direction, but if it's OK to test an arbitrarily new
snapshot, then I can't see why we need the test at all.

-- 
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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Alvaro Herrera
Andres Freund wrote:
> On 2016-07-20 11:26:11 -0400, Robert Haas wrote:
> > On Wed, Jul 20, 2016 at 3:39 AM, Andres Freund  wrote:
> > >>I think Snapshot's members whenTaken and lsn are updated/initialized
> > >>only in GetSnapshotData().  So if GetSnapshotData() is not used, how
> > >>will you expect those fields to be updated.  We need those fields to
> > >>be updated for TestForOldSnapshot().
> > >
> > > That's why I suggested copying them from the current mvcc snapshot.
> > 
> > And how do you obtain that?  The functions that reference
> > SnapshotToast are toast_delete_datum, toastrel_value_exists, and
> > toast_fetch_datum, toast_fetch_datum_slice, but none of those take a
> > snapshot as an argument, nor is there any reasonable way to make them
> > do so.  Those are indirectly called by things like bttextcmp, which
> > don't know what snapshot was used to fetch the datum that they are
> > detoasting and can't reasonably be made to know.
> > 
> > I mean, you could do something *approximately* correct by calling
> > GetActiveSnapshot() but that doesn't seem likely to be correct in
> > detail.
> 
> GetActiveSnapshot() seems like it should work well enough in this case,
> or we could use pairingheap_first() to get the actual oldest registered
> one.

Hmm.  Why is the active snapshot not sufficient?  Perhaps we need some
kind of redesign or minor tweak to snapmgr to keep track of the oldest
snapshot of a resowner or something like that?

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


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


Re: [HACKERS] Design for In-Core Logical Replication

2016-07-20 Thread Simon Riggs
On 20 July 2016 at 17:52, Rod Taylor  wrote:


> I think it's important for communication channels to be defined separately
> from the subscriptions.
>

I agree and believe it will be that way.

Craig is working on allowing Replication Slots to failover between nodes,
to provide exactly that requested behaviour.


> So, I'd rather have:
>
> CREATE CONNECTION machine1;
> CREATE CONNECTION machine2;
>

I think those map to replication slots. (This discussion might get a bit
confusing if we try to guess exactly what each others terms mean, so I'll
go no further than "I think").


> CREATE SUBSCRIPTION TO PUBLICATION mypub;
>

Yep


> I'm not certain the subscription needs to be named. IMO, a publication
> should have the same properties on all nodes (so any node may become the
> primary source). If a subscriber needs different behaviour for a
> publication, it should be created as a different publication.
>

Understood, its mostly to allow it to be dropped or altered and monitored.
It's kindof like an index, it needs a name, we just don't much care what it
is.


> Documenting that ThisPub is different from ThatPub is easier than
> documenting that ThisPub on node 1/2/4 is different from ThisPub on node
> 7/8, except Node 7 is temporarily on Node 4 too (database X instead of
> database Y) due to that power problem.
>

Which is why pg_dump support is important to allow us to sync up the
definitions.


> Clearly this is advanced. An initial implementation may only allow mypub
> from a single connection.
>

Good input and clearly explained, thanks. If any of the above changes,
these requirements will remain noted.


> I also suspect multiple publications will be normal even if only 2 nodes.
> Old slow moving data almost always got different treatment than fast-moving
> data; even if only defining which set needs to hit the other node first and
> which set can trickle through later.
>

Agreed


-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Design for In-Core Logical Replication

2016-07-20 Thread Simon Riggs
On 20 July 2016 at 16:39, Joshua D. Drake  wrote:


>  Logical Replication is a method of replicating data objects and their
>>  changes, based upon their Primary Keys (or Replication Identity). We
>>
>
> Do we want a limitation based on Primary Key, or would it be possible to
> use just UNIQUE or is that covered under Replication Identity?


That is covered by replication identity.


>
>>  Logical Replication uses a Publish and Subscribe model with one or
>>  more Subscribers subscribing to one or more Publications on a
>>  Provider node. Subscribers pull data from the Publications they
>>  subscribe to and may subsequently re-publish data to allow
>>  cascading replication or more complex configurations.
>>
>
> Is that somehow different than Origin/Subscriber or Master/Slave? If not,
> why are we using yet more terms?


Thanks for asking, an important question that we have a chance to get right
before we go too far down the road of implementation.

Issue: We need a noun for CREATE "SOMETHING" (or pg_create_$NOUN). So what
noun to use? SQLStandard gives us no guidance here.

I'll explain my thinking, so we can discuss the terms I've recommended,
which can be summarized as:
A Provider node has one or more Databases, each of which can publish its
data in zero, one or more PUBLICATIONs. A Subscribing node can receive data
in the form of zero, one or more SUBSCRIBERs, where each SUBSCRIBER may
bring together data from one or more PUBLICATIONs

Here's why...

Master/Slave is not appropriate, since both sending and receiving nodes are
Masters.

Origin/Subscriber is used by Slony. The term "Replication Origin" is
already used in PG9.5 for something related, but not identical.
Provider/Subscriber is used by Londiste.
Bucardo seems to use Master/Slave according to FAQ.

The Noun we are discussing is something that a single Database can have >1
of, so those terms aren't quite appropriate.

pglogical uses Provider/Subscriber and Replication Sets, so I started with
the thought that we might want CREATE REPLICATION SET or
pg_create_replication_set(). After some time considering this, ISTM that
the term "replication set" may not be that useful since we foresee a future
where data is actually filtered and transformed and the feature set extends
well beyond what we have with Slony, so I began looking for a term that was
general and obvious (POLA).

After some thought, I realised that we are describing this as "Publish &
Subscribe", so it makes a lot of sense to just use the terms Publication &
Subscription. Those phrases are commonly used by SQLServer, Sybase, Oracle,
Redis, RabbitMQ etc which is a pretty big set.
It's also a commonly used Enterprise Integration Design pattern
https://en.wikipedia.org/wiki/Publish–subscribe_pattern
I note especially that Publish/Subscribe does not imply any particular
topology (a mistake I made earlier when I called this stuff BDR, which
confused everybody when we tried to talk about a subset of that
functionality called UDR).
http://www.slideshare.net/ishraqabd/publish-subscribe-model-overview-13368808

So that brings us to...
A Provider node has one or more Databases, each of which can publish its
data in zero, one or more PUBLICATIONs. A Subscribing node can receive data
in the form of zero, one or more SUBSCRIBERs, where each SUBSCRIBER may
bring together data from one or more PUBLICATIONs.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] skink's test_decoding failures in 9.4 branch

2016-07-20 Thread Andres Freund
Hi,

On 2016-07-20 12:45:04 -0400, Tom Lane wrote:
> I wrote:
> > I've still had no luck reproducing it here, though.

Same here so far.

> Hah --- I take that back.  On about the fourth or fifth trial:

Interesting.


> ==00:00:00:34.291 21525== Invalid read of size 1
> ==00:00:00:34.291 21525==at 0x4A08DEC: memcpy (mc_replace_strmem.c:882)
> ==00:00:00:34.291 21525==by 0x66FA54: DecodeXLogTuple (decode.c:899)
> ==00:00:00:34.291 21525==by 0x670561: LogicalDecodingProcessRecord 
> (decode.c:711)
> ==00:00:00:34.291 21525==by 0x671BC3: pg_logical_slot_get_changes_guts 
> (logicalfuncs.c:440)
> ==00:00:00:34.291 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
> (execQual.c:2196)
> ==00:00:00:34.291 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
> ==00:00:00:34.291 21525==by 0x5C170D: ExecScan (execScan.c:82)
> ==00:00:00:34.291 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
> ==00:00:00:34.291 21525==by 0x5B8A61: standard_ExecutorRun 
> (execMain.c:1490)
> ==00:00:00:34.291 21525==by 0x6BFE36: PortalRunSelect (pquery.c:942)
> ==00:00:00:34.291 21525==by 0x6C11EF: PortalRun (pquery.c:786)
> ==00:00:00:34.291 21525==by 0x6BD7E3: exec_simple_query (postgres.c:1072)
> ==00:00:00:34.291 21525==  Address 0xe5311d6 is 6 bytes after a block of size 
> 8,192 alloc'd
> ==00:00:00:34.291 21525==at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
> ==00:00:00:34.291 21525==by 0x4ED399: XLogReaderAllocate (xlogreader.c:83)
> ==00:00:00:34.291 21525==by 0x6710B3: StartupDecodingContext 
> (logical.c:161)
> ==00:00:00:34.291 21525==by 0x671303: CreateDecodingContext 
> (logical.c:413)
> ==00:00:00:34.291 21525==by 0x671AF7: pg_logical_slot_get_changes_guts 
> (logicalfuncs.c:394)
> ==00:00:00:34.291 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
> (execQual.c:2196)
> ==00:00:00:34.291 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
> ==00:00:00:34.291 21525==by 0x5C170D: ExecScan (execScan.c:82)
> ==00:00:00:34.291 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
> ==00:00:00:34.291 21525==by 0x5B8A61: standard_ExecutorRun 
> (execMain.c:1490)
> ==00:00:00:34.291 21525==by 0x6BFE36: PortalRunSelect (pquery.c:942)
> ==00:00:00:34.291 21525==by 0x6C11EF: PortalRun (pquery.c:786)
> ==00:00:00:34.291 21525== 

> This is rather interesting because I do not recall that any of skink's
> failures have shown an access more than 1 byte past the end of the buffer.
> 
> Any suggestions how to debug this?

I guess either using valgrind's gdb server on error, or putting some
asserts checking the size would be best. I can look into it, but it'll
not be today likely.

Regards,

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] One process per session lack of sharing

2016-07-20 Thread Teodor Sigaev

On 12 July 2016 at 09:57, mailto:amatv...@bitec.ru>> wrote:

We have faced with some lack of sharing resources.
So in our test memory usage per session:
Oracle: about 5M
MSSqlServer: about 4M
postgreSql: about 160М


Using shared resources also has significant problems, so care must be taken.


To see memory allocation by opinion of pgsql it's possible to use
https://github.com/postgrespro/memstat

This module (9.6+) could collect information about MemoryContexts allocation.

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
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] Design for In-Core Logical Replication

2016-07-20 Thread Rod Taylor
On Wed, Jul 20, 2016 at 4:08 AM, Simon Riggs  wrote:


>
>   
> And on Subscriber database:
> 
> CREATE SUBSCRIPTION mysub WITH CONNECTION dbname=foo host=bar
> user=repuser PUBLICATION mypub;
> 
>   
>   
> The above will start the replication process which synchronizes the
> initial table contents of users and
> departments tables and then starts replicating
> incremental changes to those tables.
>   
> 
> 
>

I think it's important for communication channels to be defined separately
from the subscriptions.

If I have nodes 1/2 + 3/4 which operate in pairs, I don't really want to
have to have a script reconfigure replication on 3/4 every-time we do
maintenance on 1 or 2.

3/4 need to know they subscribe to mypub and that they have connections to
machine 1 and machine 2. The replication system should be able to figure
out which (of 1/2) has the most recently available data.


So, I'd rather have:

CREATE CONNECTION machine1;
CREATE CONNECTION machine2;
CREATE SUBSCRIPTION TO PUBLICATION mypub;

Notice I explicitly did not tell it how to get the publication but if we
did have a preference the DNS weighting model might be appropriate.

I'm not certain the subscription needs to be named. IMO, a publication
should have the same properties on all nodes (so any node may become the
primary source). If a subscriber needs different behaviour for a
publication, it should be created as a different publication.

Documenting that ThisPub is different from ThatPub is easier than
documenting that ThisPub on node 1/2/4 is different from ThisPub on node
7/8, except Node 7 is temporarily on Node 4 too (database X instead of
database Y) due to that power problem.


Clearly this is advanced. An initial implementation may only allow mypub
from a single connection.


I also suspect multiple publications will be normal even if only 2 nodes.
Old slow moving data almost always got different treatment than fast-moving
data; even if only defining which set needs to hit the other node first and
which set can trickle through later.

regards,

Rod Taylor


Re: [HACKERS] skink's test_decoding failures in 9.4 branch

2016-07-20 Thread Tom Lane
I wrote:
> I've still had no luck reproducing it here, though.

Hah --- I take that back.  On about the fourth or fifth trial:

==00:00:00:34.291 21525== Invalid read of size 1
==00:00:00:34.291 21525==at 0x4A08DEC: memcpy (mc_replace_strmem.c:882)
==00:00:00:34.291 21525==by 0x66FA54: DecodeXLogTuple (decode.c:899)
==00:00:00:34.291 21525==by 0x670561: LogicalDecodingProcessRecord 
(decode.c:711)
==00:00:00:34.291 21525==by 0x671BC3: pg_logical_slot_get_changes_guts 
(logicalfuncs.c:440)
==00:00:00:34.291 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
(execQual.c:2196)
==00:00:00:34.291 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
==00:00:00:34.291 21525==by 0x5C170D: ExecScan (execScan.c:82)
==00:00:00:34.291 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
==00:00:00:34.291 21525==by 0x5B8A61: standard_ExecutorRun (execMain.c:1490)
==00:00:00:34.291 21525==by 0x6BFE36: PortalRunSelect (pquery.c:942)
==00:00:00:34.291 21525==by 0x6C11EF: PortalRun (pquery.c:786)
==00:00:00:34.291 21525==by 0x6BD7E3: exec_simple_query (postgres.c:1072)
==00:00:00:34.291 21525==  Address 0xe5311d6 is 6 bytes after a block of size 
8,192 alloc'd
==00:00:00:34.291 21525==at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
==00:00:00:34.291 21525==by 0x4ED399: XLogReaderAllocate (xlogreader.c:83)
==00:00:00:34.291 21525==by 0x6710B3: StartupDecodingContext (logical.c:161)
==00:00:00:34.291 21525==by 0x671303: CreateDecodingContext (logical.c:413)
==00:00:00:34.291 21525==by 0x671AF7: pg_logical_slot_get_changes_guts 
(logicalfuncs.c:394)
==00:00:00:34.291 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
(execQual.c:2196)
==00:00:00:34.291 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
==00:00:00:34.291 21525==by 0x5C170D: ExecScan (execScan.c:82)
==00:00:00:34.291 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
==00:00:00:34.291 21525==by 0x5B8A61: standard_ExecutorRun (execMain.c:1490)
==00:00:00:34.291 21525==by 0x6BFE36: PortalRunSelect (pquery.c:942)
==00:00:00:34.291 21525==by 0x6C11EF: PortalRun (pquery.c:786)
==00:00:00:34.291 21525== 
...
...
==00:00:00:35.011 21525== Invalid read of size 1
==00:00:00:35.011 21525==at 0x4A08CCA: memcpy (mc_replace_strmem.c:882)
==00:00:00:35.011 21525==by 0x66FA54: DecodeXLogTuple (decode.c:899)
==00:00:00:35.011 21525==by 0x670561: LogicalDecodingProcessRecord 
(decode.c:711)
==00:00:00:35.011 21525==by 0x671BC3: pg_logical_slot_get_changes_guts 
(logicalfuncs.c:440)
==00:00:00:35.011 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
(execQual.c:2196)
==00:00:00:35.011 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
==00:00:00:35.011 21525==by 0x5C170D: ExecScan (execScan.c:82)
==00:00:00:35.011 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
==00:00:00:35.012 21525==by 0x5B8A61: standard_ExecutorRun (execMain.c:1490)
==00:00:00:35.012 21525==by 0x6BFE36: PortalRunSelect (pquery.c:942)
==00:00:00:35.012 21525==by 0x6C11EF: PortalRun (pquery.c:786)
==00:00:00:35.012 21525==by 0x6BD7E3: exec_simple_query (postgres.c:1072)
==00:00:00:35.012 21525==  Address 0x4ff2450 is 0 bytes after a block of size 
8,192 alloc'd
==00:00:00:35.012 21525==at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
==00:00:00:35.012 21525==by 0x4ED399: XLogReaderAllocate (xlogreader.c:83)
==00:00:00:35.012 21525==by 0x6710B3: StartupDecodingContext (logical.c:161)
==00:00:00:35.012 21525==by 0x671303: CreateDecodingContext (logical.c:413)
==00:00:00:35.012 21525==by 0x671AF7: pg_logical_slot_get_changes_guts 
(logicalfuncs.c:394)
==00:00:00:35.012 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
(execQual.c:2196)
==00:00:00:35.012 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
==00:00:00:35.012 21525==by 0x5C170D: ExecScan (execScan.c:82)
==00:00:00:35.012 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
==00:00:00:35.012 21525==by 0x5B8A61: standard_ExecutorRun (execMain.c:1490)
==00:00:00:35.012 21525==by 0x6BFE36: PortalRunSelect (pquery.c:942)
==00:00:00:35.012 21525==by 0x6C11EF: PortalRun (pquery.c:786)
==00:00:00:35.012 21525== 
==00:00:00:35.012 21525== Invalid read of size 1
==00:00:00:35.012 21525==at 0x4A08CB8: memcpy (mc_replace_strmem.c:882)
==00:00:00:35.012 21525==by 0x66FA54: DecodeXLogTuple (decode.c:899)
==00:00:00:35.012 21525==by 0x670561: LogicalDecodingProcessRecord 
(decode.c:711)
==00:00:00:35.012 21525==by 0x671BC3: pg_logical_slot_get_changes_guts 
(logicalfuncs.c:440)
==00:00:00:35.012 21525==by 0x5C0B6B: ExecMakeTableFunctionResult 
(execQual.c:2196)
==00:00:00:35.012 21525==by 0x5D4131: FunctionNext (nodeFunctionscan.c:95)
==00:00:00:35.012 21525==by 0x5C170D: ExecScan (execScan.c:82)
==00:00:00:35.012 21525==by 0x5BA007: ExecProcNode (execProcnode.c:426)
==00:00:00:35.012 21525==by 0x5B8A61: standard_Execu

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Andres Freund
On 2016-07-20 11:26:11 -0400, Robert Haas wrote:
> On Wed, Jul 20, 2016 at 3:39 AM, Andres Freund  wrote:
> >>I think Snapshot's members whenTaken and lsn are updated/initialized
> >>only in GetSnapshotData().  So if GetSnapshotData() is not used, how
> >>will you expect those fields to be updated.  We need those fields to
> >>be updated for TestForOldSnapshot().
> >
> > That's why I suggested copying them from the current mvcc snapshot.
> 
> And how do you obtain that?  The functions that reference
> SnapshotToast are toast_delete_datum, toastrel_value_exists, and
> toast_fetch_datum, toast_fetch_datum_slice, but none of those take a
> snapshot as an argument, nor is there any reasonable way to make them
> do so.  Those are indirectly called by things like bttextcmp, which
> don't know what snapshot was used to fetch the datum that they are
> detoasting and can't reasonably be made to know.
> 
> I mean, you could do something *approximately* correct by calling
> GetActiveSnapshot() but that doesn't seem likely to be correct in
> detail.

GetActiveSnapshot() seems like it should work well enough in this case,
or we could use pairingheap_first() to get the actual oldest registered
one.


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


[HACKERS] skink's test_decoding failures in 9.4 branch

2016-07-20 Thread Tom Lane
I can't help noticing that the failure rate on skink has gone from
"rare" to "100%" since 3d5b227:
http://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=skink&br=REL9_4_STABLE
I think we need to put some effort into figuring out what's up there.

Also, this morning curculio showed what might be the same issue:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=curculio&dt=2016-07-20%2014%3A30%3A01

I've still had no luck reproducing it here, though.  Wonder if it is
specific to certain compilers.

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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Robert Haas
On Wed, Jul 20, 2016 at 3:39 AM, Andres Freund  wrote:
>>I think Snapshot's members whenTaken and lsn are updated/initialized
>>only in GetSnapshotData().  So if GetSnapshotData() is not used, how
>>will you expect those fields to be updated.  We need those fields to
>>be updated for TestForOldSnapshot().
>
> That's why I suggested copying them from the current mvcc snapshot.

And how do you obtain that?  The functions that reference
SnapshotToast are toast_delete_datum, toastrel_value_exists, and
toast_fetch_datum, toast_fetch_datum_slice, but none of those take a
snapshot as an argument, nor is there any reasonable way to make them
do so.  Those are indirectly called by things like bttextcmp, which
don't know what snapshot was used to fetch the datum that they are
detoasting and can't reasonably be made to know.

I mean, you could do something *approximately* correct by calling
GetActiveSnapshot() but that doesn't seem likely to be correct in
detail.

-- 
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] Design for In-Core Logical Replication

2016-07-20 Thread Joshua D. Drake

On 07/20/2016 01:08 AM, Simon Riggs wrote:







   Logical Replication
   
 Logical Replication is a method of replicating data objects and their
 changes, based upon their Primary Keys (or Replication Identity). We


Do we want a limitation based on Primary Key, or would it be possible to 
use just UNIQUE or is that covered under Replication Identity?



   
 Logical Replication uses a Publish and Subscribe model with one or
 more Subscribers subscribing to one or more Publications on a
 Provider node. Subscribers pull data from the Publications they
 subscribe to and may subsequently re-publish data to allow
 cascading replication or more complex configurations.


Is that somehow different than Origin/Subscriber or Master/Slave? If 
not, why are we using yet more terms?




   Publication
   
 A Publication object can be defined on any master node, owned by one
 user. A Publication is a set of changes generated from a group of
 tables, and might also be described as a Change Set or Replication Set.
 Each Publication exists in only one database.


  Then on Provider database:


CREATE PUBLICATION mypub;
ALTER PUBLICATION mypub ADD TABLE users, departments;

   


Outside of my previous comments on reusing terminology that is known to 
our community, I like this. Basically a user creates a pool that is 
replicating, throws various ducks and small children into the pool and 
then replicates. Nice.


Sincerely,

JD


--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.


--
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] Password identifiers, protocol aging and SCRAM protocol

2016-07-20 Thread Robert Haas
On Fri, Jul 15, 2016 at 9:30 AM, Michael Paquier
 wrote:
> OK, I am doing that at the end.
>
> And also while moving on...
>
> On another topic, here are some ideas to extend CREATE/ALTER ROLE to
> support SCRAM password directly:
> 1) protocol PASSWORD value, where protocol is { MD5 | PLAIN | SCRAM }, giving:
> CREATE ROLE foorole SCRAM PASSWORD value;
> 2) PASSWORD (protocol) value.
> 3) Just add SCRAM PASSWORD
> My mind is thinking about 1) as being the cleanest solution as this
> does not touch the defaults, which may change a couple of releases
> later. Other opinions?

I can't really understand what you are saying here, but I'm going to
be -1 on adding SCRAM as a parser keyword.  Let's pick a syntax like
"PASSWORD SConst USING SConst" or "PASSWORD SConst ENCRYPTED WITH
SConst".

-- 
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] Oddity in handling of cached plans for FDW queries

2016-07-20 Thread Robert Haas
On Wed, Jul 20, 2016 at 10:15 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> Mumble.  Why, exactly, was this a good idea?  The upside of commit
>> 45639a0525a58a2700cf46d4c934d6de78349dac is only that you do fewer
>> plan invalidations, but surely that's not a significant benefit for
>> most people: user mappings don't change that often.  On the downside,
>> there are now cases where joins would have gotten pushed down
>> previously and now they won't.  In other words, you've saved some
>> replanning activity at the cost of delivering worse plans.  That seems
>> pretty suspect to me, although I grant that the scenarios where either
>> the old or the new behavior is actually a problem are all somewhat off
>> the beaten path.
>
> I think that you are undervaluing the removal of user-mapping-based plan
> invalidation.  That was never more than a kluge, and here is the reason:
> we have no way to lock user mappings.  The system whereby we invalidate
> plans as a consequence of table DDL changes is bulletproof, because we
> (re) acquire locks on the tables used in the plan, then check for
> invalidation signals, before deciding whether the plan is stale.  The
> corresponding scenario where a user mapping changes between that check
> and execution time is unprotected, so that we could end up using a plan
> that is insecure for the mappings selected at execution.

OK, that's a fair point.  Thanks for explaining.

> Another way we could have removed the race condition is the suggestion
> made upthread of embedding the user mapping details right into the plan
> instead of looking them up afresh at execution.  But I didn't much like
> that approach, per upthread discussion.

OK.

-- 
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] dumping database privileges broken in 9.6

2016-07-20 Thread Stephen Frost
* Noah Misch (n...@leadboat.com) wrote:
> On Fri, Jul 15, 2016 at 03:46:17PM -0400, Stephen Frost wrote:
> > * Noah Misch (n...@leadboat.com) wrote:
> > > On Sat, Jul 09, 2016 at 12:55:33AM -0400, Stephen Frost wrote:
> > > > * Noah Misch (n...@leadboat.com) wrote:
> > > > > This PostgreSQL 9.6 open item is past due for your status update.  
> > > > > Kindly send
> > > > > a status update within 24 hours, and include a date for your 
> > > > > subsequent status
> > > > > update.  Refer to the policy on open item ownership:
> > > > > http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com
> > > > 
> > > > Unfortunately, not going to make any further progress on this tonight or
> > > > over the weekend as I'm going to be out of town.  I believe I've
> > > > convinced myself that adding a template1 entry to pg_init_privs will be
> > > > both sufficient and produce the correct results, along with adjusting
> > > > the query in pg_dumpall to join through it.  Will provide an update on
> > > > Monday.
> > > 
> > > This PostgreSQL 9.6 open item is long past due for your status update.  
> > > Kindly
> > > send a status update within 24 hours, and include a date for your 
> > > subsequent
> > > status update.  (Your Tuesday posting lacked a date.)
> > 
> > Yeah, I realized that tablespaces have the same issue and have updated
> > the patch to address them as well, in the same way.
> > 
> > Going through and doing testing now.  Unfortunately, it doesn't look
> > like adding in testing of tablespaces into the TAP tests would be very
> > easy (the only TAP test that deals with tablespaces that I found was
> > pg_basebackup and that looks rather grotty..), so I'm not planning to do
> > that, at least not at this time.
> > 
> > As such, I'm planning to commit the patch with the fix+regression test
> > for database ACLs and the fix for tablespace ACLs either later today
> > or tomorrow.
> 
> Does commit 47f5bb9 fully fix this open item?

Yes, I've updated the open items wiki.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Oddity in handling of cached plans for FDW queries

2016-07-20 Thread Tom Lane
Robert Haas  writes:
> Mumble.  Why, exactly, was this a good idea?  The upside of commit
> 45639a0525a58a2700cf46d4c934d6de78349dac is only that you do fewer
> plan invalidations, but surely that's not a significant benefit for
> most people: user mappings don't change that often.  On the downside,
> there are now cases where joins would have gotten pushed down
> previously and now they won't.  In other words, you've saved some
> replanning activity at the cost of delivering worse plans.  That seems
> pretty suspect to me, although I grant that the scenarios where either
> the old or the new behavior is actually a problem are all somewhat off
> the beaten path.

I think that you are undervaluing the removal of user-mapping-based plan
invalidation.  That was never more than a kluge, and here is the reason:
we have no way to lock user mappings.  The system whereby we invalidate
plans as a consequence of table DDL changes is bulletproof, because we
(re) acquire locks on the tables used in the plan, then check for
invalidation signals, before deciding whether the plan is stale.  The
corresponding scenario where a user mapping changes between that check
and execution time is unprotected, so that we could end up using a plan
that is insecure for the mappings selected at execution.

Another way we could have removed the race condition is the suggestion
made upthread of embedding the user mapping details right into the plan
instead of looking them up afresh at execution.  But I didn't much like
that approach, per upthread discussion.

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] dumping database privileges broken in 9.6

2016-07-20 Thread Stephen Frost
* Michael Paquier (michael.paqu...@gmail.com) wrote:
> On Sat, Jul 16, 2016 at 4:46 AM, Stephen Frost  wrote:
> > Going through and doing testing now.  Unfortunately, it doesn't look
> > like adding in testing of tablespaces into the TAP tests would be very
> > easy (the only TAP test that deals with tablespaces that I found was
> > pg_basebackup and that looks rather grotty..), so I'm not planning to do
> > that, at least not at this time.
> 
> The cleanest way to handle that in PostgresNode would be to have a
> dedicated routine calling psql -c 'create tablespace' with tablespaces
> located in a folder $basedir/tbspace. And on top of that there should
> be the tablespace metadata saved in the context of the test, with a
> pair of (tbspc name, location). But I think that we'd need first
> stronger arguments (take more use cases) to introduce such an
> extension of the test APIs.

The way pg_basebackup handles this is to use TestLib::tempdir_short and
create a symlink with it, then to call psql to create the tablespace.  I
don't have any problem using $basedir/tbspace instead though.

What I was thinking is that we'd add a 'create_tablespace' or similar
routine to PostgresNode and then make the pg_basebackup and pg_dump
tests use it.  Otherwise, I suspect the next person who ends up writing
a 'create tablespace' into the TAP tests will use yet another location
and/or method.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] (re)start in our init scripts seems broken

2016-07-20 Thread Tom Lane
Michael Paquier  writes:
> On Wed, Jul 20, 2016 at 11:41 AM, Tomas Vondra
>  wrote:
>> Is there a reason why it's coded like this? I think we should use the pg_ctl
>> instead or (at the very least) check the postmaster return code. Also,
>> perhaps we should add an explicit timeout, higher than 60 seconds.

> c8196c87 is one reason.

I think that 8f5500e6b improved that situation.  You still have to be
really careful when writing the init script that there not be more than
one postgres-owned shell process.

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] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Kevin Grittner
On Tue, Jul 19, 2016 at 6:32 PM, Andres Freund  wrote:

> I mean the only difference between toast / plain heap table WRT
> old_snapshot_threshold is that we don't use a mvcc snapshot.

We use different functions and never, ever call BufferGetPage --
except for deep in the bowels of the AMs.  Countless functions
would need to be modified to pass in information about whether any
call is one of those which need to test for snapshot-too-old.
Since "normal" heap and index access is already covered without
that, yet use the AMs, there would be a weird "double coverage" to
look out for.  On top of all that, you would need to not only throw
errors for some cases but (as you pointed out earlier in the
thread) turn others into no-ops.  Also, some of the toast calls are
very far from the calls for the base row, where a function might
decide to de-toast some toast pointer.  With the naive approach of
what you suggest, frequency of checking would go from once per page
(containing multiple tuples) to that *plus* once per toast chunk
per value per heap tuple, although it seems like checking any one
(like the first) toast chunk for a value would suffice.

--
Kevin Grittner
EDB: 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] sslmode=require fallback

2016-07-20 Thread Daniel Verite
Magnus Hagander wrote:

> > I don't understand why you want to change the default.  Is it for
> > performance?  Has it been measured?
> >
> >
> Yes. I've run into it multiple times, but I haven't specifically measured
> it. But I've had more than one situation where turning it off has
> completely removed a performance problem.

Here's a test case retrieving 133000 rows representing
100Mbytes of text, that shows a 4x slowdown with ssl.
ssl_renegotiation_limit is set to 0 and the cache is warmed up
by repeated executions.

Without SSL:

$ time psql -At "postgresql://localhost/mlists?sslmode=disable"\
  -c "select subject from mail" -o /dev/null
real0m1.359s
user0m0.544s
sys 0m0.084s

With SSL:
$ time psql -At "postgresql://localhost/mlists?sslmode=require"\ 
  -c "select subject from mail" -o /dev/null
real0m5.395s
user0m1.080s
sys 0m0.116s

The CPU is Intel(R) Xeon(R) CPU E31230 @ 3.20GHz, OS is Debian7
with kernel 3.2.0-4.

Personally I think that TLS for local networking is wrong as a default, and
it's unfortunate that distros like Debian/Ubuntu end up using that.

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] Adjust recovery test file name

2016-07-20 Thread Masahiko Sawada
On Wed, Jul 20, 2016 at 5:08 AM, Alvaro Herrera
 wrote:
> Masahiko Sawada wrote:
>> Hi all,
>>
>> The file 006_logical_decoding_timelines.pl was removed by the commit c1543a8.
>> But currently 005_***.pl and 007_***.pl exist on source tree.
>> Should we change its file number to 006?
>
> I don't think we need to bother about this.  Whenever somebody submits a
> new test script they can fill the 006 gap.
>

I understood.
Thank you for the reply!

Regards,

--
Masahiko Sawada


-- 
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] Oddity in handling of cached plans for FDW queries

2016-07-20 Thread Robert Haas
On Fri, Jul 15, 2016 at 5:25 PM, Tom Lane  wrote:
> I wrote:
>> Etsuro Fujita  writes:
>>> Here is a patch for that redesign proposed by you; reverts commits
>>> fbe5a3fb73102c2cfec114a67943f4474383 and
>>> 5d4171d1c70edfe3e9be1de9e66603af28e3afe1, adds changes for that redesign
>>> to the core, and adjusts the postgres_fdw code to that changes.  Also, I
>>> rearranged the postgres_fdw regression tests to match that changes.
>
>> OK, I'll review this later today.
>
> Pushed, after fooling around with it some more so that it would cover the
> case we discussed where the join could be allowed if we restrict the plan
> to be executed by the owner of a view used in the query.

Mumble.  Why, exactly, was this a good idea?  The upside of commit
45639a0525a58a2700cf46d4c934d6de78349dac is only that you do fewer
plan invalidations, but surely that's not a significant benefit for
most people: user mappings don't change that often.  On the downside,
there are now cases where joins would have gotten pushed down
previously and now they won't.  In other words, you've saved some
replanning activity at the cost of delivering worse plans.  That seems
pretty suspect to me, although I grant that the scenarios where either
the old or the new behavior is actually a problem are all somewhat off
the beaten path.

-- 
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] [BUG] pg_basebackup from disconnected standby fails

2016-07-20 Thread Michael Paquier
On Wed, Jul 20, 2016 at 8:40 PM, Amit Kapila  wrote:
> On Wed, Jul 20, 2016 at 5:12 AM, Michael Paquier
>  wrote:
>> On Tue, Jul 19, 2016 at 9:08 PM, Amit Kapila  wrote:
>>> On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier
>>>  wrote:
 On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila  
 wrote:
> On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier
>  wrote:
>>> Why only for back-branches? Do you have better solution for head?
>>
>> Yes, I mentioned an idea upthread to set up the minimum recovery point
>> saved in the backup to the last replayed LSN. Though that's not
>> acceptable for 9.6 as this requires changing the output of
>> pg_stop_backup() with a new field containing the bytes of pg_control.
>> I am not sure how others feel about that,
>>
>
> Yeah, I think that is totally different angle to fix this issue, so
> don't you think it is better to start a separate thread to discuss
> about it for 10.0 and mark this patch as ready for committer.

I'd like to tackle this problem in 10.0, but that will strongly depend
on how my patches move on in CF1 and CF2.
-- 
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] [BUG] pg_basebackup from disconnected standby fails

2016-07-20 Thread Amit Kapila
On Wed, Jul 20, 2016 at 5:12 AM, Michael Paquier
 wrote:
> On Tue, Jul 19, 2016 at 9:08 PM, Amit Kapila  wrote:
>> On Tue, Jul 19, 2016 at 10:43 AM, Michael Paquier
>>  wrote:
>>> On Sat, Jul 16, 2016 at 9:20 PM, Amit Kapila  
>>> wrote:
 On Wed, Jul 13, 2016 at 8:56 AM, Michael Paquier
  wrote:
>> Why only for back-branches? Do you have better solution for head?
>
> Yes, I mentioned an idea upthread to set up the minimum recovery point
> saved in the backup to the last replayed LSN. Though that's not
> acceptable for 9.6 as this requires changing the output of
> pg_stop_backup() with a new field containing the bytes of pg_control.
> I am not sure how others feel about that,
>

Yeah, I think that is totally different angle to fix this issue, so
don't you think it is better to start a separate thread to discuss
about it for 10.0 and mark this patch as ready for committer.

>
>>> It is an
>>> annoyance to not be able to ensure that backups are taken while the
>>> master is stopped or if there is no activity that updates relation
>>> pages.
>>>
>>> The thing that is really annoying btw is that there will be always a
>>> gap between minRecoveryPoint and the actual moment where a backup
>>> finishes because there is no way to rely on the XLOG_BACKUP_END
>>> record.
>>
>> Sorry, but I am not able to understand what you mean by above.  What
>> kind of relation you are trying to show between minRecoveryPoint and
>> backup finish point?
>
> When taking a backup from a standby, there is no XLOG_BACKUP_END that
> can be used to determine when a hot standby is open for read-only
> connections.
>

Okay, may be we can add a comment in code to indicate the same, but
OTOH, it is apparent from code, so not sure if it is worth to add it.


-- 
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] sslmode=require fallback

2016-07-20 Thread Magnus Hagander
On Tue, Jul 19, 2016 at 10:57 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 7/19/16 3:32 PM, Magnus Hagander wrote:
> > There are definitely cases where it's useful. I'm only arguing for
> > changing the default.
>
> I don't understand why you want to change the default.  Is it for
> performance?  Has it been measured?
>
>
Yes. I've run into it multiple times, but I haven't specifically measured
it. But I've had more than one situation where turning it off has
completely removed a performance problem.

I've only seen it in apps without proper connection pooling. It's the
negotiation of new sessions that's expensive, not actually encrypting the
data.

Most people definitely don't run into it, because most people don't use
localhost when they're local - they use the Unix socket. But for example a
locally running java application will be using localhost.

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


Re: [HACKERS] sslmode=require fallback

2016-07-20 Thread Greg Stark
Iirc we changed the default to be SSL for localhost to address a particular
problem. It seemed surprising at the time but it was the most effective
solution.
On 19 Jul 2016 21:58, "Peter Eisentraut" 
wrote:

> On 7/19/16 3:32 PM, Magnus Hagander wrote:
> > There are definitely cases where it's useful. I'm only arguing for
> > changing the default.
>
> I don't understand why you want to change the default.  Is it for
> performance?  Has it been measured?
>
> --
> 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] Re: FDW handling count(*) through AnalyzeForeignTable or other constant time push-down

2016-07-20 Thread anantbhasu
Hi Gabe, 
Did you get Aggregate Pushdown FDW plugin?
Would be really helpful if you can share some insight on your investigation.
Regards
Anant



--
View this message in context: 
http://postgresql.nabble.com/FDW-handling-count-through-AnalyzeForeignTable-or-other-constant-time-push-down-tp5889291p5912699.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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


[HACKERS] Design for In-Core Logical Replication

2016-07-20 Thread Simon Riggs
At PgCon we discussed that Petr Jelinek would be working on the code for an
in-core logical replication implementation, while I would work on user
interface/security models. Petr has been actively working on the code and
will post patch in a few weeks, as discussed and agreed. Craig Ringer is
also active in coding necessary aspects. None of those things are discussed
further here at this time.

In this post, Petr and I present a joint view on a design for how this
should work in-core, based upon our implementation experiences with
physical replication, pglogical and various comments so far.

Note that this has substantial user-visible differences from pglogical,
though much of the underlying architecture is reused.

I should stress that not all of the aspects are implemented yet. The post
here today is a combination of all of our attempts to bring architecture,
usability and security into one place, including a coherent way of
describing the features and how they work.

Your comments and questions are sought now as we begin the main development
effort to get this into PostgreSQL 10.0






  Logical Replication
  
Logical Replication is a method of replicating data objects and their
changes, based upon their Primary Keys (or Replication Identity). We
use the term Logical in contrast to Physical replication which
uses exact block addresses and byte-by-byte replication.
PostgreSQL supports both mechanisms concurrently, see
. Logical Replication allows
fine-grained control over both data replication and security.
  
  
Logical Replication uses a Publish and Subscribe model with one or
more Subscribers subscribing to one or more Publications on a
Provider node. Subscribers pull data from the Publications they
subscribe to and may subsequently re-publish data to allow
cascading replication or more complex configurations.
  
  
Data for committed transactions is streamed in real-time to each
Subscriber.  Logical replication might also be described as Change
Data Capture (CDC) or Transactional Replication.
  
  
The typical use-cases for logical replication are:
  
  

  
Replicating between different major versions of the PostgreSQL
  


  
Replicating a database in full to another master node.
  


  
Replicating a subset of a database to another master node.
  


  
Firing triggers for individual changes as they are incoming to
subscriber.
  


  
Gathering data from multiple databases into a single one (for
example
for analytical purposes).
  

  


  Publication
  
A Publication object can be defined on any master node, owned by one
user. A Publication is a set of changes generated from a group of
tables, and might also be described as a Change Set or Replication Set.
Each Publication exists in only one database.
  
  
Publications are different from table schema and do not affect
how the table is accessed. Each table can be added to multiple
Publications if needed.  Publications may include both tables
and materialized views. Objects must be added explicitly, except
when a Publication is created for "ALL TABLES". There is no
default name for a Publication which specifies all tables.
  
  
Tables added to a Publication must be accessible via SELECT
privilege for the user owning the Publication. Usage on the
Publication can be GRANTed to other users.
  
  
Publications can choose to limit the changes they show using any
combination of INSERT, UPDATE, DELETE and TRUNCATE in a similar
way to the way triggers are fired by particular event types.
  
  
When UPDATEs and DELETEs are replicated by a Publication, all tables
added must have a unique index present on the REPLICA IDENTITY for
the table, or the addition will be refused.
  
  
The definition of a Publication object will be included within
pg_dump by default when all of the objects in the Publication are
requested as part of the dump specification.
  
  
Every Publication can have zero, one or more Subscribers.
  
  
Publications are created using the 
command and may be later altered or dropped using corresponding
commands.
  
  
The individual tables can be added and removed dynamically using
. Both the ADD TABLE and DROP
TABLE operations are transactional so the table will start or stop
replicating at the correct snapshot once the transaction has committed.
  


  Subscription
  
A Subscription is the downstream side of the Logical Replication. The
node where Subscription is defined is referred to as Subscriber.
Subscription defines the connection to another database and set of
Publications (one or more) to which it wants to be subscribed.
It is possible to have a Subscription that currently has no
Publications.
  
  
 

Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold

2016-07-20 Thread Andres Freund


On July 19, 2016 7:43:05 PM PDT, Amit Kapila  wrote:
>On Wed, Jul 20, 2016 at 7:57 AM, Andres Freund 
>wrote:
>>
>>
>> On July 19, 2016 7:14:42 PM PDT, Amit Kapila
> wrote:
>>>On Wed, Jul 20, 2016 at 5:02 AM, Andres Freund 
>>>wrote:
 On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote:
> As far as I can see, to do this the way that Andres and Amit
> suggest involves tying in to indexam.c and other code in
>incredibly
> ugly ways.

 Could you explain the problem you're seing?

 Isn't pretty much all all that we need to do:
 1) add a InitSnapshotToast(Snapshot originMVCCSnap), which sets
>>>SnapshotData->lsn
to the the origin snapshot's lsn
 2) adapt TestForOldSnapshot() to accept both HeapTupleSatisfiesMVCC
>>>and
HeapTupleSatisfiesToast?

>>>
>>>I also think so.  However, it is not clear what is the best place to
>>>initialize toast snapshot.  One idea could be to do it in
>>>GetSnapshotData() after capturing the required information for the
>>>valid value of old_snapshot_threshold.  Do you have something else in
>>>mind?
>>
>> There's very few callsites using toast snapshots. I'd just do it
>there. Don't think we ever use GetSnapshotData for them.
>>
>
>I think Snapshot's members whenTaken and lsn are updated/initialized
>only in GetSnapshotData().  So if GetSnapshotData() is not used, how
>will you expect those fields to be updated.  We need those fields to
>be updated for TestForOldSnapshot().

That's why I suggested copying them from the current mvcc snapshot.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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