Re: [HACKERS] SCRAM authentication, take three

2017-04-09 Thread Craig Ringer
On 10 April 2017 at 13:57, Craig Ringer  wrote:
> On 10 April 2017 at 12:34, Michael Paquier  wrote:
>
>> Attached is a patch to hopefully make the discussion progress. I
>> simply propose to use sasl as a keyword for pg_hba.conf, on the basis
>> that SASL is the protocol used, and scram is a mechanism used to
>> achieve the SASL exchange. We can always come up with a set of options
>> and aliases later, I am actually open to have more fancy extra options
>> in pg_hba.conf.
>
> I'd really like to see this approach proceed.
>
> pg_hba.conf isn't the most user-friendly thing in the world, and seems
> to be one of the top sources of confusion for new users. Simple is
> good here IMO.
>
> Let users specify 'scram' and negotiate.

sasl, rather.



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


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


Re: [HACKERS] SCRAM authentication, take three

2017-04-09 Thread Craig Ringer
On 10 April 2017 at 12:34, Michael Paquier  wrote:

> Attached is a patch to hopefully make the discussion progress. I
> simply propose to use sasl as a keyword for pg_hba.conf, on the basis
> that SASL is the protocol used, and scram is a mechanism used to
> achieve the SASL exchange. We can always come up with a set of options
> and aliases later, I am actually open to have more fancy extra options
> in pg_hba.conf.

I'd really like to see this approach proceed.

pg_hba.conf isn't the most user-friendly thing in the world, and seems
to be one of the top sources of confusion for new users. Simple is
good here IMO.

Let users specify 'scram' and negotiate.

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


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


Re: [HACKERS] scram and \password

2017-04-09 Thread Michael Paquier
On Mon, Apr 10, 2017 at 12:53 PM, Noah Misch  wrote:
> On Wed, Apr 05, 2017 at 08:11:25PM +0300, Heikki Linnakangas wrote:
>> >Heikki, are you going to do something about these?  We're running out of 
>> >time.
>>
>> Sorry I've been procrastinating. I'm on it now. (We need to do something
>> about this, feature freeze or not..)

As there have been some conflicts because of the commit of SASLprep,
here is a rebased set of patches. A couple of things worth noting:
- SASLprep does an allocation of the prepared password string. It is
definitely better to do all the ground work in pg_saslprep but this
costs a free() call with a #ifdef FRONTEND at the end of
scram_build_verifier().
- Patch 0005 does that:
+   /*
+* Hash password using SCRAM-SHA-256 when connecting to servers
+* newer than Postgres 10, and hash with MD5 otherwise.
+*/
+   if (pset.sversion < 10)
+   encrypted_password = PQencryptPassword(pw1, user, "md5");
+   else
+   encrypted_password = PQencryptPassword(pw1, user, "scram");
Actually I am thinking that guessing the hashing function according to
the value of password_encryption would make the most sense. Thoughts?
-- 
Michael
VMware vCenter server
www.vmware.com


0001-Use-base64-based-encoding-for-stored-and-server-keys.patch
Description: Binary data


0002-Move-routine-to-build-SCRAM-verifier-into-src-common.patch
Description: Binary data


0003-Refactor-frontend-side-random-number-generation.patch
Description: Binary data


0004-Extend-PQencryptPassword-with-a-hashing-method.patch
Description: Binary data


0005-Extend-psql-s-password-and-createuser-to-handle-SCRA.patch
Description: Binary data

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


[HACKERS] max_sync_workers_per_subscription is missing in postgresql.conf

2017-04-09 Thread Masahiko Sawada
Hi all,

Attached a patch for $subject.

I added this parameter into "Asynchronous Behavior" section of
"RESOURCE" section. But GUC parameter for subscriber now is written in
this section, in spite of there is "REPLICATION" section. I think that
we can coordinate these parameters to not confuse user. For example in
documentation, these parameters are described in "19.6.4. Subscribers"
section of "19.6. Replication" section. Thought?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Add_max_sync_workers_per_subscription_into_postgresql_conf.patch
Description: Binary data

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


Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-09 Thread Andrew Gierth
> "Thomas" == Thomas Munro  writes:

 >> SomeType *x = (SomeType *) lfirst(l);
 >> 
 >> (in my code I tend to omit the (SomeType *), which I dislike because
 >> it adds no real protection)

 Thomas> Just BTW, without that cast it's not compilable as C++, so I'm
 Thomas> guessing that Peter E will finish up putting it back in
 Thomas> wherever you leave it out...

There's north of 150 other examples (just grep for '= lfirst' in the
source). Some were even committed by Peter E :-)

In the discussion with Andres the same point came up for palloc, for
which I suggested we add something along the lines of:

#define palloc_object(_type_) (_type_ *) palloc(sizeof(_type_))
#define palloc_array(_type_, n) (_type_ *) palloc((n) * sizeof(_type_))

palloc() without a cast is even more common than lfirst() without one,
and something like half of those (and 80%+ of the pallocs that do have a
cast) are palloc(sizeof(...)) or palloc(something * sizeof(...)).

-- 
Andrew (irc:RhodiumToad)


-- 
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] Malformed Array Literal in PL/pgSQL Exception Block

2017-04-09 Thread David E. Wheeler
On Apr 9, 2017, at 9:59 PM, Andrew Gierth  wrote:

> Tom's response has the explanation of why it fails (everywhere, not just
> in the exception block): parse analysis prefers to match the (array ||
> array) form of the operator when given input of (array || unknown). Just
> cast the 'foo' to the array element type.

Tried to reduce this from some code I’m working on. I have a whole bunch of 
code that appends to an array in this way without casting ‘foo’ to text or 
text[]. It’s only in an exception block that it’s complaining.

Hrm, looking back through my code, it looks like I’m mostly calling format() to 
append to an array, which of course returns a ::text, so no ambiguity. Guess 
that’s my issue.

Thanks,

David



smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] Malformed Array Literal in PL/pgSQL Exception Block

2017-04-09 Thread Andrew Gierth
> "David" == David E Wheeler  writes:

 >> If you change this to EXCEPTION WHEN division_by_zero THEN, the
 >> reported error becomes:
 >> 
 >> ERROR:  malformed array literal: "foo"
 >> LINE 1: SELECT things || 'foo'

 David> So the issue stands, yes?

Tom's response has the explanation of why it fails (everywhere, not just
in the exception block): parse analysis prefers to match the (array ||
array) form of the operator when given input of (array || unknown). Just
cast the 'foo' to the array element type.

-- 
Andrew (irc:RhodiumToad)


-- 
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] Malformed Array Literal in PL/pgSQL Exception Block

2017-04-09 Thread David E. Wheeler
On Apr 9, 2017, at 9:52 PM, Andrew Gierth  wrote:

> This "raise" statement is not reached, because the previous line raises
> the "malformed array literal" error.

Bah!

> David> EXCEPTION WHEN OTHERS THEN
> 
> If you change this to  EXCEPTION WHEN division_by_zero THEN, the
> reported error becomes:
> 
> ERROR:  malformed array literal: "foo"
> LINE 1: SELECT things || 'foo'

So the issue stands, yes?

D



smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] Malformed Array Literal in PL/pgSQL Exception Block

2017-04-09 Thread Andrew Gierth
> "David" == David E Wheeler  writes:

 David> And it works great, including in PL/pgSQL functions, except in
 David> an exception block. When I run this:

 David> BEGIN;

 David> CREATE OR REPLACE FUNCTION foo(
 David> ) RETURNS BOOLEAN IMMUTABLE LANGUAGE PLPGSQL AS $$
 David> DECLARE
 David> things TEXT[] := '{}';
 David> BEGIN
 David> things := things || 'foo';
 David> RAISE division_by_zero;

This "raise" statement is not reached, because the previous line raises
the "malformed array literal" error.

 David> EXCEPTION WHEN OTHERS THEN

If you change this to  EXCEPTION WHEN division_by_zero THEN, the
reported error becomes:

ERROR:  malformed array literal: "foo"
LINE 1: SELECT things || 'foo'

-- 
Andrew (irc:RhodiumToad)


-- 
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] Malformed Array Literal in PL/pgSQL Exception Block

2017-04-09 Thread Tom Lane
"David E. Wheeler"  writes:
> I’ve been happily using the array-to-element concatenation operator || to 
> append a single value to an array, e.g, 
> SELECT array || 'foo';
> And it works great, including in PL/pgSQL functions, except in an
> exception block.

Hm, really?

regression=# create table zit (things text[]);
CREATE TABLE
regression=# insert into zit values(array['foo','bar']);
INSERT 0 1
regression=# select things || 'baz' from zit;
ERROR:  malformed array literal: "baz"
LINE 1: select things || 'baz' from zit;
 ^
DETAIL:  Array value must start with "{" or dimension information.

I think the problem here is that without any other info about the
type of the right-hand argument of the || operator, the parser will
assume that it's the same type as the left-hand argument; which
is not unreasonable, because there is an array || array operator.

If you are more specific about the type of the RHS then it's fine:

regression=# select things || 'baz'::text from zit;
   ?column?
---
 {foo,bar,baz}
(1 row)

> Note that it’s fine with the use of || outside the exception block, but
> not inside!

Don't see why an exception block would have anything to do with it.

regards, tom lane


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


Re: [HACKERS] [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output

2017-04-09 Thread Michael Paquier
On Mon, Apr 10, 2017 at 1:01 PM, Noah Misch  wrote:
> This PostgreSQL 10 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:
> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

Note for reviewers and committers: the patch sent upthread in
https://www.postgresql.org/message-id/CAB7nPqTUOpF792rDOnBkswZ=zghwxdb01oqu2taf1ku4iuu...@mail.gmail.com
still applies and passes all the tests.
-- 
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] SCRAM authentication, take three

2017-04-09 Thread Michael Paquier
On Sat, Apr 8, 2017 at 9:28 AM, Robert Haas  wrote:
> On Fri, Apr 7, 2017 at 6:32 PM, Michael Paquier
>  wrote:
>> On Sat, Apr 8, 2017 at 1:59 AM, Robert Haas  wrote:
>>> On Fri, Apr 7, 2017 at 3:59 AM, Heikki Linnakangas  wrote:
 I think the "SCRAM" part is more important than "SHA-256", so -1 on that.
>>>
>>> I agree.  The point here isn't that we're using a better hashing
>>> method, even if a lot of people *think* that's the point.  The point
>>> is we're using a modern algorithm that has nice properties like "you
>>> can't impersonate the client by steeling the verifier, or even by
>>> snooping the exchange".
>>>
>>> But "sasl" might be even better.
>>
>> FWIW, my opinion has not changed much on the matter, I would still
>> favor "sasl" as the keyword used in pg_hba.conf. What has changed in
>> my mind though is that defining no mechanisms with an additional
>> option mean that all possible choices are sent to the client. But if
>> you define a list of mechanisms, then we'll just send back to the
>> client the specified list as a possible choice of exchange mechanism:
>> host all all blah.com sasl mechanism=scram-sha-256-plus
>> Here for example the user would not be allowed to use SCRAM-SHA-256,
>> just SCRAM with channel binding.
>>
>> Such an option makes sense once we add support for one more mechanism
>> in SASL, like channel binding, but that's by far a generic approach
>> that can serve us for years to come, and by admitting that nothing
>> listed means all possible options we don't need any immediate action.
>
> Yes, that all seems quite sensible.  What exactly is the counter-argument?

I am unsure what that argument is either by reading the thread again.

Attached is a patch to hopefully make the discussion progress. I
simply propose to use sasl as a keyword for pg_hba.conf, on the basis
that SASL is the protocol used, and scram is a mechanism used to
achieve the SASL exchange. We can always come up with a set of options
and aliases later, I am actually open to have more fancy extra options
in pg_hba.conf.

Here is my original proposal:
sasl mechanism=scram-sha-256-plus,scram-sha-256

But we could have something like that as well:
sasl mechanism=scram-plus,scram
sasl mechanism=scram channel_binding=on/off

A problem with the last one is that it is not possible to control
channel binding per mechanism, but that could be discussed later on
once support for channel binding is added. For now I would just favor
the most extensive approach. No need to work later on with some
aliases in pg_hba.conf either.
-- 
Michael


reshape-sasl-hba.patch
Description: Binary data

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


[HACKERS] Malformed Array Literal in PL/pgSQL Exception Block

2017-04-09 Thread David E. Wheeler
Hackers,

I’ve been happily using the array-to-element concatenation operator || to 
append a single value to an array, e.g, 

SELECT array || 'foo';

And it works great, including in PL/pgSQL functions, except in an exception 
block. When I run this:

BEGIN;

CREATE OR REPLACE FUNCTION foo(
) RETURNS BOOLEAN IMMUTABLE LANGUAGE PLPGSQL AS $$
DECLARE
things TEXT[] := '{}';
BEGIN
things := things || 'foo';
RAISE division_by_zero;
EXCEPTION WHEN OTHERS THEN
things := things || 'bar';
END;
$$;

SELECT foo();

ROLLBACK;

The output is:

psql:array.sql:15: ERROR:  malformed array literal: "bar"
LINE 1: SELECT things || 'bar'
 ^
DETAIL:  Array value must start with "{" or dimension information.
QUERY:  SELECT things || 'bar'
CONTEXT:  PL/pgSQL function foo() line 8 at assignment

Note that it’s fine with the use of || outside the exception block, but not 
inside! I’ve worked around this by using `things || '{bar}'` instead, but it 
seems like a bug or perhaps unforeseen corner case that appending a value to an 
array doesn’t work in an exception-handling block.

Best,

David



smime.p7s
Description: S/MIME cryptographic signature


Re: [HACKERS] recent deadlock regression test failures

2017-04-09 Thread Tom Lane
Thomas Munro  writes:
> Here's a pair of draft patches for review:

I'll look at these in detail tomorrow, but:

> 2.  pg-safe-snapshot-blocking-pids.patch, to provide an end-user
> function wrapping GetSafeSnapshotBlockingPids().  Kevin expressed an
> interest in that functionality, and it does seem useful: for example,
> someone might use it to investigate which backends are holding up
> pg_dump --serializable-deferrrable.  This is a separate patch because
> you might consider it material for the next cycle, though at least
> it's handy for verifying that GetSafeSnapshotBlockingPids() is working
> correctly.

Personally I have no problem with adding this now, even though it
could be seen as a new feature.  Does anyone want to lobby against?

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] recent deadlock regression test failures

2017-04-09 Thread Thomas Munro
On Sun, Apr 9, 2017 at 11:49 PM, Thomas Munro
 wrote:
> On Sun, Apr 9, 2017 at 12:33 PM, Tom Lane  wrote:
>> Kevin Grittner  writes:
>>> On Sat, Apr 8, 2017 at 12:56 PM, Tom Lane  wrote:
 I'm imagining an API like
  isolation_test_is_waiting_for(int, int[]) returns bool
>>
>>> Good suggestion.
>>
>>> Thomas, would you like to produce a patch along these lines, or
>>> should I?
>>
>> I'll be happy to review/push if someone else does a first draft.
>
> Ok, I'll post a new patch tomorrow.

Here's a pair of draft patches for review:

1.  pg-isolation-test-session-is-blocked.patch, to fix the
CACHE_CLOBBERED_ALWAYS problem by doing all the work in C as
suggested.

2.  pg-safe-snapshot-blocking-pids.patch, to provide an end-user
function wrapping GetSafeSnapshotBlockingPids().  Kevin expressed an
interest in that functionality, and it does seem useful: for example,
someone might use it to investigate which backends are holding up
pg_dump --serializable-deferrrable.  This is a separate patch because
you might consider it material for the next cycle, though at least
it's handy for verifying that GetSafeSnapshotBlockingPids() is working
correctly.  To test, open some number of SERIALIZABLE transactions and
take a snapshot in each, and then open a SERIALIZABLE READ ONLY
DEFERRABLE transaction and try to take a snapshot; it will block and
pg_safe_snapshot_blocking_pids() will identify the culprit(s).

Thanks to Andrew Gierth for an off-list discussion about the pitfalls
of trying to use "arrayoverlap" from inside
pg_isolation_test_session_is_blocked, which helped me decide that I
should open-code that logic instead.  Does that make sense?

-- 
Thomas Munro
http://www.enterprisedb.com


pg-isolation-test-session-is-blocked.patch
Description: Binary data


pg-safe-snapshot-blocking-pids.patch
Description: Binary data

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


Re: [HACKERS] [COMMITTERS] pgsql: Sync pg_dump and pg_dumpall output

2017-04-09 Thread Noah Misch
On Wed, Apr 05, 2017 at 02:49:41AM -0400, Noah Misch wrote:
> On Fri, Mar 24, 2017 at 12:26:33PM +0900, Michael Paquier wrote:
> > On Thu, Mar 23, 2017 at 1:10 AM, Stephen Frost  wrote:
> > > * Andrew Dunstan (andrew.duns...@2ndquadrant.com) wrote:
> > >> On 03/22/2017 11:39 AM, Stephen Frost wrote:
> > >> > * Andrew Dunstan (and...@dunslane.net) wrote:
> > >> >> Sync pg_dump and pg_dumpall output
> > >> > This probably should have adjusted all callers of pg_dump in the
> > >> > regression tests to use the --no-sync option, otherwise we'll end up
> > >> > spending possibly a good bit of time calling fsync() during the
> > >> > regression tests unnecessairly.
> > >>
> > >> All of them? The imnpact is not likely to be huge in most cases
> > >> (possibly different on Windows). On crake, the bin-check stage actually
> > >> took less time after the change than before, so I suspect that the
> > >> impact will be pretty small.
> > >
> > > Well, perhaps not all, but I'd think --no-sync would be better to have
> > > in most cases.  We turn off fsync for all of the TAP tests and all
> > > initdb calls, so it seems like we should here too.  Perhaps it won't be
> > > much overhead on an unloaded system, but not all of the buildfarm
> > > animals seem to be on unloaded systems, nor are they particularly fast
> > > in general or have fast i/o..
> > 
> > Agreed.
> > 
> > >> Still I agree that we should have tests for both cases.
> > >
> > > Perhaps, though if I recall correctly, we've basically had zero calls
> > > for fsync() until this.  If we don't feel like we need to test that in
> > > the backend then it seems a bit silly to feel like we need it for
> > > pg_dump's regression coverage.
> > >
> > > That said, perhaps the right answer is to have a couple tests for both
> > > the backend and for pg_dump which do exercise the fsync-enabled paths.
> > 
> > And agreed.
> > 
> > The patch attached uses --no-sync in most places for pg_dump, except
> > in 4 places of 002_pg_dump.pl to stress as well the sync code path.
> 
> [Action required within three days.  This is a generic notification.]
> 
> The above-described topic is currently a PostgreSQL 10 open item.  Andrew,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> v10 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within three calendar days of
> this message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping v10.  Consequently, I will appreciate your efforts
> toward speedy resolution.  Thanks.
> 
> [1] 
> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

This PostgreSQL 10 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:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] scram and \password

2017-04-09 Thread Noah Misch
On Wed, Apr 05, 2017 at 08:11:25PM +0300, Heikki Linnakangas wrote:
> On 04/05/2017 06:53 PM, Robert Haas wrote:
> >On Sat, Mar 25, 2017 at 1:10 AM, Michael Paquier
> > wrote:
> >>On Fri, Mar 24, 2017 at 10:12 PM, Heikki Linnakangas  
> >>wrote:
> >>>On 03/24/2017 03:02 PM, Michael Paquier wrote:
> 
> In order to close this thread, I propose to reuse the patches I sent
> here to make scram_build_verifier() available to frontends:
> 
> https://www.postgresql.org/message-id/CAB7nPqT4yc3u8wspYkWbG088Ndp6asMH3=zb___ck89ctvz...@mail.gmail.com
> 
> And on top of it modify \password so as it generates a md5 verifier
> for pre-9.6 servers and a scram one for post-10 servers by looking at
> the backend version of the current connection. What do you think?
> >>>
> >>>Yep, sounds like a plan.
> >>
> >>And attached is a set of rebased patches, with createuser and psql's
> >>\password extended to do that.
> >
> >Heikki, are you going to do something about these?  We're running out of 
> >time.
> 
> Sorry I've been procrastinating. I'm on it now. (We need to do something
> about this, feature freeze or not..)

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Heikki,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] postgres_fdw bug in 9.6

2017-04-09 Thread Ashutosh Bapat
On Sat, Apr 8, 2017 at 12:03 AM, Robert Haas  wrote:
> On Mon, Apr 3, 2017 at 2:12 AM, Etsuro Fujita
>  wrote:
>> On 2017/04/01 1:32, Jeff Janes wrote:
>>> On Thu, Mar 23, 2017 at 5:20 AM, Etsuro Fujita
>>> > wrote:
>>> Done.  Attached is a new version of the patch.
>>> Is the fix for 9.6.3 going to be just a back port of this, or will it
>>> look different?
>>
>> +1 for backporting; although that requires that GetForeignJoinPaths be
>> updated so that the FDW uses a new function to create an alternative local
>> join path (ie, CreateLocalJoinPath), that would make maintenance of the code
>> easy.
>
> Well, the problem here is that this breaks ABI compatibility.  If we
> applied this to 9.6, and somebody tried to use a previously-compiled
> FDW .so against a new server version, it would fail after the upgrade,
> because the new server wouldn't have GetExistingLocalJoinPath and also
> possibly because of the change to the structure of JoinPathExtraData.
> Maybe there's no better alternative, and maybe nothing outside of
> postgres_fdw is using this stuff anyway, but it seems like a concern.

I had submitted a patch in [1]. We thought that that patch is good to
fix the issue on the backbranches. But it got berried in the thread.
If you think that's a feasible solution for backbranches, I will work
on the comments.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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


Re: [HACKERS] logical replication and SIGHUP

2017-04-09 Thread Michael Paquier
On Mon, Apr 10, 2017 at 11:41 AM, Noah Misch  wrote:
> On Thu, Apr 06, 2017 at 02:21:29AM +0900, Fujii Masao wrote:
>> Both launcher and worker don't handle SIGHUP signal and cannot
>> reload the configuration. I think that this is a bug. Will add this as
>> an open item barring objection.
>
> [Action required within three days.  This is a generic notification.]
>
> The above-described topic is currently a PostgreSQL 10 open item.  Peter,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> v10 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within three calendar days of
> this message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping v10.  Consequently, I will appreciate your efforts
> toward speedy resolution.  Thanks.
>
> [1] 
> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

After more review, I think that got_SIGTERM should be of type volatile
sig_atomic_t in launcher.c or that's not signal-safe. I think as well
that for correctness errno should be saved as SetLatch() is called and
restored afterwards. Please find attached a patch to address all that.
-- 
Michael
VMware vCenter Server
www.vmware.com


logirep-sighup.patch
Description: Binary data

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


Re: [HACKERS] strange parallel query behavior after OOM crashes

2017-04-09 Thread Noah Misch
On Thu, Apr 06, 2017 at 03:04:13PM +0530, Kuntal Ghosh wrote:
> On Wed, Apr 5, 2017 at 6:49 PM, Amit Kapila  wrote:
> > On Wed, Apr 5, 2017 at 12:35 PM, Kuntal Ghosh
> >  wrote:
> >> On Tue, Apr 4, 2017 at 11:22 PM, Tomas Vondra
> >>> I'm probably missing something, but I don't quite understand how these
> >>> values actually survive the crash. I mean, what I observed is OOM followed
> >>> by a restart, so shouldn't BackgroundWorkerShmemInit() simply reset the
> >>> values back to 0? Or do we call ForgetBackgroundWorker() after the crash 
> >>> for
> >>> some reason?
> >> AFAICU, during crash recovery, we wait for all non-syslogger children
> >> to exit, then reset shmem(call BackgroundWorkerShmemInit) and perform
> >> StartupDataBase. While starting the startup process we check if any
> >> bgworker is scheduled for a restart.
> >>
> >
> > In general, your theory appears right, but can you check how it
> > behaves in standby server because there is a difference in how the
> > startup process behaves during master and standby startup?  In master,
> > it stops after recovery whereas in standby it will keep on running to
> > receive WAL.
> >
> While performing StartupDatabase, both master and standby server
> behave in similar way till postmaster spawns startup process.
> In master, startup process completes its job and dies. As a result,
> reaper is called which in turn calls maybe_start_bgworker().
> In standby, after getting a valid snapshot, startup process sends
> postmaster a signal to enable connections. Signal handler in
> postmaster calls maybe_start_bgworker().
> In maybe_start_bgworker(), if we find a crashed bgworker(crashed_at !=
> 0) with a NEVER RESTART flag, we call ForgetBackgroundWorker().to
> forget the bgworker process.
> 
> I've attached the patch for adding an argument in
> ForgetBackgroundWorker() to indicate a crashed situation. Based on
> that, we can take the necessary actions. I've not included the Assert
> statement in this patch.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Problem in Parallel Bitmap Heap Scan?

2017-04-09 Thread Noah Misch
On Sat, Mar 25, 2017 at 09:55:21PM +1300, Thomas Munro wrote:
> On Sat, Mar 25, 2017 at 6:04 PM, Amit Kapila  wrote:
> > On Tue, Mar 21, 2017 at 5:51 PM, Dilip Kumar  wrote:
> >> On Tue, Mar 21, 2017 at 4:47 PM, Thomas Munro
> >>  wrote:
> >>> I noticed a failure in the inet.sql test while running the regression
> >>> tests with parallelism cranked up, and can reproduce it interactively
> >>> as follows.  After an spgist index is created and the plan changes to
> >>> the one shown below, the query returns no rows.
> >>
> >> Thanks for reporting.  Seems like we are getting issues related to
> >> TBM_ONE_PAGE and TBM_EMPTY.
> >>
> >> I think in this area we need more testing, reason these are not tested
> >> properly because these are not the natural case for parallel bitmap.
> >> I think in next few days I will test more such cases by forcing the
> >> parallel bitmap.
> >>
> >
> > Okay, is your testing complete?
> >
> >> Here is the patch to fix the issue in hand.  I have also run the
> >> regress suit with force_parallel_mode=regress and all the test are
> >> passing.
> >>
> >
> > Thomas, did you get chance to verify Dilip's latest patch?
> >
> > I have added this issue in PostgreSQL 10 Open Items list so that we
> > don't loose track of this issue.
> 
> The result is correct with this patch.  I ran make installcheck then
> the same steps as above and the query result was correct after
> creating the index.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] logical replication and SIGHUP

2017-04-09 Thread Noah Misch
On Thu, Apr 06, 2017 at 02:21:29AM +0900, Fujii Masao wrote:
> Both launcher and worker don't handle SIGHUP signal and cannot
> reload the configuration. I think that this is a bug. Will add this as
> an open item barring objection.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] error handling in RegisterBackgroundWorker

2017-04-09 Thread Noah Misch
On Wed, Mar 29, 2017 at 04:58:40PM -0300, Alvaro Herrera wrote:
> Robert Haas wrote:
> > On Wed, Mar 29, 2017 at 2:10 PM, Peter Eisentraut
> >  wrote:
> > > How specifically would we do that?  And what user would choose the
> > > behavior "start this background worker but don't worry if it doesn't 
> > > work"?
> > 
> > Well, if the background worker is auto-prewarm, you'd probably rather
> > have the database start rather than get unhappy about auto-prewarm
> > failing.  If the background worker is your logical replication
> > launcher it's a bit more serious, but if you have no subscriptions or
> > they're not that critical, maybe you don't care.  If the background
> > worker is in charge of telling your failover solution that this node
> > is up, then starting without it is entirely pointless.
> > 
> > I would be inclined to leave this alone for now and revisit it for a
> > future release.  I don't feel confident that we really know what the
> > right thing to do is here.
> 
> I think the common case is for modules to be critical: you may not care
> about it for auto-prewarm, but that seems like a special case.  I would
> put it the other way around: having the load fail is a serious problem
> unless specifically configured not to be.  I'd do as Peter suggests, and
> perhaps allow the current behavior optionally.  In hindsight, the
> current behavior seems like a mistake.

Agreed.  There are times when starting up degraded beats failing to start,
particularly when the failing component has complicated dependencies.  In this
case, as detailed upthread, this can only fail in response to basic
misconfiguration.  It's not the kind of thing that will spontaneously fail
after a minor upgrade, for example.


[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] logical replication worker and statistics

2017-04-09 Thread Noah Misch
On Wed, Apr 05, 2017 at 05:02:18PM +0300, Stas Kelvich wrote:
> > On 27 Mar 2017, at 18:59, Robert Haas  wrote:
> > On Mon, Mar 27, 2017 at 11:14 AM, Fujii Masao  wrote:
> >> Logical replication worker should call pgstat_report_stat()?
> >> Currently it doesn't seem to do that and no statistics about
> >> table accesses by logical replication workers are collected.
> >> For example, this can prevent autovacuum from working on
> >> those tables properly.
> > 
> > Yeah, that doesn't sound good.
> 
> Seems that nobody is working on this, so i’m going to create the patch.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Re: Somebody has not thought through subscription locking considerations

2017-04-09 Thread Noah Misch
On Sat, Apr 01, 2017 at 02:25:54AM +0200, Petr Jelinek wrote:
> On 01/04/17 01:57, Petr Jelinek wrote:
> > On 01/04/17 01:20, Tom Lane wrote:
> >> Petr Jelinek  writes:
> >>> But the pg_subscription_rel is also not accessed on heap_open, the
> >>> problematic code is called from heap_drop_with_catalog. And VACUUM FULL
> >>> pg_class calls heap_drop_with_catalog() when doing the heap swap (and it
> >>> goes through performDeletion so through dependency info which is what I
> >>> mean by everything else does this).
> >>
> >> Hmmm.  What the heap_drop_with_catalog call is being applied to is a
> >> short-lived table that is not pg_class.  It happens to own the disk
> >> storage that used to belong to pg_class, but it is not pg_class;
> >> in particular it doesn't have the same OID, and it is not what would
> >> be looked in if someone happened to need to fetch a pg_class row
> >> at that point.  So there's no circularity involved.
> >>
> >> On further reflection it seems like you were right, the problem is
> >> taking a self-exclusive lock on pg_subscription_rel during low-level
> >> catalog operations.  We're going to have to find a way to reduce that
> >> lock strength, or we're going to have a lot of deadlock problems.
> >>
> > 
> > Well we have heavy lock because we were worried about failure scenarios
> > in our dumb upsert in SetSubscriptionRelState which does cache lookup
> > and if it finds something it updates, otherwise inserts. We have similar
> > pattern in other places but they are called from DDL statements usually
> > so the worst that can happen is DDL fails with weird errors when done
> > concurrently with same name so using RowExclusiveLock is okay.
> > 
> > That being said, looking at use-cases for SetSubscriptionRelState that's
> > basically CREATE SUBSCRIPTION, ALTER SUBSCRIPTION REFRESH and tablesync
> > worker. So the DDL thing applies to first ones as well and tablesync
> > should not be running in case the record does not exist so it's fine if
> > it fails. In terms of RemoveSubscriptionRel that's only called from
> > heap_drop_with_catalog and tablesync holds relation lock so there is no
> > way heap_drop_with_catalog will happen on the same relation. This leads
> > me to thinking that RowExclusiveLock is fine for both
> > SetSubscriptionRelState and RemoveSubscriptionRel as long as we document
> > that callers should be aware that SetSubscriptionRelState has
> > concurrency issues and fail on unique index check.
> > 
> 
> And a simple patch to do so. Peter do you see any problem with doing this?

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] logical replication apply to run with sync commit off by default

2017-04-09 Thread Noah Misch
On Thu, Apr 06, 2017 at 01:38:41PM -0400, Peter Eisentraut wrote:
> On 3/24/17 10:49, Petr Jelinek wrote:
> > On 07/03/17 06:23, Petr Jelinek wrote:
> >> there has been discussion at the logical replication initial copy 
> >> thread
> >> [1] about making apply work with sync commit off by default for
> >> performance reasons and adding option to change that per subscription.
> >>
> >> Here I propose patch to implement this - it adds boolean column
> >> subssynccommit to pg_subscription catalog which decides if
> >> synchronous_commit should be off or local for apply. And it adds
> >> SYNCHRONOUS_COMMIT = boolean to the list of WITH options for CREATE and
> >> ALTER SUBSCRIPTION. When nothing is specified it will set it to false.
> 
> > Rebase after table copy patch got committed.
> 
> I think this change could use some more documentation.  Under what
> circumstances would a user want to turn this back on?

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Logical replication and inheritance

2017-04-09 Thread Noah Misch
On Wed, Apr 05, 2017 at 08:25:56AM -0400, Peter Eisentraut wrote:
> After thinking about it some more, I think the behavior we want would be
> that changes to inheritance would reflect in the publication membership.
>  So if you have a partitioned table, adding more partitions over time
> would automatically add them to the publication.
> 
> We could implement this either by updating pg_publication_rel as
> inheritance changes, or perhaps more easily by checking and expanding
> inheritance in the output plugin/walsender.  There might be subtle
> behavioral differences between those two approaches that we should think
> through.  But I think one of these two should be done.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] SUBSCRIPTIONS and pg_upgrade

2017-04-09 Thread Noah Misch
On Fri, Feb 17, 2017 at 09:33:32AM -0500, Stephen Frost wrote:
> Peter,
> 
> * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> > On 2/16/17 21:04, Stephen Frost wrote:
> > > I'm not entirely sure about the reasoning behind requiring a flag to
> > > include subscriptions in pg_dump output,
> > 
> > One reason is that pg_subscription is only readable by a superuser, so
> > we can't even dump them unless we're superuser.
> 
> Sure, but that's true of roles and other things..  On a system with RLS,
> likely only the database superuser can actually read everything.
> 
> > Also, restoring a subscription will immediately attempt to start
> > replication, which might be kind of surprising.
> 
> Isn't that what the 'disabled' option is for?  Also, when it comes to
> pg_upgrade, isn't that what you would probably want?
> 
> Perhaps it shouldn't start immediately, but rather create them as
> disabled at first and then start them at the end of the restore.
> 
> > We had pondered this issue extensively during early review.  I don't
> > think we're all happy with the current behavior, but it's probably the
> > safest and easiest one for now.
> > 
> > Better ideas are welcome.
> 
> I don't think we can simply punt entirely on this, particularly for the
> pg_upgrade case.  At the least, I think we should be exporting the
> subscriptions in a disabled fashion, so that at least the user *has*
> them in the backup, even if they aren't enabled.  With pg_upgrade, I
> would think we'd want to have some option to tell pg_upgrade to include
> subscriptions, or to enable them afterwards if they're included but off
> by default.
> 
> I've added it to the open items for PG10.  Perhaps we can get some other
> input/suggestions on it.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Peter,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] pg_dump emits ALTER TABLE ONLY partitioned_table

2017-04-09 Thread Tom Lane
Robert Haas  writes:
> I would appreciate help from other contributors and committers on this
> open item; pg_dump is not my strong point.  In the absence of such
> help, I will do my best with it.  I will set aside time this week to
> study this and send another update no later than Thursday.

The proposed patch seems rather ad-hoc, and I think that it is working
around a backend behavior that might be broken.

While I admit that I've not been paying close attention to the whole
table partitioning business, I wonder whether we have any clearly written
down specification about (a) how much partition member tables are allowed
to deviate schema-wise from their parent, and (b) how DDL semantics on
partitioned tables differ from DDL semantics for traditional inheritance.
Obviously those are closely related questions.  But the fact that this
bug exists at all shows that there's been some lack of clarity on (b),
and so I wonder whether we have any clarity on (a) either.

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: [pgsql-www] [HACKERS] Small issue in online devel documentation build

2017-04-09 Thread Bruce Momjian
On Sun, Apr  9, 2017 at 06:59:09PM +0200, Magnus Hagander wrote:
> On Sat, Apr 8, 2017 at 3:52 AM, Bruce Momjian  wrote:
> Attached is a patch that can be applied to pgweb which should fix all of
> this.
>
> Is there any chance we can find a way to do this with actual CSS selectors and
> not use javascript? I realize there might not be, but have we explored the
> option properly on the way the site layout looks now and with reasonably 
> modern
> browsers?

I realize that using JavaScript to insert CSS styles into an HTML
document is complex, but I know of no other way to scale a font style to
match another font style.  CSS doesn't give you that control.  You can
use CSS to prevent the display of certain font styles, but that hardly
seems like a win.  There is probably a way to do the entire thing only
in JavaScript, but again, that doesn't seem any better.

-- 
  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: [pgsql-www] [HACKERS] Small issue in online devel documentation build

2017-04-09 Thread Bruce Momjian
On Sun, Apr  9, 2017 at 07:00:38PM +0200, Magnus Hagander wrote:
> 
> 
> On Sun, Apr 9, 2017 at 2:32 AM, Bruce Momjian  wrote:
> 
> On Sat, Apr  8, 2017 at 12:50:19PM -0400, Robert Haas wrote:
> > On Sat, Apr 8, 2017 at 6:39 AM, Bruce Momjian  wrote:
> > > What other problems do we have with pgweb that I can work on?
> >
> > Well, the 10devel documentation doesn't believe in orange.  Compare:
> >
> > https://www.postgresql.org/docs/devel/static/sql-createtable.html
> > https://www.postgresql.org/docs/9.6/static/sql-createtable.html
> >
> > I think that needs to be fixed.
> 
> The attached CSS patch will fix that specific issue, but I am unclear if
> there are more places that need color.
> 
> 
> 
> Does this not also affect existing versions of the docs? AFAICT it would hit
> any and all  tags, and we certainly have those in earlier versions?

It would hit any class of .refentrytitle inside of an  block.  I
didn't that that happened in the old docs, but I can use an href filter
to do it only for docs of versions starting with '1' or saying 'devel'.

> And it also modifies a global stylesheet for *all* pages. So what about the
>  on all the non-docs pages?

Again, we can restrict to only certain href patterns.

-- 
  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] pg_dump emits ALTER TABLE ONLY partitioned_table

2017-04-09 Thread Robert Haas
On Sun, Apr 9, 2017 at 7:50 PM, Noah Misch  wrote:
> The above-described topic is currently a PostgreSQL 10 open item.  Robert,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> v10 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within three calendar days of
> this message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping v10.  Consequently, I will appreciate your efforts
> toward speedy resolution.  Thanks.

I would appreciate help from other contributors and committers on this
open item; pg_dump is not my strong point.  In the absence of such
help, I will do my best with it.  I will set aside time this week to
study this and send another update no later than Thursday.

-- 
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] dropping a partition may cause deadlock

2017-04-09 Thread Noah Misch
On Mon, Apr 03, 2017 at 03:48:05PM +0900, Amit Langote wrote:
> I noticed that a deadlock can occur due to the way locking when dropping a
> partition proceeds.  Steps to reproduce:
> 
> 1. Attach debugger to two sessions, one of which will do a select on the
> partitioned parent and the other will drop one of its partitions.
> 
> 2. In the first debugging session, set a breakpoint at the start of
> expand_inherited_rtentry() which is the first point in a select query's
> processing where individual partitions will be locked (the parent will
> have already been locked by the rewriter).
> 
> 3. In the second session, set a breakpoint at the start of
> heap_drop_with_catalog(), which is the first point in the drop command's
> processing where the parent will be locked (the partition will have
> already been locked by RangeVarGetRelidExtended()).  This will wait for
> the first session to release the lock on the parent.
> 
> 4. In the first session, proceeding with locking of the partition will
> cause it wait for the second session that is holding a lock on it; a
> deadlock is detected, because that session is waiting for us to release
> the lock on the parent.
> 
> Attached is a patch to fix that.  In the original partitioning patch, I
> had aped the approach of index_drop() where the parent heap relation is
> locked along with the index relation so that the parent's cached list of
> indexes can be invalidated.  But I failed to also ape what
> RangeVarCallbackForDropRelation() does when dropping an index, which is to
> lock the parent heap relation before locking the index relation at all.
> For dropping a partition case, it means we lock the parent before we lock
> the partition relation.
> 
> Will add this to open items list.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] tuple-routing and constraint violation error message, revisited

2017-04-09 Thread Noah Misch
On Fri, Mar 31, 2017 at 11:13:03AM +0900, Amit Langote wrote:
> Last message regarding this was by Robert on the original partitioning thread:
> 
> https://www.postgresql.org/message-id/CA%2BTgmoZjGzSM5WwnyapFaw3GxnDLWh7pm8Xiz8_QWQnUQy%3DSCA%40mail.gmail.com
> 
> Summary is: We decided in f1b4c771ea7 [1] that passing the original slot
> (one containing the tuple formatted per root partitioned table's tupdesc)
> to ExecConstraints(), but that breaks certain cases.  Imagine what would
> happen if a BR insert trigger changed the tuple - the original slot would
> not contain those changes. So, it seems better to convert (if necessary)
> the tuple formatted per partition tupdesc after tuple-routing back to the
> root table's format and use the converted tuple to make val_desc shown in
> the message if an error occurs.
> 
> Attached rebased version of the patch that I had originally proposed
> (summary above is the commit message).  Robert thought it would be fine to
> show the row formatted per partition rowtype, but would look better if we
> could show the column names as well (remember that we're trying to account
> for possible differences in the ordering of columns between the root table
> and leaf partitions to which tuples are routed.)
> 
> Added this to PostgreSQL 10 open items list.

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] pg_dump emits ALTER TABLE ONLY partitioned_table

2017-04-09 Thread Noah Misch
On Wed, Mar 29, 2017 at 05:38:41PM +0900, Amit Langote wrote:
> On 2017/03/29 0:39, Robert Haas wrote:
> > On Tue, Mar 28, 2017 at 6:50 AM, Amit Langote
> >  wrote:
> >>> Isn't it bogus that this is generating ALTER TABLE .. SET NOT NULL
> >>> columns at all?  You didn't say anything like that when setting up the
> >>> database, so why should it be there when dumping?
> >>
> >> So we should find a way for the NOT NULL constraints added for the range
> >> partition key columns to not be emitted *separately*?  Like when a table
> >> has primary key:
> >>
> >> --
> >> -- Name: foo; Type: TABLE; Schema: public; Owner: amit
> >> --
> >>
> >> CREATE TABLE foo (
> >> a integer NOT NULL
> >> );
> >>
> >>
> >> ALTER TABLE foo OWNER TO amit;
> >>
> >> --
> >> -- Name: foo foo_pkey; Type: CONSTRAINT; Schema: public; Owner: amit
> >> --
> >>
> >> ALTER TABLE ONLY foo
> >> ADD CONSTRAINT foo_pkey PRIMARY KEY (a);
> >>
> >> The NOT NULL constraint is emitted with CREATE TABLE, not separately.
> > 
> > Hmm, that's not exactly what I was thinking, but I think what I was
> > thinking was wrong, so, yes, can we do what you said?
> 
> I thought about this for a while.  Although it seems we can do what I said
> for (partitioned) tables themselves, it's not real clear to me how
> straightforward it is to do for partitions (child tables). Inheritance or
> localness of attributes/constraints including NOT NULL dictates whether an
> attribute or a constraint is emitted separately.  I think that any
> additional consideration will make the logic to *not* dump separately (or
> perhaps to not emit at all) will become more involved.  For example, if a
> NOT NULL constraint on a column has been inherited and originated in the
> parent from the range partition key, then does it mean we should not emit
> it or emit it but not separately?

[Action required within three days.  This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
since you committed the patch believed to have created it, you own this open
item.  If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know.  Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message.  Include a date for your subsequent status update.  Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10.  Consequently, I will appreciate your efforts
toward speedy resolution.  Thanks.

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] partitioned tables and contrib/sepgsql

2017-04-09 Thread Joe Conway
On 04/09/2017 04:14 PM, Tom Lane wrote:
> Joe Conway  writes:
>>> I turned on the buildfarm "keep" setting and looked at the diffs. The
>>> issue is that in there are a few places that do "SELECT ... FROM
>>> pg_seclabels ... ORDER BY ..." and when manually testing I get default
>>> database encoding "UTF8" but with the buildfarm I get "SQL_ASCII", hence
>>> a different sorting.
> 
>> Looks like adding COLLATE "C" in the ORDER BY clauses does the trick. Is
>> that the preferred method for this kind of issue?
> 
> Yeah, that's pretty much the standard fix nowadays.


Good to hear, thanks -- rhino is back to green.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] pgbench - allow to store select results into variables

2017-04-09 Thread Tatsuo Ishii
Fabien,

>> As the variable infrastructures are pretty different between psql &
>> pgbench (typed vs untyped values, sorted array vs linked list data
>> structure, no hook vs 2 hooks, name spaces vs no such thing...), I
>> have chosen the simplest option of just copying the name checking
>> function and extending the lexer to authorize non-ascii letters, so
>> that psql/pgbench would accept the same variable names with the same
>> constraint about encodings.
>>
>> See patch attached & test script.
> 
> Argh, I'm jet-lagged, wrong patch suffix... Here it is with the right
> suffix.

Thank you for the patch. I tested a little bit and found that it does
not allow value replacement against non ascii variables in given SQL
statements . Is it intentional? If not, I think you need to fix
parseVariable() as well.

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


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


Re: [HACKERS] "left shift of negative value" warnings

2017-04-09 Thread Tom Lane
Andres Freund  writes:
> For a while I've been getting warnings like
> /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c: In 
> function ‘inet_cidr_ntop_ipv6’:
> /home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c:205:11: 
> warning: left shift of negative value [-Wshift-negative-value]
> m = ~0 << (8 - b);
>^~

I imagine forcing the LHS to unsigned would silence that, though you'd
have to be careful that the sign extension (widening) happened before
you changed the value to unsigned, in the int64 cases.  It's a bit odd
though that it seems to think ~0 is signed.

> If I understand C99 correctly, the behaviour of a left-shift of a
> negative number is undefined (6.5.7 4.).

As I read that, it's only "undefined" if overflow would occur (ie
the sign bit would change).  Your compiler is being a useless annoying
nanny, but that seems to be the in thing for compiler authors these
days.

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] SCRAM authentication, take three

2017-04-09 Thread Noah Misch
On Fri, Apr 07, 2017 at 10:28:59AM +0300, Heikki Linnakangas wrote:
> On 04/07/2017 08:21 AM, Noah Misch wrote:
> >Michael shared[1] better pg_hba.conf syntax on 2016-11-05.  I agreed[2] with
> >his framing of the problem and provided two syntax alternatives, on
> >2017-01-18.  Michael implemented[3] a variation of one of those on 
> >2017-02-20,
> >which you declined in your 2017-03-07 commit with just the explanation quoted
> >above.  I say Michael came up with something better five months ago.
> 
> OK. My feeling is that we should have a relatively short and
> easy-to-pronounce name for it. People editing pg_hba.conf with a text editor
> will need to type in the keyword, and "scram" is a lot easier to remember
> than "scram-sha-256". The word will also be used in conversations, "hey,
> Noah, can you add example.com to the hba file, with scram, please?" If md5
> had a more difficult name, I think we would've come up with a shorthand for
> it back in the day, too.
> 
> I might be wrong, of course. I don't set up PostgreSQL installations for a
> living, so I might be out of touch of what's important.

Likewise, but I've never seen pg_hba.conf edits become a large slice of
PostgreSQL DBA work.  Whereas experts can appreciate terse query syntax,
pg_hba.conf syntax gains little from terseness.

> In [1], Michael wrote:
> >There is also the channel binding to think about... So we could have a
> >list of keywords perhaps associated with SASL? Imagine for example:
> >sasl$algo,$channel_binding
> >Giving potentially:
> >saslscram_sha256
> >saslscram_sha256,channel
> >saslscram_sha512
> >saslscram_sha512,channel
> >In the case of the patch of this thread just the first entry would
> >make sense, once channel binding support is added a second
> >keyword/option could be added. And there are of course other methods
> >that could replace SCRAM..
> 
> It should also be possible to somehow specify "use channel binding, if the
> client supports it".
> 
> I don't think "sasl" is interesting to a user, it's the actual mechanisms
> (e.g "scram-sha256") that matter. So I'd suggest that we allow a list of
> algorithms in the method field. If we go with the longer "scram-sha-256"
> name, it would look like this:
> 
> # TYPE  DATABASEUSERADDRESS METHOD
> host  all all example.com scram-sha-256-plus,
> scram-sha-256

I agree "sasl" focuses on a less-interesting aspect of the authentication
method.  Allowing multiple methods per HBA line may be the better answer, so
long as the policy questions it raises aren't too thorny.  Do you allow any
combination of methods or limit it somehow (e.g. only SASL methods)?  If the
same option pertains to two methods, do we provide a way to set the option on
just one method?  Those don't seem too challenging, though.

> The problem again is that those names are quite long. Is that OK?

Yes.

With this, you could have a meta-method name (e.g. "default") that stands for
all methods generally considered safe.  Compare SSL default cipher lists.

> In [2], you wrote:
> >The latest versions document this precisely, but I agree with Peter's concern
> >about plain "scram".  Suppose it's 2025 and PostgreSQL support SASL 
> >mechanisms
> >OAUTHBEARER, SCRAM-SHA-256, SCRAM-SHA-256-PLUS, and SCRAM-SHA3-512.  What
> >should the pg_hba.conf options look like at that time?  I don't think having 
> >a
> >single "scram" option fits in such a world.  I see two strategies that fit:
> >
> >1. Single "sasl" option, with a GUC, similar to ssl_ciphers, controlling the
> >   mechanisms to offer.
> >2. Separate options "scram_sha_256", "scram_sha3_512", "oauthbearer", etc.
> 
> The example I gave above is like option 2. The problem with option 1 is that
> different SASL mechanisms can have very different properties. You might want
> to allow "NTLM" from a trusted network, but require "OTP" from the public
> internet, for example. So I don't think a single GUC would be flexible
> enough.
> 
> That said, a GUC with a more narrow scope might be OK. If we called the
> method in pg_hba.conf "secure_password", and had a GUC to list which
> password-based mechanisms are considered secure, that would be OK. But I
> think we'd still need a separate pg_hba.conf method name for OAUTHBEARER,
> for example.

Michael replied to my message with the idea to use a mechanism= HBA option.
That's better than a GUC.

> PS. If we go with the full names, I think it should "scram-sha-256", rather
> than "scram_sha_256", because the official name uses dashes rather than
> underscores.

Agreed, I don't remember why I wrote underscores.  One could argue on that
basis for using uppercase, but I won't.


These are the two chief approaches I'm seeing:

1. scram-sha-256, scram-sha-256-plus, and successors will be their own
   pg_hba.conf authentication methods.  Until and unless someone implements an
   ability to name multiple methods per HBA line, you must choose 

Re: [HACKERS] partitioned tables and contrib/sepgsql

2017-04-09 Thread Tom Lane
Joe Conway  writes:
>> I turned on the buildfarm "keep" setting and looked at the diffs. The
>> issue is that in there are a few places that do "SELECT ... FROM
>> pg_seclabels ... ORDER BY ..." and when manually testing I get default
>> database encoding "UTF8" but with the buildfarm I get "SQL_ASCII", hence
>> a different sorting.

> Looks like adding COLLATE "C" in the ORDER BY clauses does the trick. Is
> that the preferred method for this kind of issue?

Yeah, that's pretty much the standard fix nowadays.

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] partitioned tables and contrib/sepgsql

2017-04-09 Thread Joe Conway
On 04/09/2017 03:01 PM, Joe Conway wrote:
> On 04/09/2017 02:49 PM, Andres Freund wrote:
>> On 2017-04-09 14:28:48 -0700, Joe Conway wrote:
>>> Interesting -- rhino is now failing. I tested a minute ago manually on
>>> the same buildfarm animal and it passed. I'm on it.
>> 
>> The module for segpsql really needs to be improved so it logs
>> regression.diffs - iirc several other modules do.
> 
> Sure, but for another day I think.
> 
> I turned on the buildfarm "keep" setting and looked at the diffs. The
> issue is that in there are a few places that do "SELECT ... FROM
> pg_seclabels ... ORDER BY ..." and when manually testing I get default
> database encoding "UTF8" but with the buildfarm I get "SQL_ASCII", hence
> a different sorting.

Looks like adding COLLATE "C" in the ORDER BY clauses does the trick. Is
that the preferred method for this kind of issue?

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] partitioned tables and contrib/sepgsql

2017-04-09 Thread Joe Conway
On 04/09/2017 02:49 PM, Andres Freund wrote:
> On 2017-04-09 14:28:48 -0700, Joe Conway wrote:
>> Interesting -- rhino is now failing. I tested a minute ago manually on
>> the same buildfarm animal and it passed. I'm on it.
> 
> The module for segpsql really needs to be improved so it logs
> regression.diffs - iirc several other modules do.

Sure, but for another day I think.

I turned on the buildfarm "keep" setting and looked at the diffs. The
issue is that in there are a few places that do "SELECT ... FROM
pg_seclabels ... ORDER BY ..." and when manually testing I get default
database encoding "UTF8" but with the buildfarm I get "SQL_ASCII", hence
a different sorting.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


[HACKERS] "left shift of negative value" warnings

2017-04-09 Thread Andres Freund
Hi,

For a while I've been getting warnings like
/home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c: In function 
‘inet_cidr_ntop_ipv6’:
/home/andres/src/postgresql/src/backend/utils/adt/inet_cidr_ntop.c:205:11: 
warning: left shift of negative value [-Wshift-negative-value]
m = ~0 << (8 - b);
   ^~
/home/andres/src/postgresql/src/backend/utils/adt/network.c: In function 
‘inetmi’:
/home/andres/src/postgresql/src/backend/utils/adt/network.c:1482:24: warning: 
left shift of negative value [-Wshift-negative-value]
res |= ((int64) -1) << (byte * 8);
^~
/home/andres/src/postgresql/src/backend/utils/adt/varbit.c: In function 
‘bitfromint4’:
/home/andres/src/postgresql/src/backend/utils/adt/varbit.c:1546:16: warning: 
left shift of negative value [-Wshift-negative-value]
val |= (-1) << (srcbitsleft + 8 - destbitsleft);
^~
/home/andres/src/postgresql/src/backend/utils/adt/varbit.c: In function 
‘bitfromint8’:
/home/andres/src/postgresql/src/backend/utils/adt/varbit.c:1626:16: warning: 
left shift of negative value [-Wshift-negative-value]
val |= (-1) << (srcbitsleft + 8 - destbitsleft);

If I understand C99 correctly, the behaviour of a left-shift of a
negative number is undefined (6.5.7 4.).  In C89 the spec was very
unclear about that.

Greetings,

Andres Freund


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


Re: [HACKERS] [sqlsmith] Planner crash on foreign table join

2017-04-09 Thread Thomas Munro
On Sun, Apr 9, 2017 at 8:27 AM, Andrew Gierth
 wrote:
> foreach(l, blah)
> {
> SomeType *x = (SomeType *) lfirst(l);
>
> (in my code I tend to omit the (SomeType *), which I dislike because it
> adds no real protection)

Just BTW, without that cast it's not compilable as C++, so I'm
guessing that Peter E will finish up putting it back in wherever you
leave it out...

-- 
Thomas Munro
http://www.enterprisedb.com


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


Re: [HACKERS] partitioned tables and contrib/sepgsql

2017-04-09 Thread Andres Freund
On 2017-04-09 14:28:48 -0700, Joe Conway wrote:
> Interesting -- rhino is now failing. I tested a minute ago manually on
> the same buildfarm animal and it passed. I'm on it.

The module for segpsql really needs to be improved so it logs
regression.diffs - iirc several other modules do.

Greetings,

Andres Freund


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


Re: [HACKERS] partitioned tables and contrib/sepgsql

2017-04-09 Thread Joe Conway
On 04/09/2017 02:04 PM, Joe Conway wrote:
> On 04/08/2017 07:29 AM, Stephen Frost wrote:
>> * Joe Conway (m...@joeconway.com) wrote:
>>> On 04/07/2017 05:36 PM, Robert Haas wrote:
>>> > On Fri, Apr 7, 2017 at 5:22 PM, Joe Conway  wrote:
>>> >> 1) commit the 0002 patch now before the feature freeze and follow up
>>> >>with the regression test patch when ready in a couple of days
>>> >> 2) hold off on both patches until ready
>>> >> 3) push both patches to the next commitfest/pg11
>>> >>
>>> >> Some argue this is an open issue against the new partitioning feature in
>>> >> pg10 and therefore should be addressed now, and others do not. I can see
>>> >> both sides of that argument.
>>> >>
>>> >> In any case, thoughts on what to do?
>>> > 
>>> > Speaking only for myself, I'm OK with any of those options, provided
>>> > that that "a couple" means what my dictionary says it means.
>>> 
>>> Thanks. I'd prefer not to do #1 actually, and I think we can adhere to
>>> the dictionary meaning of "a couple" (i.e. by EOD Sunday). Assuming we
>>> are ready by Sunday I will push both together (#2) or else I will move
>>> them both together to the next CF (#3).
>> 
>> Sounds good to me.
> 
> Having heard no objections, committed and pushed as per option #2.

Interesting -- rhino is now failing. I tested a minute ago manually on
the same buildfarm animal and it passed. I'm on it.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] missing isinf declaration on solaris

2017-04-09 Thread Andres Freund
On 2014-09-24 16:26:33 -0400, Tom Lane wrote:
> Peter Eisentraut  writes:
> > On 9/24/14 9:21 AM, Tom Lane wrote:
> >> Agreed, but what about non-GCC compilers?
>
> > Stick AC_PROG_CC_C99 into configure.in.
>
> I think that's a bad idea, unless you mean to do it only on Solaris.
> If we do that unconditionally, we will pretty much stop getting any
> warnings about C99-isms on modern platforms.

FWIW, that's not that much of an issue these days, because e.g. mylodon
runs with -std=c89 -Werror=c99-extensions - so far that seems to have been
reasonably accurate.

- 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] partitioned tables and contrib/sepgsql

2017-04-09 Thread Joe Conway
On 04/08/2017 07:29 AM, Stephen Frost wrote:
> * Joe Conway (m...@joeconway.com) wrote:
>> On 04/07/2017 05:36 PM, Robert Haas wrote:
>> > On Fri, Apr 7, 2017 at 5:22 PM, Joe Conway  wrote:
>> >> 1) commit the 0002 patch now before the feature freeze and follow up
>> >>with the regression test patch when ready in a couple of days
>> >> 2) hold off on both patches until ready
>> >> 3) push both patches to the next commitfest/pg11
>> >>
>> >> Some argue this is an open issue against the new partitioning feature in
>> >> pg10 and therefore should be addressed now, and others do not. I can see
>> >> both sides of that argument.
>> >>
>> >> In any case, thoughts on what to do?
>> > 
>> > Speaking only for myself, I'm OK with any of those options, provided
>> > that that "a couple" means what my dictionary says it means.
>> 
>> Thanks. I'd prefer not to do #1 actually, and I think we can adhere to
>> the dictionary meaning of "a couple" (i.e. by EOD Sunday). Assuming we
>> are ready by Sunday I will push both together (#2) or else I will move
>> them both together to the next CF (#3).
> 
> Sounds good to me.

Having heard no objections, committed and pushed as per option #2.

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] problems compiling in solaris 10

2017-04-09 Thread Tom Lane
Jaime Casanova  writes:
> I'm compiling HEAD on solaris 10 and i'm getting this warning

> float.c: In function ‘is_infinite’:
> float.c:201:2: warning: implicit declaration of function ‘isinf’
> [-Wimplicit-function-declaration]

There was a previous discussion of that here:

https://www.postgresql.org/message-id/flat/542269E4.40406%40ohmu.fi

The discussion seems to have trailed off without any agreement to do
anything about it.  The concern was basically that turning on C99
mode might cause problems worse than an apparently-harmless warning.

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] pg_basebackup: Allow use of arbitrary compression program

2017-04-09 Thread Magnus Hagander
On Fri, Apr 7, 2017 at 4:04 AM, Michael Harris  wrote:

> Hello,
>
> Back in pg 9.2, we hacked a copy of pg_basebackup to add a command
> line option which would allow the user to specify an arbitrary
> external program (potentially including arguments) to be used to
> compress the tar backup.
>
> Our motivation was to be able to use pigz (parallel gzip
> implementation) to speed up the compression. It also allows using
> tools like bzip2, xz, etc instead of the inbuilt zlib.
>
> I never ended up submitting that upstream, but now it looks like I
> will have to repeat the exercise for 9.6, so I was wondering if such a
> feature would be welcomed.
>
> I found one or two references to people asking for this, eg:
> https://www.commandprompt.com/blog/a_pg_basebackup_wish_list/
>
> To do it properly would require:
>
> 1) Adding command line option as follows:
>
>   -C, --compressprog=PROG
>  Use supplied program for compression
>
> 2) The current logic either uses zlib if compiled in, or offers no
> compression at all, controlled by a series of #ifdef/#endif. I would
> prefer that the user can either use zlib or an external program
> without having to recompile, so I would remove the #ifdefs and replace
> them with run time branching.
>

Not sure how that would work or be needed. The reasonable thing would be if
zlib is available when building the choices would be "no compression",
"zlib compression" or "external compression". If there was no zlib
available when building, the choices would be "no compression" or "external
compression".

Or maybe I'm misunderstanding what you're saying?



> 3) When opening the output file, if the -C option was used, use popen
> to open a child process and write to that.
>
> My questions are:
> - Has anything like this already been discussed?
>

I think it has, but not in detail.



> - Would this be a welcome contribution?
>

Yes, I definitely think this would be useful.



> - Can anyone see any problems with the above approach?
>

One thing to consider is the work done recently to ensure that the output
is properly synchronized when written to disk. I don't think it's
reasonable to expect that from an external compression, but if it can be
made optional that'd be good. Or at least be careful not to break the
current one.

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


Re: [pgsql-www] [HACKERS] Small issue in online devel documentation build

2017-04-09 Thread Magnus Hagander
On Sun, Apr 9, 2017 at 2:32 AM, Bruce Momjian  wrote:

> On Sat, Apr  8, 2017 at 12:50:19PM -0400, Robert Haas wrote:
> > On Sat, Apr 8, 2017 at 6:39 AM, Bruce Momjian  wrote:
> > > What other problems do we have with pgweb that I can work on?
> >
> > Well, the 10devel documentation doesn't believe in orange.  Compare:
> >
> > https://www.postgresql.org/docs/devel/static/sql-createtable.html
> > https://www.postgresql.org/docs/9.6/static/sql-createtable.html
> >
> > I think that needs to be fixed.
>
> The attached CSS patch will fix that specific issue, but I am unclear if
> there are more places that need color.
>
>
Does this not also affect existing versions of the docs? AFAICT it would
hit any and all  tags, and we certainly have those in earlier versions?

And it also modifies a global stylesheet for *all* pages. So what about the
 on all the non-docs pages?

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


Re: [pgsql-www] [HACKERS] Small issue in online devel documentation build

2017-04-09 Thread Magnus Hagander
On Sat, Apr 8, 2017 at 3:52 AM, Bruce Momjian  wrote:

> On Fri, Mar 24, 2017 at 07:01:46AM +0100, Fabien COELHO wrote:
> >
> > Hello Peter,
> >
> > >I think the fix belongs into the web site CSS, so there is nothing to
> > >commit into PostgreSQL here.
> >
> > Indeed, the changes were only for the "remove nesting" solution.
> >
> > >I will close the commit fest entry, but I have added a section to the
> open
> > >items list so we keep track of it. (https://wiki.postgresql.org/
> wiki/PostgreSQL_10_Open_Items#Documentation_tool_chain)
> >
> > I put forward that the quick workaround a colleague of mine suggested
> (aka
> > something like code code { font-size: 100%; important! }) could also be
> > applied to the web site CSS while waiting for a more definite answer
> which
> > might take some pretty unknown time close to never?
>
> Sorry I am just getting back to this.  Below I am going to cover only
> the problem with the font size of nested  tags, and I am going to
> confirm what most people already figured out.
>
> The basic problem was already posted by Fabien, with an image example.
> The cause of the fonts being too large on Chrome is an interaction of
> Chrome's default font size for different blocks, the JavaScript that is
> meant to fix such mismatches, and the new nested code blocks in the PG
> 10 docs.
>
> First, the JavaScript:
>
> https://github.com/postgres/pgweb/blob/master/media/js/
> monospacefix.js
>
> There is no git history for this file except for its initial checkin in
> 2011, but I am pretty sure I wrote it.  What it does is to create 
> and  blocks, find the font point size, and compute a ratio.  If the
> ratio is not 1, , , and  blocks are adjusted in size to
> match .  The complex part is that the JavaScript conditionally
> injects CSS into the web-page to accomplish this.
>

There's more history in the pgweb-old repository. You can see that this
incarnation of the file actually came from Thom Brown:

https://git.postgresql.org/gitweb/?p=pgweb-old.git;a=commitdiff;h=c67090da83079be8f268564adb63e3f0d3379f29

Your was probably the version even before that one.



> The reason the PG 10 docs look fine on Linux Firefox is because the font
> points sizes match so no CSS is injected.  They don't match on Chrome,
> so the CSS is injected.  When the CSS hits double-embedded code blocks,
>  , it makes the font too large because it double-adjusts.
>
> The fix, as Fabien identified, is to conditionally inject additional CSS
> to be _more_ specific than the first CSS and set the font-size to a
> simple '1em' so the first CSS is not called twice.  I don't think
> 'important!' is necessary but it would be good to test this.
>
> Attached is a patch that can be applied to pgweb which should fix all of
> this.
>
>
Is there any chance we can find a way to do this witha ctual CSS selectors
and not use javascript? I realize there might not be, but have we explored
the option properly on the way the site layout looks now and with
reasonably modern browsers?

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


Re: [HACKERS] [sqlsmith] ERROR: badly formatted node string "RESTRICTINFO...

2017-04-09 Thread Tom Lane
Andreas Seltenreich  writes:
> regression=> select (select max(result) from fdw_postgres.num_result) from 
> tt0;
> ERROR:  badly formatted node string "RESTRICTINFO :clause {NULLTEST :"...
> CONTEXT:  parallel worker

Apparently, postgres_fdw is trying to store RestrictInfos in the
fdw_private field of a ForeignScan node.  That won't do; those aren't
supposed to be present in a finished plan tree, so there's no readfuncs.c
support for them (and we're not adding it).

Don't know if this is a new bug, or ancient but not previously reachable.
It seems to be nearly the inverse of the problem you found yesterday,
in which postgres_fdw was stripping RestrictInfos sooner than it really
ought to.  Apparently that whole business needs a fresh look.

regards, tom lane


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


Re: [HACKERS] [PATCH] Remove unused argument in btree_xlog_split

2017-04-09 Thread Aleksander Alekseev
Hi Robert,

> Thanks.  Please add this to the next CommitFest, as there seems to be
> no urgency (and some risk) in committing it right before feature
> freeze.

Sure. Already done [1].

[1] https://commitfest.postgresql.org/14/1097/

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature


Re: [HACKERS] pgbench - allow to store select results into variables

2017-04-09 Thread Fabien COELHO


As the variable infrastructures are pretty different between psql & pgbench 
(typed vs untyped values, sorted array vs linked list data structure, no hook 
vs 2 hooks, name spaces vs no such thing...), I have chosen the simplest 
option of just copying the name checking function and extending the lexer to 
authorize non-ascii letters, so that psql/pgbench would accept the same 
variable names with the same constraint about encodings.


See patch attached & test script.


Argh, I'm jet-lagged, wrong patch suffix... Here it is with the right 
suffix.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 1eee8dc..502d31b 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -825,6 +825,8 @@ pgbench  options  dbname
  
   Sets variable varname to a value calculated
   from expression.
+  The variable name must consist of letters (including non-Latin letters),
+  digits, and underscores.
   The expression may contain integer constants such as 5432,
   double constants such as 3.14159,
   references to variables :variablename,
diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index dc1367b..1862fe4 100644
--- a/src/bin/pgbench/exprscan.l
+++ b/src/bin/pgbench/exprscan.l
@@ -58,9 +58,9 @@ extern void expr_yyset_column(int column_no, yyscan_t yyscanner);
 %option prefix="expr_yy"
 
 /* Character classes */
-alpha			[a-zA-Z_]
+alpha			[a-zA-Z\200-\377_]
 digit			[0-9]
-alnum			[a-zA-Z0-9_]
+alnum			[A-Za-z\200-\377_0-9]
 /* {space} + {nonspace} + {newline} should cover all characters */
 space			[ \t\r\f\v]
 nonspace		[^ \t\r\f\v\n]
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 78f1e6b..85f2edb 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1020,19 +1020,35 @@ makeVariableNumeric(Variable *var)
 	return true;
 }
 
-/* check whether the name consists of alphabets, numerals and underscores. */
+/*
+ * Check whether a variable's name is allowed.
+ *
+ * We allow any non-ASCII character, as well as ASCII letters, digits, and
+ * underscore.  Keep this in sync with the definition of variable_char in
+ * psqlscan.l and psqlscanslash.l.
+ *
+ * This static function is copied from "src/bin/psql/variables.c"
+ */
 static bool
-isLegalVariableName(const char *name)
+valid_variable_name(const char *name)
 {
-	int			i;
+	const unsigned char *ptr = (const unsigned char *) name;
 
-	for (i = 0; name[i] != '\0'; i++)
+	/* Mustn't be zero-length */
+	if (*ptr == '\0')
+		return false;
+
+	while (*ptr)
 	{
-		if (!isalnum((unsigned char) name[i]) && name[i] != '_')
+		if (IS_HIGHBIT_SET(*ptr) ||
+			strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz"
+   "_0123456789", *ptr) != NULL)
+			ptr++;
+		else
 			return false;
 	}
 
-	return (i > 0);/* must be non-empty */
+	return true;
 }
 
 /*
@@ -1054,7 +1070,7 @@ lookupCreateVariable(CState *st, const char *context, char *name)
 		 * Check for the name only when declaring a new variable to avoid
 		 * overhead.
 		 */
-		if (!isLegalVariableName(name))
+		if (!valid_variable_name(name))
 		{
 			fprintf(stderr, "%s: invalid variable name: \"%s\"\n",
 	context, name);


pgbench-utf8-vars.sql
Description: application/sql

-- 
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] Remaining 2017-03 CF entries

2017-04-09 Thread Masahiko Sawada
On Sat, Apr 8, 2017 at 11:24 PM, Andres Freund  wrote:
> Hi,
>
> On 2017-04-08 13:09:13 +0900, Masahiko Sawada wrote:
>> Could you consider the item 2PC on FDW as well? It is marked as "Move
>> to Next CF" early yesterday but I'm not sure that reason..
>
> I've not moved it, but given that it was moved just before the feature
> freeze, it doesn't seem wrong to me - it's too large a patchset to just
> have been merged yesterday afternoon.  I also suspect that some more
> design discussion will be needed next cycle...
>

Thank you for comment. Yeah, I agree to push this patch  out to next
cycle. It needs more design discussion and feedback.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


-- 
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] [sqlsmith] Planner crash on foreign table join

2017-04-09 Thread Andreas Seltenreich
Tom Lane writes:

> I made the attached quick-hack patch, and found that check-world
> passes just fine with it.  That's not complete proof that we have
> no other bugs of this ilk, but it definitely supports the idea
> that we don't really need to add the overhead.  I'll just put this
> in the archives for possible future reference.
>
> (Or perhaps Andreas would like to try bashing on a copy with this
> installed.)

I certainly do :-).  SQLsmith has been fuzzing for couple hours with the
patch applied, and so far none of the assertions fired.  I'll leave the
patch on my fuzzing branch until merging becomes burdensome.

regards,
Andreas


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


[HACKERS] [sqlsmith] ERROR: badly formatted node string "RESTRICTINFO...

2017-04-09 Thread Andreas Seltenreich
Hi,

I see the above ERROR logged a lot when testing master at eef8c0069e
with a postgres_fdw around.  Below is a recipe to reproduce it on top of
the regression DB.

regards,
Andreas

create extension postgres_fdw;
create server myself foreign data wrapper postgres_fdw;
create schema fdw_postgres;
create user fdw login;
grant all on schema public to fdw;
grant all on all tables in schema public to fdw;
create user mapping for public server myself options (user 'fdw');
import foreign schema public from server myself into fdw_postgres;

set max_parallel_workers_per_gather = 8;
set parallel_setup_cost = 0;
set parallel_tuple_cost = 0;

regression=> select (select max(result) from fdw_postgres.num_result) from tt0;
ERROR:  badly formatted node string "RESTRICTINFO :clause {NULLTEST :"...
CONTEXT:  parallel worker
regression=> explain select (select max(result) from fdw_postgres.num_result) 
from tt0;
QUERY PLAN
---
 Gather  (cost=100.06..122.51 rows=1947 width=32)
   Workers Planned: 1
   InitPlan 2 (returns $1)
 ->  Result  (cost=100.05..100.06 rows=1 width=32)
   InitPlan 1 (returns $0)
 ->  Limit  (cost=100.00..100.05 rows=1 width=74)
   ->  Foreign Scan on num_result  (cost=100.00..138.64 
rows=831 width=74)
   ->  Append  (cost=0.00..22.45 rows=1145 width=0)
 ->  Parallel Seq Scan on tt0  (cost=0.00..2.04 rows=104 width=0)
 ->  Parallel Seq Scan on tt6  (cost=0.00..20.41 rows=1041 width=0)


-- 
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] recent deadlock regression test failures

2017-04-09 Thread Thomas Munro
On Sun, Apr 9, 2017 at 12:33 PM, Tom Lane  wrote:
> Kevin Grittner  writes:
>> On Sat, Apr 8, 2017 at 12:56 PM, Tom Lane  wrote:
>>> I'm imagining an API like
>>>  isolation_test_is_waiting_for(int, int[]) returns bool
>
>> Good suggestion.
>
>> Thomas, would you like to produce a patch along these lines, or
>> should I?
>
> I'll be happy to review/push if someone else does a first draft.

Ok, I'll post a new patch tomorrow.

-- 
Thomas Munro
http://www.enterprisedb.com


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


Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present

2017-04-09 Thread Michael Banck
Hi,

Am Freitag, den 24.03.2017, 19:32 +0100 schrieb Michael Banck:
> On Thu, Mar 23, 2017 at 12:41:54PM +0100, Magnus Hagander wrote:
> > On Tue, Mar 21, 2017 at 8:34 PM, Robert Haas  wrote:
> > > So I tend to think that there should always be some explicit user
> > > action to cause the creation of a slot, like --create-slot-if-needed
> > > or --create-slot=name.  That still won't prevent careless use of that
> > > option but it's less dangerous than assuming that a user who refers to
> > > a nonexistent slot intended to create it when, perhaps, they just
> > > typo'd it.
> > 
> > Well, the explicit user action would be "--slot". But sure, I can
> > definitely live with a --create-if-not-exists. 
> 
> Can we just make that option create slot and don't worry if one exists
> already? CreateReplicationSlot() can be told to not mind about already
> existing slots, and I don't see a huge point in erroring out if the slot
> exists already, unless somebody can show that this leads to data loss
> somehow.
> 
> > The important thing, to me, is that you can run it as a single
> > command, which makes it a lot easier to work with. (And not like we
> > currently have for pg_receivewal which requires a separate command to
> > create the slot)
> 
> Oh, that is how it works with pg_receivewal, I have to admit I've never
> used it so was a bit confused about this when I read its code.
> 
> So in that case I think we don't necessarily need to have the same user
> interface at all. I first thought about just adding "-C, --create" (as
> in "--create --slot=foo"), but this on second thought this looked a bit
> shortsighted - who knows what flashy thing pg_basebackup might create in
> 5 years... So I settled on --create-slot, which is only slightly more to
> type (albeit repetive, "--create-slot --slot=foo"), but adding a short
> option "-C" would be fine I thinkg "-C -S foo".
> 
> So attached is a patch with adds that option. If people really think it
> should be --create-slot-if-not-exists instead I can update the patch, of
> course.
> 
> I again added a second patch with some further refactoring which makes
> it print a message on temporary slot creation in verbose mode.

Rebased, squashed and slighly edited version attached. I've added this
to the 2017-07 commitfest now as well:

https://commitfest.postgresql.org/14/1112/


Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.ba...@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
From 47fc0d543a43441f239f109d08bfc7c082f4c091 Mon Sep 17 00:00:00 2001
From: Michael Banck 
Date: Fri, 24 Mar 2017 18:27:47 +0100
Subject: [PATCH] Add option to create a replication slot in pg_basebackup if
 not yet present.

When requesting a particular replication slot, the new pg_basebackup option
--create-slot tries to create it before starting to replicate from it in case
it does not exist already.

In order to allow reporting of temporary replication slot creation in --verbose
mode, further refactor the slot creation logic. Add new argument `is_temporary'
to CreateReplicationSlot() in streamutil.c which specifies whether a physical
replication slot is temporary or not. Update all callers. Move the creation of
the temporary replication slot for pg_basebackup from receivelog.c to
pg_basebackup.c and mention it in --verbose mode. At the same time, also create
the temporary slot via CreateReplicationSlot() instead of creating the
temporary slot with an explicit SQL command.
---
 doc/src/sgml/ref/pg_basebackup.sgml  | 12 ++
 src/bin/pg_basebackup/pg_basebackup.c| 58 ++--
 src/bin/pg_basebackup/pg_receivewal.c|  2 +-
 src/bin/pg_basebackup/pg_recvlogical.c   |  2 +-
 src/bin/pg_basebackup/receivelog.c   | 18 -
 src/bin/pg_basebackup/streamutil.c   | 18 ++---
 src/bin/pg_basebackup/streamutil.h   |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl | 23 +--
 8 files changed, 102 insertions(+), 33 deletions(-)

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index e1cec9d..65ca8dc 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -269,6 +269,18 @@ PostgreSQL documentation
  
 
  
+  -C
+  --create-slot
+  
+   
+This option can only be used together with the --slot 
+option.  It causes the specified slot name to be created if it does not
+exist already.  
+   
+  
+ 
+
+ 
   -T olddir=newdir
   --tablespace-mapping=olddir=newdir
   
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 40ec0e1..6442a54 100644
--- 

Re: [HACKERS] Making clausesel.c Smarter

2017-04-09 Thread David Rowley
On 8 April 2017 at 09:33, Claudio Freire  wrote:
> Otherwise, the patch LGTM, but I'd like to solve the quadratic
> behavior too... are you going to try? Otherwise I could take a stab at
> it myself. It doesn't seem very difficult.

I have some ideas in my head in a fairly generic way of solving this
which I'd like to take care of in PG11.

Many thanks for your reviews and suggestions on this patch, it's much
appreciated.

-- 
 David Rowley   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] Interval for launching the table sync worker

2017-04-09 Thread Masahiko Sawada
On Sat, Apr 8, 2017 at 8:13 AM, Peter Eisentraut
 wrote:
> On 4/7/17 01:10, Masahiko Sawada wrote:
>> It's not critical but it could be problem. So I thought we should fix
>> it before the PostgreSQL 10 release. If it's not appropriate as an
>> open item I'll remove it.
>
> You wrote that you "sent" a patch, but I don't see a patch anywhere.
>
> I think a nonintrusive patch for this could be considered.

Oops, I made a mistake. I'll send a patch tomorrow.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


-- 
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] Performance improvement for joins where outer side is unique

2017-04-09 Thread David Rowley
On 8 April 2017 at 14:23, Tom Lane  wrote:
> David Rowley  writes:
> [ unique_joins_2017-04-07b.patch ]
>
> It turned out that this patch wasn't as close to committable as I'd
> thought, but after a full day of whacking at it, I got to a place
> where I thought it was OK.  So, pushed.

Many thanks for taking the time to do this, and committing too!

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