Re: [HACKERS] jsonb_delete with arrays

2016-11-20 Thread Magnus Hagander
On Mon, Nov 21, 2016 at 5:05 AM, Dmitry Dolgov <9erthali...@gmail.com>
wrote:

> > On 15 November 2016 at 22:53, Magnus Hagander 
> wrote:
> > Attached is an implantation of jsonb_delete that instead of taking a
> single key to remove accepts an array of keys (it still does just keys, so
> it's using the - operator, it's not like the path delete function that also
> takes an array, but uses a different operator).
> >
> > In some simple testing of working through a real world usecases where we
> needed to delete 7 keys from jsonb data, it shows approximately a 9x
> speedup over calling the - operator multiple times. I'm guessing since we
> copy a lot less and don't have to re-traverse the structure.
>
> I wonder, is it worth it to create some sort of helper function to handle
> both
> deleting one key and deleting an array of keys (like `setPath` for
> `jsonb_set`,
> `jsonb_insert` and `jsonb_delete_path`)? At first glance it looks like
> `jsonb_delete` and `jsonb_delete_array` can reuse some code.
>
> Speaking about the performance I believe it's the same problem as here [1],
> since for each key the full jsonb will be decompressed. Looks like we need
> a
> new set of functions to read/update/delete an array of elements at once.
>
> [1]: https://www.postgresql.org/message-id/flat/1566eab8731.10193ac585742.
> 5467876610052746443%40zohocorp.com
>

It can be partially related, but the usecase itself had jsonb in memory
only and never stored on disk, so it's not the decompression itself.
Shouldn't be deep parsing either as we just copy the data over. But it's a
different angle on the same core problem, I think, which comes from the
fact that jsonb is just "one value".

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


Re: [HACKERS] Document how to set up TAP tests for Perl 5.8.8

2016-11-20 Thread Michael Paquier
On Mon, Nov 21, 2016 at 3:38 PM, Craig Ringer  wrote:
> On 21 November 2016 at 12:23, Kyotaro HORIGUCHI
>  wrote:
>> I understand you :p By the way, the new doc points to
>> http://install.perlbrew.pl and it just shows the install
>> script. It seems more puzzling..
>>
>> I suppose https://perlbrew.pl/ seems to me the primary site for
>> perbrew.
>
> Oops!
>
> Fixed in attached.

No objections to this version. It's a good idea to document that.
-- 
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] regression tests fails

2016-11-20 Thread Pavel Stehule
2016-11-21 8:13 GMT+01:00 Pavel Stehule :

>
>
> 2016-11-21 8:09 GMT+01:00 Craig Ringer :
>
>> On 21 November 2016 at 14:45, Pavel Stehule 
>> wrote:
>>
>> >  SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
>> > (array_agg(data))[1], (array_agg(data))[count(*)]
>> >   FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE
>> data
>> > ~ 'INSERT'
>> >   GROUP BY 1 ORDER BY 1;
>> >
>> > but result is sensitive on locales setting - doesn't work well with
>> czech
>> > locales.
>>
>> Simple fix here is to append COLLATE "C" after the ORDER BY.
>>
>>
>>
> it needs little bit bigger change - COLLATE cannot be used with positional
> ORDER BY
>

here is a patch

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>>
>> --
>>  Craig Ringer   http://www.2ndQuadrant.com/
>>  PostgreSQL Development, 24x7 Support, Training & Services
>>
>
>
diff --git a/contrib/test_decoding/expected/spill.out b/contrib/test_decoding/expected/spill.out
index 363e9a3..10734bd 100644
--- a/contrib/test_decoding/expected/spill.out
+++ b/contrib/test_decoding/expected/spill.out
@@ -164,7 +164,7 @@ SAVEPOINT s2;
 INSERT INTO spill_test SELECT 'serialize-subsmall-subbig--2:'||g.i FROM generate_series(2, 5001) g(i);
 RELEASE SAVEPOINT s2;
 COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
 FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
 GROUP BY 1 ORDER BY 1;
  regexp_split_to_array | count |  array_agg   |array_agg
@@ -182,7 +182,7 @@ INSERT INTO spill_test SELECT 'serialize-nested-subbig-subbig--2:'||g.i FROM gen
 RELEASE SAVEPOINT s2;
 RELEASE SAVEPOINT s1;
 COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
 FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
 GROUP BY 1 ORDER BY 1;
regexp_split_to_array| count |  array_agg   |   array_agg   
@@ -200,7 +200,7 @@ INSERT INTO spill_test SELECT 'serialize-nested-subbig-subsmall--2:'||g.i FROM g
 RELEASE SAVEPOINT s2;
 RELEASE SAVEPOINT s1;
 COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
 FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
 GROUP BY 1 ORDER BY 1;
 regexp_split_to_array | count |   array_agg|   array_agg
@@ -218,7 +218,7 @@ INSERT INTO spill_test SELECT 'serialize-nested-subsmall-subbig--2:'||g.i FROM g
 RELEASE SAVEPOINT s2;
 RELEASE SAVEPOINT s1;
 COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
 FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
 GROUP BY 1 ORDER BY 1;
 regexp_split_to_array | count |  array_agg  |   array_agg
@@ -238,7 +238,7 @@ SAVEPOINT s3;
 INSERT INTO spill_test SELECT 'serialize-nested-subbig-subbigabort-subbig-3:'||g.i FROM generate_series(5001, 1) g(i);
 RELEASE SAVEPOINT s1;
 COMMIT;
-SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
+SELECT (regexp_split_to_array(data, ':'))[4] COLLATE "C", COUNT(*), (array_agg(data))[1], (array_agg(data))[count(*)]
 FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data ~ 'INSERT'
 GROUP BY 1 ORDER BY 1;
  regexp_split_to_array | count |array_agg|array_agg 
diff --git a/contrib/test_decoding/sql/spill.sql b/contrib/test_decoding/sql/spill.sql
index 358af0b..e638cac 100644
--- a/contrib/test_decoding/sql/spill.sql
+++ b/contrib/test_decoding/sql/spill.sql

Re: [HACKERS] regression tests fails

2016-11-20 Thread Pavel Stehule
2016-11-21 8:09 GMT+01:00 Craig Ringer :

> On 21 November 2016 at 14:45, Pavel Stehule 
> wrote:
>
> >  SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
> > (array_agg(data))[1], (array_agg(data))[count(*)]
> >   FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE
> data
> > ~ 'INSERT'
> >   GROUP BY 1 ORDER BY 1;
> >
> > but result is sensitive on locales setting - doesn't work well with czech
> > locales.
>
> Simple fix here is to append COLLATE "C" after the ORDER BY.
>
>
>
it needs little bit bigger change - COLLATE cannot be used with positional
ORDER BY

Regards

Pavel


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


Re: [HACKERS] regression tests fails

2016-11-20 Thread Craig Ringer
On 21 November 2016 at 14:45, Pavel Stehule  wrote:

>  SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
> (array_agg(data))[1], (array_agg(data))[count(*)]
>   FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data
> ~ 'INSERT'
>   GROUP BY 1 ORDER BY 1;
>
> but result is sensitive on locales setting - doesn't work well with czech
> locales.

Simple fix here is to append COLLATE "C" after the ORDER BY.



-- 
 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] regression tests fails

2016-11-20 Thread Pavel Stehule
2016-11-16 5:54 GMT+01:00 Pavel Stehule :

> Hi
>
> I have a repeated problem with regress tests
>
> master, Fedora 25,
>
> running on port 50848 with PID 5548
> == creating database "regression" ==
> CREATE DATABASE
> ALTER DATABASE
> == running regression test queries==
> test ddl  ... ok
> test xact ... ok
> test rewrite  ... ok
> test toast... ok
> test permissions  ... ok
> test decoding_in_xact ... ok
> test decoding_into_rel... ok
> test binary   ... ok
> test prepared ... ok
> test replorigin   ... ok
> test time ... ok
> test messages ... ok
> test spill... FAILED
> == shutting down postmaster   ==
>
> The result is in unstable order.
>

I was wrong - there is ORDER BY clause

 SELECT (regexp_split_to_array(data, ':'))[4], COUNT(*),
(array_agg(data))[1], (array_agg(data))[count(*)]
  FROM pg_logical_slot_get_changes('regression_slot', NULL,NULL) WHERE data
~ 'INSERT'
  GROUP BY 1 ORDER BY 1;

but result is sensitive on locales setting - doesn't work well with czech
locales.

Regards

Pavel



> Regards
>
> Pavel
>


Re: [HACKERS] Fix checkpoint skip logic on idle systems by tracking LSN progress

2016-11-20 Thread Kyotaro HORIGUCHI
Hello,

At Mon, 21 Nov 2016 14:41:27 +0900, Michael Paquier  
wrote in 
> On Mon, Nov 21, 2016 at 1:31 PM, Kyotaro HORIGUCHI
>  wrote:
> > So, all my original concern were cleared.
> 
> Cool. Perhaps this could be marked as ready for committer then?

^^;

> > The last one is
> > resetting by a checkpointer restart.. I'd like to remove that if
> > Andres agrees.
> 
> Could you clarify this point? v18 makes sure that the last segment
> switch stays in shared memory so as we could still skip the activity
> of archive_timeout correctly.

I don't doubt that it works. (I don't comment on the comment:) My
concern is complexity. I don't think we wish to save almost no
harm behavior caused by a thing rarely happens.  But, if you and
others on this thread don't mind the complexity, It's not worth
asserting myself more.

So, after a day waiting, I'll mark this as ready for committer
again.

reagards,

-- 
Kyotaro Horiguchi
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] Document how to set up TAP tests for Perl 5.8.8

2016-11-20 Thread Craig Ringer
On 21 November 2016 at 12:23, Kyotaro HORIGUCHI
 wrote:
> Hello,
>
> At Mon, 21 Nov 2016 11:58:34 +0800, Craig Ringer  
> wrote in 
>> On 18 November 2016 at 08:15, Craig Ringer  wrote:
>> > On 18 November 2016 at 07:10, Michael Paquier  
>> > wrote:
>> >> On Thu, Nov 17, 2016 at 2:39 PM, Craig Ringer  
>> >> wrote:
>> >>> I wasted a bunch of time getting set up to test for such an ancient
>> >>> Perl and would love to save the next person along the hassle by
>> >>> documenting the easy way.
>> >>
>> >> It looks sensible to mention that in the README, so +1.
>
> I appreciate the labor. Anyway I don't object for adding that to
> the README. +1.

Thanks.

> I understand you :p By the way, the new doc points to
> http://install.perlbrew.pl and it just shows the install
> script. It seems more puzzling..
>
> I suppose https://perlbrew.pl/ seems to me the primary site for
> perbrew.

Oops!

Fixed in attached.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From e9a654aebd76abeadd5bc592191f8b01bce3716e Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Tue, 15 Nov 2016 15:45:41 +0800
Subject: [PATCH] Document that perl 5.8.0 is required

---
 src/test/perl/README | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/test/perl/README b/src/test/perl/README
index cfb45a1..0732cac 100644
--- a/src/test/perl/README
+++ b/src/test/perl/README
@@ -64,3 +64,21 @@ For available PostgreSQL-specific test methods and some example tests read the
 perldoc for the test modules, e.g.:
 
 perldoc src/test/perl/PostgresNode.pm
+
+Required Perl
+-
+
+Tests must run on perl 5.8.0 and newer. perlbrew is a good way to obtain
+such a Perl; see http://perlbrew.pl .
+
+Just install and
+
+perlbrew --force install 5.8.0
+perlbrew use 5.8.0
+perlbrew install-cpanm
+cpanm install IPC::Run
+
+then re-run configure to ensure the correct Perl is used when running tests. To verify
+that the right Perl was found:
+
+grep ^PERL= config.log
-- 
2.5.5


-- 
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] Floating point comparison inconsistencies of the geometric types

2016-11-20 Thread Kyotaro HORIGUCHI
Hello,

# Sorry for a trash I emitted before..

Perhaps you don't assume any calculations applied on stored
geo-type values. Please be patient to disucuss with me. (Sorry
for perhas hard-to-read English..)

At Fri, 18 Nov 2016 10:58:27 +0100, Emre Hasegeli  wrote in 

> > To keep such kind of integrity, we should deeply consider about
> > errors.
> 
> My point is that I don't think we can keep integrity together with the
> fuzzy behaviour, or at least I don't have the skills to do that.  I
> can just leave the more complicated operators like "is a
> point on a line" as it is, and only change the basic ones.  Do you
> think a smaller patch like this would be acceptable?

The size of the patch is not a matter. It broken the current
behavior is a matter. It shows strange behavior for some cases
but it designed to work reasonable for some cases.

> > That's a fate of FP numbers. Btree is uasble for such data since
> > it is constructed using inequality operators but hash is almost
> > unsuitable without rounding and normalization because of the
> > nature of floating point numbers. Range scan on bree will work
> > find but on the other hand equality doesn't work even on btree
> > index from the same reason to the below.
> 
> Btree is very useful with floats.  There is no problem with it.

I don't mind btrees on bare floats. It will 'work' if error is
properly considered, or error can be omitted for the
purpose. Point has some kind of physical context or expected
behavior other than that of bare floats which the EPSILON
implies.

> > Again, FP numbers generally cannot match exactly each other. You
> > have to avoid that.
> 
> Again, they do match very well.  Floating point numbers are no magic.
> They are perfectly deterministic.

No magic, yes, so it is always accompanied by error. If you store
points, then searching for *any of them*, it works fine. It's no
magic. If you obtained a ponit from any physical data source, say
GPS receiver, then search identical points from a table, it would
be almost impossible to find a "exactly the same" location. It's
no magic.  Once a calculation has been applied on them, it is no
longer guaranteed to work in the same way. It's also no magic.

CREATE OR REPLACE FUNCTION rotate_point (integer, float8) RETURNS void AS $$ 
UPDATE p SET x = x * cos($2) - y * sin($2), y = x * sin($2) + y * cos($2) WHERE 
i = $1 $$ LANGUAGE SQL;
ALTER TABLE p (i int, x float8, y float8);
INSERT INTO p VALUES (0, 1.0, 1.0), (1, 1.0, 1.0);
SELECT rotate_point(0, pi() * 2.0 / 3.0);
SELECT rotate_point(0, pi() * 2.0 / 3.0);
SELECT rotate_point(0, pi() * 2.0 / 3.0);
SELECT * FROM p WHERE i = 0;
 i | x | y 
---+---+---
 0 | 1 | 0.999

So

SELECT p0.x = p1.x AND p0.y = p1.y FROM p p0, p p1 WHERE p0.i = 0 and p1.i = 1;
 ?column? 
--
 f
(1 row)

This just repeats 1/3 rotation 3 times. Ideally p0 and p1 are
identical but this primitive operation generates visible error
that makes the comparison fail. Even though these two points are
expected be identical in some geometric viewpoint. This is a part
of the meaning of the EPSILON.

-- 
Kyotaro Horiguchi
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] Fix checkpoint skip logic on idle systems by tracking LSN progress

2016-11-20 Thread Michael Paquier
On Mon, Nov 21, 2016 at 1:31 PM, Kyotaro HORIGUCHI
 wrote:
> So, all my original concern were cleared.

Cool. Perhaps this could be marked as ready for committer then?

> The last one is
> resetting by a checkpointer restart.. I'd like to remove that if
> Andres agrees.

Could you clarify this point? v18 makes sure that the last segment
switch stays in shared memory so as we could still skip the activity
of archive_timeout correctly.
-- 
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] Fix checkpoint skip logic on idle systems by tracking LSN progress

2016-11-20 Thread Michael Paquier
On Tue, Nov 15, 2016 at 9:59 PM, Amit Kapila  wrote:
> On Tue, Nov 15, 2016 at 9:27 AM, Kyotaro HORIGUCHI
>  wrote:
>> The term "WAL activity' is used in the comment for
>> GetProgressRecPtr. Its meaning is not clear but not well
>> defined. Might need a bit detailed explanation about that or "WAL
>> activity tracking". What do you think about this?
>>
>
> I would have written it as below:
>
> GetProgressRecPtr -- Returns the WAL progress.  WAL progress is
> determined by scanning each WALinsertion lock by taking directly the
> light-weight lock associated to it.

Not sure if that's better.. What about something as fancy as that?
 /*
- * Get the time of the last xlog segment switch
+ * GetProgressRecPtr -- Returns the newest WAL progress position.  WAL
+ * progress is determined by scanning each WALinsertion lock by taking
+ * directly the light-weight lock associated to it.  The result of this
+ * routine can be compared with the last checkpoint LSN to check if
+ * a checkpoint can be skipped or not.
+ *
It may be worth mentioning that the result of this routine is
basically used for checkpoint skip logic.
-- 
Michael
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index adab2f8..d2a8ec2 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -2826,17 +2826,16 @@ include_dir 'conf.d'
 parameter is greater than zero, the server will switch to a new
 segment file whenever this many seconds have elapsed since the last
 segment file switch, and there has been any database activity,
-including a single checkpoint.  (Increasing
-checkpoint_timeout will reduce unnecessary
-checkpoints on an idle system.)
-Note that archived files that are closed early
-due to a forced switch are still the same length as completely full
-files.  Therefore, it is unwise to use a very short
-archive_timeout  it will bloat your archive
-storage.  archive_timeout settings of a minute or so are
-usually reasonable.  You should consider using streaming replication,
-instead of archiving, if you want data to be copied off the master
-server more quickly than that.
+including a single checkpoint.  Checkpoints can however be skipped
+if there is no database activity, making this parameter a safe
+setting for environments which are idle for a long period of time.
+Note that archived files that are closed early due to a forced switch
+are still the same length as completely full files.  Therefore, it is
+unwise to use a very short archive_timeout  it will
+bloat your archive storage.  archive_timeout settings of
+a minute or so are usually reasonable.  You should consider using
+streaming replication, instead of archiving, if you want data to
+be copied off the master server more quickly than that.
 This parameter can only be set in the
 postgresql.conf file or on the server command line.

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index b019bc1..ac40731 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2507,7 +2507,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId 
cid,
heaptup->t_len - 
SizeofHeapTupleHeader);
 
/* filtering by origin on a row level is much more efficient */
-   XLogIncludeOrigin();
+   XLogSetFlags(XLOG_INCLUDE_ORIGIN);
 
recptr = XLogInsert(RM_HEAP_ID, info);
 
@@ -2846,7 +2846,7 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, 
int ntuples,
XLogRegisterBufData(0, tupledata, totaldatalen);
 
/* filtering by origin on a row level is much more 
efficient */
-   XLogIncludeOrigin();
+   XLogSetFlags(XLOG_INCLUDE_ORIGIN);
 
recptr = XLogInsert(RM_HEAP2_ID, info);
 
@@ -3308,7 +3308,7 @@ l1:
}
 
/* filtering by origin on a row level is much more efficient */
-   XLogIncludeOrigin();
+   XLogSetFlags(XLOG_INCLUDE_ORIGIN);
 
recptr = XLogInsert(RM_HEAP_ID, XLOG_HEAP_DELETE);
 
@@ -6035,7 +6035,7 @@ heap_finish_speculative(Relation relation, HeapTuple 
tuple)
XLogBeginInsert();
 
/* We want the same filtering on this as on a plain insert */
-   XLogIncludeOrigin();
+   XLogSetFlags(XLOG_INCLUDE_ORIGIN);
 
XLogRegisterData((char *) , SizeOfHeapConfirm);
XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
@@ -7703,7 +7703,7 @@ log_heap_update(Relation reln, Buffer oldbuf,
}
 
/* filtering by origin on a row level is much more efficient */
-

Re: [HACKERS] Tuple count used while costing MergeAppend and that for an append rel

2016-11-20 Thread Ashutosh Bapat
>
> AFAICS, what you propose to add in set_append_rel_size is pure overhead.
> There's no conceivable use to computing sum-of-raw-tuple-counts for an
> appendrel ... or at least, if there is, you didn't say what you expect
> it would be good for.  Normally the difference between rel->tuples and
> rel->rows corresponds to the selectivity of the rel's restriction clauses.
> Since an appendrel has no restrictions of its own (they've all been pushed
> down to the child rels) it doesn't seem unreasonable for it to have tuples
> equal to rows.  While we could also define it as you suggest, I don't see
> the point of expending extra cycles to do so.
>

I suggested that change, just to keep the computation consistent with
the comment in RelOptInfo. I agree that it's a pure overhead for now.
If we were to maintain it per the comments, we may be able to find the
average selectivity for an appendrel, but again, I do not see any real
purpose in doing so.

> I concur that using path->rows not rel->tuples in create_merge_append_path
> would be an improvement.  I think it makes no difference now, but if we
> were to support parameterized mergeappend paths, rel->rows and rel->tuples
> would both be wrong for such a path.  (I believe the possibility of a
> parameterized path is the reason why create_append_path computes
> path->rows the hard way instead of just copying it from rel->rows.
> The logic in create_merge_append_path was probably just copied from
> create_append_path; it's overkill today but might not be so forever.)

Yes, this code did puzzle me. add_path() does not allow paths to have
both parameterization and pathkeys. So, a merge append on
parameterized paths does look odd. But then, I thought, that producing
a parameterized merge append would get allow merging ordered rows for
a given value of parameter. Although we don't do that now, but may be
it's useful in the future.

Thanks a lot for your explanation.
-- 
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] Fix checkpoint skip logic on idle systems by tracking LSN progress

2016-11-20 Thread Kyotaro HORIGUCHI
Thank you very much for the testing on the nice machine.

At Fri, 18 Nov 2016 20:35:43 -0800, Michael Paquier  
wrote in 

Re: [HACKERS] Document how to set up TAP tests for Perl 5.8.8

2016-11-20 Thread Kyotaro HORIGUCHI
Hello,

At Mon, 21 Nov 2016 11:58:34 +0800, Craig Ringer  wrote 
in 
> On 18 November 2016 at 08:15, Craig Ringer  wrote:
> > On 18 November 2016 at 07:10, Michael Paquier  
> > wrote:
> >> On Thu, Nov 17, 2016 at 2:39 PM, Craig Ringer  
> >> wrote:
> >>> I wasted a bunch of time getting set up to test for such an ancient
> >>> Perl and would love to save the next person along the hassle by
> >>> documenting the easy way.
> >>
> >> It looks sensible to mention that in the README, so +1.

I appreciate the labor. Anyway I don't object for adding that to
the README. +1.

> >> By the way, would it matter to mention ways to install perlbrew? For
> >> example, I just bumped into http://install.perlbrew.pl by looking
> >> around, though I don't doubt that most people would just use cpan with
> >> "install App::perlbrew" for example. For OSX users with brew and
> >> without macports that would matter.
> >
> > The docs linked mention it, so I figured that'd probably do. Though I
> > notice it's only mentioned in the "configuration" section not under
> > "installation", so maybe it's worth adding yeah.
> 
> Updated docs patch. Since configure checks for 5.8.0 that's what's
> specified. Anyone who wants to argue about the actual version we
> _should_ target can do so elsewhere, all I'm interested in is what we
> _do_ officially target so I can document this.

I understand you :p By the way, the new doc points to
http://install.perlbrew.pl and it just shows the install
script. It seems more puzzling..

I suppose https://perlbrew.pl/ seems to me the primary site for
perbrew.


| Perlbrew
| 
| perlbrew is an admin-free perl installation management tool. The latest 
version is 0.78, read the release note: Release 0.78.
| 
| Install, quickly
| 
| Copy & Paste this line into your terminal:
| 
| \curl -L https://install.perlbrew.pl | bash

regards,

-- 
Kyotaro Horiguchi
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] DECLARE STATEMENT setting up a connection in ECPG

2016-11-20 Thread Ideriha, Takeshi
Thank you for your comment.

Friday, November 18, 2016 4:45 AM Michael Meskes wrote :
> >  - Translate the DECLARE STATEMENT into a new function with parameters.
> >These parameters carry the information like connection_name and
> statement_name.
> >  - The function is a new function defined in the ECPG library.
> 
> Why's that? I haven't checked if the standard says anything about this and my
> memory might be wrong, but isn't "DECLARE STATEMENT" supposed to be purely
> declarative, i.e. not executed at run time?

My lack of explanation caused the confusion, sorry.
Your point is true.
"DECLARE STATEMENT" is declarative ,not function in .pgc file.

I wanted to say that in order to use the connection pointed 
by the DECLARE STATEMENT some functions like ECPGdo() would be modified or
new function would be added under the directory ecpglib/.

This modification or new function will be used to get the connection by 
statement_name.

Regards, 
Ideriha, Takeshi
Fujitsu

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

2016-11-20 Thread Craig Ringer
On 19 November 2016 at 10:04, Euler Taveira  wrote:
> On 01-09-2016 01:41, Craig Ringer wrote:
>> Here's a rebased version of my pg_recvlogical --endpos patch from the
>> 9.5 series, updated to incoroprate Álvaro's changes.
>>
> I should review this patch in the other commitfest but was swamped with
> work. The patch is almost ready but I have some points.
>
> * We usually don't include itemlist into binary options. I suggest you
> to add a new paragraph for the first item and the second one you could
> add a note;
> * I think you should add a small note explaining the 'endpos' behavior.
> Also, I think endpos could be inclusive (since recovery also has this
> behavior by default);
> * I think there is a typo in those pieces of code:
>
> +   if (endpos != InvalidXLogRecPtr && walEnd >= endpos)
>
> +   if (endpos != InvalidXLogRecPtr && cur_record_lsn > endpos)
>
> If you decide to be inclusive, it should be > otherwise >=.
>
> There could be TAP tests for pg_recvlogical but it is material for
> another patch.
>
> I'll mark this patch waiting on author for your considerations.

Thanks.

I've updated the patch for this. It's already posted on the logical
decoding timeline following thread, so I'll avoid repeating it here.

https://www.postgresql.org/message-id/CAMsr%2BYGd5dv3zPNch6BU4UXX49NJDC9m3-Y%3DV5q%3DTNcE9QgSaQ%40mail.gmail.com

I addressed the docs formatting and made endpos inclusive, and added tests.

You're right about the keepalive boundary, it should've been > not >=
. In the updated patch it's >= because endpos is now inclusive.



-- 
 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] jsonb_delete with arrays

2016-11-20 Thread Dmitry Dolgov
> On 15 November 2016 at 22:53, Magnus Hagander  wrote:
> Attached is an implantation of jsonb_delete that instead of taking a
single key to remove accepts an array of keys (it still does just keys, so
it's using the - operator, it's not like the path delete function that also
takes an array, but uses a different operator).
>
> In some simple testing of working through a real world usecases where we
needed to delete 7 keys from jsonb data, it shows approximately a 9x
speedup over calling the - operator multiple times. I'm guessing since we
copy a lot less and don't have to re-traverse the structure.

I wonder, is it worth it to create some sort of helper function to handle
both
deleting one key and deleting an array of keys (like `setPath` for
`jsonb_set`,
`jsonb_insert` and `jsonb_delete_path`)? At first glance it looks like
`jsonb_delete` and `jsonb_delete_array` can reuse some code.

Speaking about the performance I believe it's the same problem as here [1],
since for each key the full jsonb will be decompressed. Looks like we need a
new set of functions to read/update/delete an array of elements at once.

[1]:
https://www.postgresql.org/message-id/flat/1566eab8731.10193ac585742.5467876610052746443%40zohocorp.com


Re: [HACKERS] Document how to set up TAP tests for Perl 5.8.8

2016-11-20 Thread Craig Ringer
On 18 November 2016 at 08:15, Craig Ringer  wrote:
> On 18 November 2016 at 07:10, Michael Paquier  
> wrote:
>> On Thu, Nov 17, 2016 at 2:39 PM, Craig Ringer  wrote:
>>> I wasted a bunch of time getting set up to test for such an ancient
>>> Perl and would love to save the next person along the hassle by
>>> documenting the easy way.
>>
>> It looks sensible to mention that in the README, so +1.
>>
>> By the way, would it matter to mention ways to install perlbrew? For
>> example, I just bumped into http://install.perlbrew.pl by looking
>> around, though I don't doubt that most people would just use cpan with
>> "install App::perlbrew" for example. For OSX users with brew and
>> without macports that would matter.
>
> The docs linked mention it, so I figured that'd probably do. Though I
> notice it's only mentioned in the "configuration" section not under
> "installation", so maybe it's worth adding yeah.

Updated docs patch. Since configure checks for 5.8.0 that's what's
specified. Anyone who wants to argue about the actual version we
_should_ target can do so elsewhere, all I'm interested in is what we
_do_ officially target so I can document this.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From e659e76ed2a1049ce5f7428fb6a8ef9deaef3ca6 Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Tue, 15 Nov 2016 15:45:41 +0800
Subject: [PATCH] Document that perl 5.8.0 is required

---
 src/test/perl/README | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/src/test/perl/README b/src/test/perl/README
index cfb45a1..d2550a9 100644
--- a/src/test/perl/README
+++ b/src/test/perl/README
@@ -64,3 +64,21 @@ For available PostgreSQL-specific test methods and some example tests read the
 perldoc for the test modules, e.g.:
 
 perldoc src/test/perl/PostgresNode.pm
+
+Required Perl
+-
+
+Tests must run on perl 5.8.0 and newer. perlbrew is a good way to obtain
+such a Perl; see http://install.perlbrew.pl .
+
+Just install and
+
+perlbrew --force install 5.8.0
+perlbrew use 5.8.0
+perlbrew install-cpanm
+cpanm install IPC::Run
+
+then re-run configure to ensure the correct Perl is used when running tests. To verify
+that the right Perl was found:
+
+grep ^PERL= config.log
-- 
2.5.5


-- 
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] Fun fact about autovacuum and orphan temp tables

2016-11-20 Thread Michael Paquier
On Sat, Nov 19, 2016 at 2:16 PM, Michael Paquier
 wrote:
> On Fri, Nov 18, 2016 at 1:11 PM, Robert Haas  wrote:
>> That might sound adding unnecessary work just for the sake of
>> paranoia, but I don't think it is.  Failures here won't be common, but
>> since they are entirely automated there will be no human intelligence
>> available to straighten things out.  Barring considerable caution,
>> we'll just enter a flaming death spiral.
>
> Thinking more paranoid, an extra way to enter in this flaming death
> spiral is to not limit the maximum number of failures authorized when
> dropping a set of orphaned tables and transactions fail multiple
> times. This is basically not important as the relation on which the
> drop failed gets dropped from the list but failing on each one of them
> is a good way to slow down autovacuum, so putting a limit of say 10
> transactions failing is I think really important.

By the way, when hacking this patch I asked myself three questions:
1) How many objects should be dropped per transaction? 50 sounds like
a fine number to me so the patch I sent is doing so. This should
definitely not be more than the default for max_locks_per_transaction,
aka 64. Another thing to consider would be to use a number depending
on max_locks_per_transaction, like half of it.
2) How many transaction failures should autovacuum forgive? The patch
uses 10. Honestly I put that number out of thin air.
3) Should the drop of orphaned tables be done for a wraparound
autovacuum? Obviously, the answer is no. It is vital not to consume
transaction XIDs in this case. The patch I sent is dropping the
objects even for a wraparound job, that's easily switchable.
-- 
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] WAL recycle retading based on active sync rep.

2016-11-20 Thread Kyotaro HORIGUCHI
Hello,

At Fri, 18 Nov 2016 10:16:22 -0800, Andres Freund  wrote in 
<20161118181622.hklschaizwaxo...@alap3.anarazel.de>
> Hi,
> 
> On 2016-11-18 14:12:42 +0900, Kyotaro HORIGUCHI wrote:
> > We had too-early WAL recycling during a test we had on a sync
> > replication set. This is not a bug and a bit extreme case but is
> > contrary to expectation on synchronous replication.
> 
> I don't think you can expect anything else.

My sentense was inaccurate. "is contrary to *naive* expectation
on synchronous replication." But I agree to you.

> > This is because sync replication doesn't wait non-commit WALs to
> > be replicated. This situation is artificially caused with the
> > first patch attached and the following steps.
> 
> You could get that situation even if we waited for syncrep. The
> SyncRepWaitForLSN happens after delayChkpt is unset.
> 
> Additionally a syncrep connection could break for a a short while, and
> you'd loose all guarantees anyway.

I know. Replication slots are for such cases.

> > - Is this situation required to be saved? This is caused by a
> >   large transaction, spans over two max_wal_size segments, or
> >   replication stall lasts for a chackepoint period.
> 
> I very strongly think not.
> 
> 
> > - Is the measure acceptable?  For the worst case, a master
> >   crashes from WAL space exhaustion. (But such large transaction
> >   won't/shouldn't exist?)
> 
> No, imo not.

Thanks for clarifying that.

regards,

-- 
Kyotaro Horiguchi
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] [RFC] Should we fix postmaster to avoid slow shutdown?

2016-11-20 Thread Tsunakawa, Takayuki
From: Robert Haas [mailto:robertmh...@gmail.com]
> On Fri, Nov 18, 2016 at 4:12 PM, Tom Lane  wrote:
> > Alvaro Herrera  writes:
> >> Tom Lane wrote:
> >>> IMO it's not, and closer analysis says that this patch series is an
> >>> attempt to solve something we already fixed, better, in 9.4.
> >
> >> ... by the same patch submitter.
> >
> > [ confused ]  The commit log credits 82233ce7e to MauMau and yourself.
> 
> IIUC, MauMau = Tsunakawa Takayuki.

Yes, it's me.  I'm pleased that you remember me!

First, I understand that zapping the stats file during recovery can be a 
problem.  In fact, it's me who proposed adding a sentence in the manual that 
the stats file is reset after immediate shutdown.  I think addressing this 
problem is another topic in a new thread.

The reasons why I proposed this patch are:

* It happened in a highly mission-critical production system of a customer who 
uses 9.2.

* 9.4's solution is not perfect, because it wastes 5 seconds anyway, which is 
unexpected for users.  The customer's requirement includes failover within 30 
seconds, so 5 seconds can be seen as a risk.
Plus, I'm worried about the possibility that the SIGKILLed process wouldn't 
disappear if it's writing to a network storage like NFS.

* And first of all, the immediate shutdown should shut the server down 
immediately without doing anything heavy, as the name means.

So, I think this patch should also be applied to later releases.  The purpose 
of the patch in 9.4 was to avoid PostgreSQL's bug, where the ereport() in 
quickdie() gets stuck waiting for malloc()'s lock to be released.

Regards
Takayuki Tsunakawa


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


Re: [HACKERS] WAL recycle retading based on active sync rep.

2016-11-20 Thread Kyotaro HORIGUCHI
Thanks for the comment.

At Fri, 18 Nov 2016 17:06:55 +0800, Craig Ringer  
wrote in 
> > We had too-early WAL recycling during a test we had on a sync
> > replication set. This is not a bug and a bit extreme case but is
> > contrary to expectation on synchronous replication.
> 
> Isn't this prevented by using a physical replication slot?
> 
> You hint that you looked at slots but they didn't meet your needs in some
> way. I'm not sure I understood the last part.

Yes, repslot does the similar. The point was whether "Do we
expect that removal of necessary WAL doesn't occur on an active
sync replication?", with a strong doubt.

At Fri, 18 Nov 2016 10:16:22 -0800, Andres Freund  wrote in 
<20161118181622.hklschaizwaxo...@alap3.anarazel.de>
> On 2016-11-18 14:12:42 +0900, Kyotaro HORIGUCHI wrote:
> > We had too-early WAL recycling during a test we had on a sync
> > replication set. This is not a bug and a bit extreme case but is
> > contrary to expectation on synchronous replication.
> 
> I don't think you can expect anything else.

I think this is the answer for it.

regards,

-- 
堀口恭太郎

日本電信電話株式会社 NTTオープンソースソフトウェアセンタ
Phone: 03-5860-5115 / Fax: 03-5463-5490




-- 
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] Remove the comment on the countereffectiveness of large shared_buffers on Windows

2016-11-20 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit Kapila
> > shared_buffers  tps
> > 256MB  990
> > 512MB  813
> > 1GB  1189
> > 2GB  2258
> > 4GB  5003
> > 8GB  5062
> >
> > "512MB is the largest effective size" seems to be a superstition, although
> I don't know the reason for the drop at 512MB.
> >
> 
> It is difficult to say why the performance drops at 512MB, it could be
> run-to-run variation.  How long have you run each test?

5 minutes (-T 300).  I avoided 20-30 minutes runs for fear of wearing out and 
destroying my disk...

Regards
Takayuki Tsunakawa



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


Re: [HACKERS] Remove the comment on the countereffectiveness of large shared_buffers on Windows

2016-11-20 Thread Amit Kapila
On Mon, Nov 21, 2016 at 5:22 AM, Tsunakawa, Takayuki
 wrote:
> From: Tsunakawa, Takayuki/綱川 貴之
>> Thank you, I'll try the read-write test with these settings on the weekend,
>> when my PC is available.  I understood that your intention is to avoid being
>> affected by checkpointing and WAL segment creation.
>
> The result looks nice as follows.  I took the mean value of three runs.
>
> shared_buffers  tps
> 256MB  990
> 512MB  813
> 1GB  1189
> 2GB  2258
> 4GB  5003
> 8GB  5062
>
> "512MB is the largest effective size" seems to be a superstition, although I 
> don't know the reason for the drop at 512MB.
>

It is difficult to say why the performance drops at 512MB, it could be
run-to-run variation.  How long have you run each test?

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


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


Re: [HACKERS] Patch: Implement failover on libpq connect level.

2016-11-20 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Robert Haas
> On Thu, Nov 17, 2016 at 10:08 PM, Craig Ringer 
> wrote:
> > We can and probably should have both.
> >
> > If the server tells us on connect whether it's a standby or not, use that.
> >
> > Otherwise, ask it.
> >
> > That way we don't pay the round-trip cost and get the log spam when
> > talking to newer servers that send us something useful in the startup
> > packet, but we can still query it on older servers. Graceful fallback.
> >
> > Every round trip is potentially very expensive. Having libpq do them
> > unnecessarily is bad.
> 
> True, but raising the bar for this feature so that it doesn't get done is
> also bad.  It can be improved in a later patch.

I thought you are very strict about performance, so I hesitate to believe you 
forgive the additional round trip.  libpq failover is a new feature in v10, so 
I think it should provide the best user experience for v10 client+server users 
from the start.  If the concern is the time for development, then support for 
older releases can be added in a later patch.

There are still several months left for v10.  Why don't we try the best?  Could 
you share the difficulty?


Regards
Takayuki Tsunakawa



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


Re: [HACKERS] Patch: Implement failover on libpq connect level.

2016-11-20 Thread Tsunakawa, Takayuki
From: Mithun Cy [mailto:mithun...@enterprisedb.com]
> > +   {"target_server_type", "PGTARGETSERVERTYPE",
> DefaultTargetServerType, NULL,
> > +   "Target-Server-Type", "", 6,
> 
> Thanks fixed.

+   {"target_server_type", "PGTARGETSERVERTYPE", NULL, NULL,

The default value field is still NULL.


> > Please avoid adding another round trip by using a GUC_REPORTed variable
> (ParameterStatus entry).  If you want to support this libpq failover with
> >pre-10 servers, you can switch the method of determining the primary based
> on the server version.  But I don't think it's worth supporting older
> servers > at the price of libpq code complexity.
> 
> Currently there is no consensus around this. For now, making this patch
> to address failover to next primary as similar to JDBC seems sufficient
> for me.
> On next proposal of patch I think we can try to extend as you have proposed

I don't think show transaction is correct, because it's affected by 
default_transaction_read_only and ALTER DATABASE/USER SET 
transaction_read_only.  transaction_read_only is a transaction attribute, not 
the server type.  What we want to use to determine the connection target should 
be not only whether the transaction is read only, but also other attributes 
such as whether temporary tables can be used (only standby), maintenance 
commands can be executed (VACUUM and ANALYZE can run when transaction_read_only 
is on, but not on standby), etc.  And how about other DBMSs?  Do we really want 
to determine the target based on transaction_read_only while e.g. Oracle is 
based on primary/standby?

If you really want the transaction_read_only attribute for your application, 
then your app should execute "SET default_transaction_read_only = on" upon 
connection, or ALTER DATABASE/USER SET default_transaction_read_only = on in 
advance.


Regards
Takayuki Tsunakawa






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


[HACKERS] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.

2016-11-20 Thread Corey Huinker
I ran into this today:

select current_database() as current_db \gset
CREATE EXTENSION postgres_fdw;
CREATE EXTENSION
CREATE EXTENSION dblink;
CREATE EXTENSION
CREATE ROLE bob LOGIN PASSWORD 'bob';
CREATE ROLE
CREATE SERVER bob_srv FOREIGN DATA WRAPPER postgres_fdw OPTIONS ( host
'localhost', dbname :'current_db' );
CREATE SERVER
CREATE USER MAPPING FOR public SERVER bob_srv OPTIONS ( user 'bob',
password 'bob' );
CREATE USER MAPPING
SELECT *
FROM dblink('bob_srv','SELECT 1') as t(x integer);
 x
---
 1
(1 row)

ALTER SERVER bob_srv OPTIONS (updatable 'true');
ALTER SERVER
SELECT *
FROM dblink('bob_srv','SELECT 1') as t(x integer);
psql:bug_example.sql:18: ERROR:  could not establish connection
DETAIL:  invalid connection option "updatable"


Is this something we want to fix?
If so, are there any other fdw/server/user-mapping options that we don't
want to pass along to the connect string?

Steps to re-create:

bug_example.sh:#!/bin/bash
dropdb bug_example
dropuser bob
createdb bug_example
psql bug_example -f bug_example.sql


bug_example.sql:

\set ECHO all

select current_database() as current_db \gset

CREATE EXTENSION postgres_fdw;
CREATE EXTENSION dblink;
CREATE ROLE bob LOGIN PASSWORD 'bob';
CREATE SERVER bob_srv FOREIGN DATA WRAPPER postgres_fdw OPTIONS ( host
'localhost', dbname :'current_db' );
CREATE USER MAPPING FOR public SERVER bob_srv OPTIONS ( user 'bob',
password 'bob' );

SELECT *
FROM dblink('bob_srv','SELECT 1') as t(x integer);

ALTER SERVER bob_srv OPTIONS (updatable 'true');

SELECT *
FROM dblink('bob_srv','SELECT 1') as t(x integer);


Re: [HACKERS] Remove the comment on the countereffectiveness of large shared_buffers on Windows

2016-11-20 Thread Tsunakawa, Takayuki
From: Tsunakawa, Takayuki/綱川 貴之
> Thank you, I'll try the read-write test with these settings on the weekend,
> when my PC is available.  I understood that your intention is to avoid being
> affected by checkpointing and WAL segment creation.

The result looks nice as follows.  I took the mean value of three runs.

shared_buffers  tps
256MB  990
512MB  813
1GB  1189
2GB  2258
4GB  5003
8GB  5062

"512MB is the largest effective size" seems to be a superstition, although I 
don't know the reason for the drop at 512MB.


Regards
Takayuki Tsunakawa



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


Re: [HACKERS] amcheck (B-Tree integrity checking tool)

2016-11-20 Thread Robert Haas
On Sat, Nov 19, 2016 at 11:38 PM, Peter Geoghegan  wrote:
> On Sat, Nov 19, 2016 at 6:45 PM, Robert Haas  wrote:
>>> What do you think about new argument with default vs. GUC? I guess
>>> that the GUC might be a lot less of a foot-gun. We might even give it
>>> a suitably scary name, to indicate that it will make the server PANIC.
>>> (I gather that you don't care about other aspects of verbosity -- just
>>> about the ability to make amcheck PANIC in the event of an invariant
>>> violation without recompiling it.)
>>
>> Yikes.  I don't think I want to expose any kind of API that lets the
>> user PANIC the server.  A value < ERROR sounds far more reasonable
>> than a value > ERROR.
>
> In general, I don't want to get into the business of reasoning about
> how well we can limp along when there is a would-be error condition
> within amcheck. Once "the impossible" has actually occurred, it's very
> difficult to reason about what still works. Also, I actually agree
> that making it possible for the tool to force a PANIC through a
> user-visible interface is a bad idea.
>
> Maybe we should just leave it as it is -- experts can recompile the
> tool after modifying it to use an elevel that is != ERROR (the thing I
> mention about elevel < ERROR is already documented in code comments).
> If that breaks, they get to keep both halves.

OK.  If it's not reasonable to continue checking after an ERROR, then
I think ERROR is the way to go.  If somebody really doesn't like that
lack of flexibility (in either direction), they can propose a change
later for separate consideration.  That limitation is not, in my view,
a sufficient reason to hold up the patch on the table.

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


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


Re: [HACKERS] Mail thread references in commits

2016-11-20 Thread Andrew Dunstan



On 11/20/2016 11:34 AM, Magnus Hagander wrote:




I'm prepared to go along with this if there's general agreement.
However, maybe we should make it actually work first, because I
get a 404 from

http://postgr.es/m/29df5b73-d823-ade4-cb17-47142742a...@dunslane.net


which ought to be the URL for your message.  I tried the same
experiment
for some of the older messages in this thread, and those are 404 too.


Right, it hasn't been set up yet. Pending the actual outcome of this 
thread.






I think it's worth having independently of this.

cheers

andrew


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

2016-11-20 Thread Steve Singer

On Sun, 20 Nov 2016, Petr Jelinek wrote:


On 13/11/16 23:02, Steve Singer wrote:



There is one exception though:

*** 195,214 


  A conflict will produce an error and will stop the replication; it
! must be resolved manually by the user.


! The resolution can be done either by changing data on the subscriber
! so that it does not conflict with incoming change or by skipping the
! transaction that conflicts with the existing data. The transaction
! can be skipped by calling the
! 
! pg_replication_origin_advance() function
! with a node_name corresponding to the subscription name. The
! current position of origins can be seen in the
! 
! pg_replication_origin_status system view.
!   
  
  


I don't see why this needs to be removed? Maybe it could be improved but
certainly not removed?



Sorry, I was confused. I noticed that the function was missing in the patch 
and thought it was documentation for a function that you had removed from 
recent versions of the patch versus referencing a function that is already 
committed.




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

2016-11-20 Thread Petr Jelinek
On 13/11/16 23:02, Steve Singer wrote:
> On 10/31/2016 06:38 AM, Petr Jelinek wrote:
>> On 31/10/16 00:52, Steve Singer wrote:
>> There are some fundamental issues with initial sync that need to be
>> discussed on list but this one is not known. I'll try to convert this
>> to test case (seems like useful one) and fix it, thanks for the
>> report. In meantime I realized I broke the last patch in the series
>> during rebase so attached is the fixed version. It also contains the
>> type info in the protocol.
>>
> 
> Attached are some proposed documentation updates (to be applied ontop of
> your 20161031 patch set)
> 

Merged into v8, thanks!

There is one exception though:
> *** 195,214 
> 
> 
>   A conflict will produce an error and will stop the replication; it
> ! must be resolved manually by the user.
> 
> 
> ! The resolution can be done either by changing data on the subscriber
> ! so that it does not conflict with incoming change or by skipping the
> ! transaction that conflicts with the existing data. The transaction
> ! can be skipped by calling the
> ! 
> ! pg_replication_origin_advance() function
> ! with a node_name corresponding to the subscription name. The
> ! current position of origins can be seen in the
> ! 
> ! pg_replication_origin_status system 
> view.
> !   
>   
>   

I don't see why this needs to be removed? Maybe it could be improved but
certainly not removed?

> Also
> 
> 
>   Publication
> 
> 
> +  
> +The tables are matched using fully qualified table name. Renaming of
> +tables or schemas is not supported.
> +  
> 
> Is renaming of tables any less supported than other DDL operations
> For example
> 

I changed that text as it means something completely different.

> alter table nokey2 rename to nokey3
> select * FROM pg_publication_tables ;
>  pubname | schemaname | tablename
> -++---
>  tpub| public | nokey3
> (1 row)
> 
> 
> If I then kill the postmaster on my subscriber and restart it, I get
> 
> 2016-11-13 16:17:11.341 EST [29488] FATAL:  the logical replication
> target public.nokey3 not found
> 2016-11-13 16:17:11.342 EST [29272] LOG:  worker process: logical
> replication worker 41076 (PID 29488) exited with exit code 1
> 2016-11-13 16:17:16.350 EST [29496] LOG:  logical replication apply for
> subscription nokeysub started
> 2016-11-13 16:17:16.358 EST [29498] LOG:  logical replication sync for
> subscription nokeysub, table nokey2 started
> 2016-11-13 16:17:16.515 EST [29498] ERROR:  table public.nokey2 not
> found on publisher
> 2016-11-13 16:17:16.517 EST [29272] LOG:  worker process: logical
> replication worker 41076 sync 24688 (PID 29498) exited with exit code 1
> 
> but if I then rename the table on the subscriber everything seems to work.
> 
> (I suspect the need to kill+restart is a bug, I've seen other instances
> where a hard restart of the subscriber following changes to is required)
> 

This is another initial sync patch bug.


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


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


Re: [HACKERS] Mail thread references in commits

2016-11-20 Thread Vik Fearing
On 11/20/2016 03:59 PM, Andrew Dunstan wrote:
> 
> On 11/20/2016 06:55 AM, Magnus Hagander wrote:
>>
>> We can replace the website part with a http://postgr.es/m/
>> which will make it a bit shorter and still as easy to generate from
>> the client side and to search off. It'd be trivial to do, and since
>> there is no state held at the forwarder for it, it wouldn't introduce
>> a risk of "losing the forwards at some point in the future". But the
>> bulk of the URL, being the messageid, would remain.
> 
> That's 19 extra characters, less 2 if you remove the <> delimiters. I
> think the extra usability makes the slight increase in ugliness worth
> it. Can we just agree or disagree on this?

If non-committers are voting, I'll add my +1 to this.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
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] Failed assertion in parallel worker in ExecInitSubPlan

2016-11-20 Thread Andreas Seltenreich
Hi,

the query below triggers a parallel worker assertion for me when run on
the regression database of master as of 0832f2d.  The plan sports a
couple of InitPlan nodes below Gather.

regards,
Andreas

 Gather  (cost=1.64..84.29 rows=128 width=4)
   Workers Planned: 1
   Single Copy: true
   ->  Limit  (cost=1.64..84.29 rows=128 width=4)
 ->  Subquery Scan on subq_0  (cost=1.64..451.06 rows=696 width=4)
   Filter: (subq_0.c6 IS NOT NULL)
   ->  Nested Loop Left Join  (cost=1.64..444.07 rows=699 width=145)
 Join Filter: (sample_0.aa = sample_1.pageno)
 InitPlan 4 (returns $3)
   ->  Result  (cost=1.21..5.36 rows=15 width=0)
 One-Time Filter: ($0 AND ($1 = $2))
 InitPlan 1 (returns $0)
   ->  Result  (cost=0.00..0.00 rows=0 width=0)
 One-Time Filter: false
 InitPlan 2 (returns $1)
   ->  Limit  (cost=0.35..0.52 rows=1 width=4)
 ->  Gather  (cost=0.00..1.04 rows=6 
width=4)
   Workers Planned: 1
   ->  Parallel Seq Scan on reltime_tbl 
 (cost=0.00..1.04 rows=4 width=4)
 InitPlan 3 (returns $2)
   ->  Limit  (cost=0.52..0.69 rows=1 width=4)
 ->  Gather  (cost=0.00..1.04 rows=6 
width=4)
   Workers Planned: 1
   ->  Parallel Seq Scan on reltime_tbl 
reltime_tbl_1  (cost=0.00..1.04 rows=4 width=4)
 ->  Sample Scan on pg_foreign_data_wrapper 
sample_2  (cost=1.21..5.36 rows=15 width=0)
   Sampling: system ('3.1'::real)
 ->  Nested Loop  (cost=0.15..382.85 rows=699 width=4)
   ->  Sample Scan on pg_largeobject sample_1  
(cost=0.00..209.03 rows=699 width=8)
 Sampling: bernoulli ('2.9'::real)
 Filter: (pageno IS NOT NULL)
   ->  Index Only Scan using 
pg_foreign_table_relid_index on pg_foreign_table ref_0  (cost=0.15..0.24 rows=1 
width=4)
 Index Cond: (ftrelid = sample_1.loid)
 ->  Materialize  (cost=0.00..16.06 rows=4 width=4)
   ->  Append  (cost=0.00..16.04 rows=4 width=4)
 ->  Sample Scan on a sample_0  
(cost=0.00..4.01 rows=1 width=4)
   Sampling: system ('5'::real)
 ->  Sample Scan on b sample_0_1  
(cost=0.00..4.01 rows=1 width=4)
   Sampling: system ('5'::real)
 ->  Sample Scan on c sample_0_2  
(cost=0.00..4.01 rows=1 width=4)
   Sampling: system ('5'::real)
 ->  Sample Scan on d sample_0_3  
(cost=0.00..4.01 rows=1 width=4)
   Sampling: system ('5'::real)


--8<---cut here---start->8---
set force_parallel_mode to on;
set max_parallel_workers_per_gather to 2;

select
  91 as c0
from
  (select
(select pfname from public.pslot limit 1 offset 3)
   as c0,
ref_1.grandtot as c1,
(select pg_catalog.min(procost) from pg_catalog.pg_proc)
   as c2,
ref_0.ftoptions as c3,
ref_1.grandtot as c4,
sample_1.loid as c5,
pg_catalog.pg_rotate_logfile() as c6,
(select random from public.hash_i4_heap limit 1 offset 5)
   as c7,
sample_1.loid as c8
  from
public.a as sample_0 tablesample system (5)
right join pg_catalog.pg_largeobject as sample_1 tablesample 
bernoulli (2.9)
  inner join pg_catalog.pg_foreign_table as ref_0
  on (sample_1.loid = ref_0.ftrelid )
on (sample_0.aa = sample_1.pageno )
  left join public.mvtest_tvv as ref_1
  on (EXISTS (
  select
  sample_2.fdwoptions as c0,
  sample_2.fdwhandler as c1,
  (select during from public.test_range_excl limit 1 offset 89)
 as c2
from
  pg_catalog.pg_foreign_data_wrapper as sample_2 tablesample 
system (3.1)
where (EXISTS (
select
sample_3.b as c0,
(select grantee from information_schema.udt_privileges 
limit 1 offset 4)
   as c1,
sample_3.b as c2,
sample_3.rf_a as c3,
sample_3.b as c4,
 

Re: [HACKERS] How to change order sort of table in HashJoin

2016-11-20 Thread Tom Lane
Man  writes:
> Additional information.
> In 9.6 the second table (lesser tuple) was choosen (the same testdata).
> There are something (cost estimation?) different  in previous versions.

I'd bet on different statistics in the two installations (either you
forgot to ANALYZE, or the random sample came up quite a bit different).
And I'm a little suspicious that these tests weren't all done with the
same work_mem setting.

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] Mail thread references in commits

2016-11-20 Thread Magnus Hagander
On Sun, Nov 20, 2016 at 5:26 PM, Tom Lane  wrote:

> Andrew Dunstan  writes:
> > On 11/20/2016 06:55 AM, Magnus Hagander wrote:
> >> We can replace the website part with a http://postgr.es/m/
> >> which will make it a bit shorter and still as easy to generate from
> >> the client side and to search off.
>
> > That's 19 extra characters, less 2 if you remove the <> delimiters. I
> > think the extra usability makes the slight increase in ugliness worth
> > it. Can we just agree or disagree on this?
>
> I'm prepared to go along with this if there's general agreement.
> However, maybe we should make it actually work first, because I
> get a 404 from
>
> http://postgr.es/m/29df5b73-d823-ade4-cb17-47142742a...@dunslane.net
>
> which ought to be the URL for your message.  I tried the same experiment
> for some of the older messages in this thread, and those are 404 too.
>

Right, it hasn't been set up yet. Pending the actual outcome of this thread.



> BTW, should it be "http:" or "https:"?
>

As of today both should work, so it should be https.

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


Re: [HACKERS] Mail thread references in commits

2016-11-20 Thread Tom Lane
Andrew Dunstan  writes:
> On 11/20/2016 06:55 AM, Magnus Hagander wrote:
>> We can replace the website part with a http://postgr.es/m/ 
>> which will make it a bit shorter and still as easy to generate from 
>> the client side and to search off.

> That's 19 extra characters, less 2 if you remove the <> delimiters. I 
> think the extra usability makes the slight increase in ugliness worth 
> it. Can we just agree or disagree on this?

I'm prepared to go along with this if there's general agreement.
However, maybe we should make it actually work first, because I
get a 404 from

http://postgr.es/m/29df5b73-d823-ade4-cb17-47142742a...@dunslane.net

which ought to be the URL for your message.  I tried the same experiment
for some of the older messages in this thread, and those are 404 too.

BTW, should it be "http:" or "https:"?

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: Implement failover on libpq connect level.

2016-11-20 Thread Mithun Cy
On Fri, Nov 18, 2016 at 6:39 AM, Tsunakawa, Takayuki <
tsunakawa.ta...@jp.fujitsu.com> wrote:
 >Typo.   , and "observering" -> "observing".

Thanks fixed.

> +   {"target_server_type", "PGTARGETSERVERTYPE",
DefaultTargetServerType, NULL,
> +   "Target-Server-Type", "", 6,

Thanks fixed.

> Please avoid adding another round trip by using a GUC_REPORTed variable
(ParameterStatus entry).  If you want to support this libpq failover with
>pre-10 servers, you can switch the method of determining the primary based
on the server version.  But I don't think it's worth supporting older
servers > at the price of libpq code complexity.

Currently there is no consensus around this. For now, making this patch to
address failover to next primary as similar to JDBC seems sufficient for me.
On next proposal of patch I think we can try to extend as you have proposed

>Please consider supporting "standby" and "prefer_standby" like PgJDBC.
They are useful without load balancing when multiple standbys are used for
HA.

I think they become more relevant with load-balancing. And, making it
usable when we extend this feature to have load-balancing makes sense to
me.

> I haven't tracked the progress of logical replication, but will
target_server_type be likely to be usable with it?  How will
target_server_type fit logical   > replication?

I tried to check logical replication WIP patch, not very sure how to
accomodate same.

-- 
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com


failover_to_new_master_v3.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] Mail thread references in commits

2016-11-20 Thread Magnus Hagander
On Sun, Nov 20, 2016 at 3:59 PM, Andrew Dunstan  wrote:

>
>
> On 11/20/2016 06:55 AM, Magnus Hagander wrote:
>
>>
>>
>> We can replace the website part with a http://postgr.es/m/
>> which will make it a bit shorter and still as easy to generate from the
>> client side and to search off. It'd be trivial to do, and since there is no
>> state held at the forwarder for it, it wouldn't introduce a risk of "losing
>> the forwards at some point in the future". But the bulk of the URL, being
>> the messageid, would remain.
>>
>>
>>
>>
>
> That's 19 extra characters, less 2 if you remove the <> delimiters. I
> think the extra usability makes the slight increase in ugliness worth it.
> Can we just agree or disagree on this?
>

I personally don't really care either way, just wanted people to know that
this option is available. (And it would be without the <> characters, of
course). I doubt it's worth the trouble to do it if we're keeping the full
msgid.

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


Re: [HACKERS] Mail thread references in commits

2016-11-20 Thread Andrew Dunstan



On 11/20/2016 06:55 AM, Magnus Hagander wrote:



We can replace the website part with a http://postgr.es/m/ 
which will make it a bit shorter and still as easy to generate from 
the client side and to search off. It'd be trivial to do, and since 
there is no state held at the forwarder for it, it wouldn't introduce 
a risk of "losing the forwards at some point in the future". But the 
bulk of the URL, being the messageid, would remain.







That's 19 extra characters, less 2 if you remove the <> delimiters. I 
think the extra usability makes the slight increase in ugliness worth 
it. Can we just agree or disagree on this?


cheers

andrew


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


[HACKERS] [patch] psql tab completion for ALTER DEFAULT PRIVILEGES

2016-11-20 Thread Gilles Darold
Hi,

When tab-completing after ALTER DEFAULT PRIVILEGES ... GRANT|REVOKE,
currently psql injects completion from the GRANT|REVOKE order, rather
than the one expected.

A patch is attached. It adds the right completion to GRANT|REVOKE after
ALTER DEFAULT PRIVILEGES and after FOR ROLE|USER + IN SCHEMA.

If there's no objection I will add it to next commit fest.

Best regards,

-- 
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b556c00..d0c8eda 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1574,9 +1574,23 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER DEFAULT PRIVILEGES FOR */
 	else if (Matches4("ALTER", "DEFAULT", "PRIVILEGES", "FOR"))
 		COMPLETE_WITH_LIST2("ROLE", "USER");
-	/* ALTER DEFAULT PRIVILEGES { FOR ROLE ... | IN SCHEMA ... } */
-	else if (Matches6("ALTER", "DEFAULT", "PRIVILEGES", "FOR", "ROLE|USER", MatchAny) ||
-		Matches6("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA", MatchAny))
+	else if (Matches4("ALTER", "DEFAULT", "PRIVILEGES", "IN"))
+		COMPLETE_WITH_CONST("SCHEMA");
+	/* ALTER DEFAULT PRIVILEGES FOR ROLE ... */
+	else if (Matches6("ALTER", "DEFAULT", "PRIVILEGES", "FOR", "ROLE|USER",
+MatchAny))
+		COMPLETE_WITH_LIST3("GRANT", "REVOKE", "IN SCHEMA");
+	/* ALTER DEFAULT PRIVILEGES IN SCHEMA ... */
+	else if (Matches6("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA",
+MatchAny))
+		COMPLETE_WITH_LIST4("GRANT", "REVOKE", "FOR ROLE", "FOR USER");
+	else if (Matches7("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA",
+MatchAny, "FOR"))
+		COMPLETE_WITH_LIST2("ROLE", "USER");
+	else if (Matches9("ALTER", "DEFAULT", "PRIVILEGES", "FOR", "ROLE|USER",
+	MatchAny, "IN", "SCHEMA", MatchAny) ||
+		Matches9("ALTER", "DEFAULT", "PRIVILEGES", "IN", "SCHEMA",
+	MatchAny, "FOR", "ROLE|USER", MatchAny))
 		COMPLETE_WITH_LIST2("GRANT", "REVOKE");
 	/* ALTER DOMAIN  */
 	else if (Matches3("ALTER", "DOMAIN", MatchAny))
@@ -2541,10 +2555,18 @@ psql_completion(const char *text, int start, int end)
 	else if (TailMatches2("FOREIGN", "SERVER"))
 		COMPLETE_WITH_QUERY(Query_for_list_of_servers);
 
-/* GRANT && REVOKE --- is allowed inside CREATE SCHEMA, so use TailMatches */
+/* GRANT && REVOKE --- is allowed inside CREATE SCHEMA and
+   ALTER DEFAULT PRIVILEGES, so use TailMatches */
 	/* Complete GRANT/REVOKE with a list of roles and privileges */
 	else if (TailMatches1("GRANT|REVOKE"))
-		COMPLETE_WITH_QUERY(Query_for_list_of_roles
+		/* With ALTER DEFAULT PRIVILEGES, restrict completion
+			to authorized keywords */
+		if (HeadMatches3("ALTER","DEFAULT","PRIVILEGES"))
+			COMPLETE_WITH_LIST10("SELECT", "INSERT", "UPDATE",
+"DELETE", "TRUNCATE", "REFERENCES", "TRIGGER",
+		"EXECUTE", "USAGE", "ALL");
+		else
+			COMPLETE_WITH_QUERY(Query_for_list_of_roles
 			" UNION SELECT 'SELECT'"
 			" UNION SELECT 'INSERT'"
 			" UNION SELECT 'UPDATE'"
@@ -2585,7 +2607,12 @@ psql_completion(const char *text, int start, int end)
 	 * privilege.
 	 */
 	else if (TailMatches3("GRANT|REVOKE", MatchAny, "ON"))
-		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsvmf,
+		/* With ALTER DEFAULT PRIVILEGES, restrict completion
+			to authorized keywords */
+		if (HeadMatches3("ALTER","DEFAULT","PRIVILEGES"))
+			COMPLETE_WITH_LIST4("TABLES", "SEQUENCES", "FUNCTIONS", "TYPES");
+		else
+			COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tsvmf,
    " UNION SELECT 'ALL FUNCTIONS IN SCHEMA'"
    " UNION SELECT 'ALL SEQUENCES IN SCHEMA'"
    " UNION SELECT 'ALL TABLES IN SCHEMA'"
@@ -2648,7 +2675,8 @@ psql_completion(const char *text, int start, int end)
 	else if ((HeadMatches1("GRANT") && TailMatches1("TO")) ||
 			 (HeadMatches1("REVOKE") && TailMatches1("FROM")))
 		COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles);
-
+	else if (HeadMatches3("ALTER","DEFAULT", "PRIVILEGES") && TailMatches1("TO|FROM"))
+		COMPLETE_WITH_QUERY(Query_for_list_of_grant_roles);
 	/* Complete "GRANT/REVOKE ... ON * *" with TO/FROM */
 	else if (HeadMatches1("GRANT") && TailMatches3("ON", MatchAny, MatchAny))
 		COMPLETE_WITH_CONST("TO");

-- 
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] Parallel worker crash on seqscan

2016-11-20 Thread Andreas Seltenreich
Hi,

the following query appears to reliably crash parallel workers on master
as of 0832f2d.

--8<---cut here---start->8---
set max_parallel_workers_per_gather to 2;
set force_parallel_mode to 1;

select subq.context from pg_settings,
lateral (select context from pg_opclass limit 1) as subq
limit 1;
--8<---cut here---end--->8---

Backtrace of a worker below.

regards,
Andreas

Core was generated by `postgres: bgworker: parallel worker for PID 27448'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  MakeExpandedObjectReadOnlyInternal (d=0) at expandeddatum.c:100
100 if (!VARATT_IS_EXTERNAL_EXPANDED_RW(DatumGetPointer(d)))
(gdb) bt
#0  MakeExpandedObjectReadOnlyInternal (d=0) at expandeddatum.c:100
#1  0x563b0c9a4e98 in ExecTargetList (tupdesc=, 
isDone=0x7ffdd20400ac, itemIsDone=0x563b0e6a8b50, isnull=0x563b0e6a8ae0 "", 
values=0x563b0e6a8ac0, econtext=0x563b0e6a7db8, targetlist=0x563b0e6a8498) at 
execQual.c:5491
#2  ExecProject (projInfo=projInfo@entry=0x563b0e6a8368, 
isDone=isDone@entry=0x7ffdd20400ac) at execQual.c:5710
#3  0x563b0c9a514f in ExecScan (node=node@entry=0x563b0e6a8038, 
accessMtd=accessMtd@entry=0x563b0c9bc910 , 
recheckMtd=recheckMtd@entry=0x563b0c9bc900 ) at execScan.c:220
#4  0x563b0c9bc9c3 in ExecSeqScan (node=node@entry=0x563b0e6a8038) at 
nodeSeqscan.c:127
#5  0x563b0c99d6e8 in ExecProcNode (node=node@entry=0x563b0e6a8038) at 
execProcnode.c:419
#6  0x563b0c9995be in ExecutePlan (dest=0x563b0e67da40, 
direction=, numberTuples=0, sendTuples=, 
operation=CMD_SELECT, use_parallel_mode=, 
planstate=0x563b0e6a8038, estate=0x563b0e6a77f8) at execMain.c:1567
#7  standard_ExecutorRun (queryDesc=0x563b0e6a2828, direction=, 
count=0) at execMain.c:338
#8  0x563b0c99c911 in ParallelQueryMain (seg=, 
toc=0x7f55173aa000) at execParallel.c:745
#9  0x563b0c898b02 in ParallelWorkerMain (main_arg=) at 
parallel.c:1108
#10 0x563b0ca49cad in StartBackgroundWorker () at bgworker.c:726
#11 0x563b0ca55770 in do_start_bgworker (rw=0x563b0e621080) at 
postmaster.c:5535
#12 maybe_start_bgworker () at postmaster.c:5710
#13 0x563b0ca56238 in sigusr1_handler (postgres_signal_arg=) 
at postmaster.c:4959
#14 
#15 0x7f5516788293 in __select_nocancel () at 
../sysdeps/unix/syscall-template.S:84
#16 0x563b0c818249 in ServerLoop () at postmaster.c:1665
#17 0x563b0ca577e2 in PostmasterMain (argc=3, argv=0x563b0e5fa490) at 
postmaster.c:1309
#18 0x563b0c819f6d in main (argc=3, argv=0x563b0e5fa490) at main.c:228


-- 
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] Mail thread references in commits

2016-11-20 Thread Magnus Hagander
On Sat, Nov 19, 2016 at 5:38 PM, Tom Lane  wrote:

> Andrew Dunstan  writes:
> > On 11/19/2016 10:11 AM, Stephen Frost wrote:
> >> That's actually not the case if we use a hash, because the client could
> >> figure out what the hash is before sending it.
>
> > The fact that it could possibly be done is not a good reason for doing
> > it.
>
> Quite.  I will *not* go along with any proposal that requires me to
> duplicate some hashing logic being done elsewhere.  Way too much risk
> of error there.
>

SHA-1 is fairly well defined, but sure, I can see your issue -- dealing
with leading spaces/trailing spaces and whatnot can certainly cause trouble.



> > Every proposal along these lines strikes me as making more unobvious
> > hoops for people to jump through. We're just reinventing the wheel here
> > because we don't like the color of the standard wheels that some email
> > MUAs produce.
>
> Yeah, anything like this would be going very very far out of our way to
> keep these lines in commit messages under an arbitrary limit (which we're
> already exceeding anyway in many cases).  I'm inclined to think we should
> just straight up decide that having the message link be a clickable URL is
> worth the longer lines, or that it is not.  Personally I vote for "not",
> but I recognize that I might be in the minority.
>
> BTW, before we give up completely on reconciling the two objectives,
> is anyone aware of a line-continuation convention that would be likely
> to work?  If we could do something like
>
> Discussion: https://www.postgresql.org/message-id/\
> caf4au4w6rrh_j1bvvhzposrihcog7sgj3lsx0ty8zdwhht8...@mail.gmail.com



I don't think there is such a thing. There is continuation at the mail
header line level, but not for an URL in the contents of the message. And
this is in the commit message itself, so nobody really knows what the
viewer will be using -- it could be our list archives or a local copy of
the commit mail, but it could also be the git log (viewed locally, through
our gitweb site, or through github or some other such site).

We can put just the message-id in there and hack our gitweb instance to
know how to turn that into links on our site. But that would only cover
*our* gitweb, and not all the other potential ways to view it. I'm pretty
sure there is no standard that will work across all the different potential
views.

We can replace the website part with a http://postgr.es/m/ which
will make it a bit shorter and still as easy to generate from the client
side and to search off. It'd be trivial to do, and since there is no state
held at the forwarder for it, it wouldn't introduce a risk of "losing the
forwards at some point in the future". But the bulk of the URL, being the
messageid, would remain.


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


Re: [HACKERS] How to change order sort of table in HashJoin

2016-11-20 Thread Man

Thanks for response, sir.

On 11/20/2016 1:18 AM, Tom Lane wrote:

Man Trieu  writes:

As in the example below, i think the plan which hash table is created on
testtbl2 (the fewer tuples) should be choosen.

The planner usually prefers to hash on the table that has a flatter
MCV histogram, since a hash table with many key collisions will be
inefficient.  You might find it illuminating to read the comments around
estimate_hash_bucketsize().


Thanks, I will read it.

Additional information.
In 9.6 the second table (lesser tuple) was choosen (the same testdata).
There are something (cost estimation?) different  in previous versions.

--- In 9.6.1 ---
postgres=# explain analyze select * from testtbl1 inner join testtbl2 
using(c1,c2,c3);

 QUERY PLAN

-
 Hash Join  (cost=6935.57..60389.58 rows=1 width=60) (actual 
time=80.214..1165.762 rows=142857 loops=1)
   Hash Cond: ((testtbl1.c1 = testtbl2.c1) AND (testtbl1.c2 = 
testtbl2.c2) AND (testtbl1.c3 = testtbl2.c3))
   ->  Seq Scan on testtbl1  (cost=0.00..21276.00 rows=100 
width=56) (actual time=0.038..226.324 rows=100 loops=1)
   ->  Hash  (cost=3039.57..3039.57 rows=142857 width=56) (actual 
time=79.632..79.632 rows=142857 loops=1)

 Buckets: 65536  Batches: 4  Memory Usage: 3658kB
 ->  Seq Scan on testtbl2  (cost=0.00..3039.57 rows=142857 
width=56) (actual time=0.028..20.646 rows=142857 loops=1)

 Planning time: 0.252 ms
 Execution time: 1174.588 ms
(8 rows)
--

--- In 9.4.10 ---
postgres=# explain analyze select * from testtbl1 inner join testtbl2 
using(c1,c2,c3);

   QUERY PLAN

-
 Hash Join  (cost=48542.00..67353.86 rows=1 width=60) (actual 
time=880.580..1277.611 rows=142857 loops=1)
   Hash Cond: ((testtbl2.c1 = testtbl1.c1) AND (testtbl2.c2 = 
testtbl1.c2) AND (testtbl2.c3 = testtbl1.c3))
   ->  Seq Scan on testtbl2  (cost=0.00..3039.57 rows=142857 width=56) 
(actual time=0.016..24.421 rows=142857 loops=1)
   ->  Hash  (cost=21276.00..21276.00 rows=100 width=56) (actual 
time=878.296..878.296 rows=100 loops=1)

 Buckets: 8192  Batches: 32  Memory Usage: 2839kB
 ->  Seq Scan on testtbl1  (cost=0.00..21276.00 rows=100 
width=56) (actual time=0.025..258.193 rows=100 loops=1)

 Planning time: 2.683 ms
 Execution time: 1285.868 ms
(8 rows)
--


In general, given a hashtable that fits in memory and light bucket
loading, a hash join is more or less O(M) + O(N); it doesn't matter
so much whether the larger table is on the inside.  It does matter if
the table gets big enough to force batching of the join, but that's
not happening in your example (at least not the first one; it's unclear
to me why it did happen in the second one).  The key thing that will
drive the choice, then, is avoiding a skewed bucket distribution that
causes lots of comparisons for common values.

regards, tom lane


Thanks and best regards,


--
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] pgpassfile connection option

2016-11-20 Thread Fabien COELHO


Hello Julian,

I've updated my patch to work with the changes introduced to libpq by 
allowing multiple hosts.


Ok. Patch applies cleanly, compiles & checks (although yet again the 
feature is not tested).


Feature tested and works for me, although I'm not sure how the multi-host 
warning about pgpassfile works, but I could not find a wrong warning.


Independently of your patch, while testing I concluded that the multi-host 
feature and documentation should be improved:


 - If a connection fails, it does not say for which host/port.

  sh> PGPASSFILE=$HOME/.pgpass.test \
  LD_LIBRARY_PATH=$PWD/src/interfaces/libpq \
  psql -d "host=host1,host2,host3 user=test dbname=test"
  psql: FATAL:  password authentication failed for user "test"
  password retrieved from file "$HOME/.pgpass.test"

 - doc says "If multiple host names are specified, each will be tried in
   turn in the order given.".

  In fact they are tried in turn if the network connection fails, but not
  if the connection fails for some other reason such as the auth.
  The documentation is not precise enough.

On Fabien's recommendations, I've kept the variable dot_pgpassfile_used, 
however I renamed that to pgpassfile_used, to keep a consistent naming 
scheme.


Ok.

I'm still not sure about the amount of error messages produced by libpq, 
I think it would be ideal to report an error while accessing a file 
provided in the connection string, however I would not report that same 
type of error when the .pgpass file has been tried to retrieve a 
password. (Else, you'd get an error message on every connection string 
that doesn't specify a pgpassfile or password, since .pgpass will be 
checked every time, before prompting the user to input the password)


Yep. This issue is independent of your patch as it is already there.
There could be a boolean set when the pass file is explicitely provided 
that could trigger a warning only in this case.


A few detailed comments:

Beware of spacing:
 . "if(" -> "if (" (2 instances)
 . " =='\0')" -> " == '\0')" (at least one instance)

Elsewhere:

 + if (conn->pgpassfile_used && conn->password_needed && conn->result &&
 + conn->pgpassfile && conn->pgpassfile[0]!='\0' &&

ISTM that if pgpassfile_used is true then pgpassfile is necessarily 
defined, so the second line tests are redundant? Or am I missing 
something?


--
Fabien.


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