Re: [HACKERS] pgbench - minor fix for meta command only scripts

[Heikki Linnakangas]

On 09/29/2017 08:43 PM, Fabien COELHO wrote:

reality.  So I don't know if this needs backpatching or not.  But it
should be fixed for v10, as there it becomes a demonstrably live issue.


Yes.


Patch looks good to me, so committed to master and v10. Thanks!

- Heikki


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

Re: [HACKERS] Bug with pg_basebackup and 'shared' tablespace

[Mark Kirkwood]

On 30/09/17 06:43, Robert Haas wrote:

On Fri, Sep 29, 2017 at 2:06 AM, Michael Paquier
 wrote:

My tendency about this patch is still that it should be rejected. This
is presenting additional handling for no real gain.

I vehemently disagree.  If the server lets you create a tablespace,
then everything that happens after that ought to work.

On another thread, there is the issue that if you create a tablespace
inside $PGDATA, things break.  We should either unbreak those things
or not allow creating the tablespace in the first place.  On this
thread, there is the issue that if you create two tablespaces for
different PG versions in the same directory, things break.  We should
either unbreak those things or not allow creating the tablespace in
the first place.

It is completely awful behavior to let users do things and then punish
them later for having done them.  Users are not obliged to read the
minds of the developers and guess what things the developers consider
"reasonable".  They should be able to count on the principle that if
they do something that we consider wrong, they'll get an error when
they try to do it -- not have it superficially appear to work and then
explode later.

To put that another way, there should be ONE rule about what is or is
not allowable in a particular situation, and all commands, utilities,
etc. that deal with that situation should handle it in a uniform
fashion.  Each .c file shouldn't get to make up its own notion of what
is or is not supported.



+1

It seems simply wrong (and potentially dangerous) to allow users to 
arrange a system state that cannot be backed up (easily/without surgery 
etc etc).


Also the customer concerned that did exactly that started the 
conversation about it with me like this (paraphrasing) 'So this 
pg_basebackup thing is a bit temperamental...'. I'm thinking we do not 
want to be giving users this impression.


regards

Mark


--
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] extension build issue with PostgreSQL 10 on Centos6

[Pavel Stehule]

2017-10-01 4:55 GMT+02:00 Devrim Gündüz :

>
> Hi,
>
> On Sat, 2017-09-30 at 11:15 -0400, Tom Lane wrote:
> > So the point is that postgresql-devel now needs to have a dependency
> > on icu-devel.
>
> Oh, I see. Ack, added. Will appear in 10.0 RPMs.
>

Thank you

Pavel

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

Re: [HACKERS] extension build issue with PostgreSQL 10 on Centos6

[Devrim Gündüz]

Hi,

On Sat, 2017-09-30 at 11:15 -0400, Tom Lane wrote:
> So the point is that postgresql-devel now needs to have a dependency
> on icu-devel.

Oh, I see. Ack, added. Will appear in 10.0 RPMs.

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


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

Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

[Peter Geoghegan]

On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera  wrote:
> Maybe what this means is that we need to do both Dan's initially
> proposed patch (or something related to it) apart from the fixes already
> pushed.  IOW we need to put back some of the "tupkeep" business ...

I took the time to specifically check if that would fix the problem.
Unfortunately, it did not. We see exactly the same problem, or at
least amcheck/REINDEX produces exactly the same error. I checked both
Dan's original update_freeze.patch, and your revision that retained
some of the "tupkeep" stuff,
0002-Don-t-freeze-recently-dead-HOT-tuples, which you posted on
September 6th.

-- 
Peter Geoghegan


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

Re: [HACKERS] 64-bit queryId?

[Andres Freund]

On 2017-09-30 18:17:37 -0400, Robert Haas wrote:
> On Sat, Sep 30, 2017 at 11:55 AM, Andres Freund  wrote:
> > On 2017-09-30 11:50:08 -0400, Robert Haas wrote:
> >> Well, I think that the fact that pg_stat_statements.max exists at all
> >> is something that could be fixed now that we have DSA.
> >
> > You normally *do* want a limit imo. And given that query strings are now
> > stored externally, I don't think there's a huge space concern anymore?
> 
> Well, that's probably true.  I guess you don't want your query
> generator to run the system out of memory.

Hah. I'm just pondering running sqlsmith against a pg without such a
limit.


-- 
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] 64-bit queryId?

[Robert Haas]

On Sat, Sep 30, 2017 at 11:55 AM, Andres Freund  wrote:
> On 2017-09-30 11:50:08 -0400, Robert Haas wrote:
>> Well, I think that the fact that pg_stat_statements.max exists at all
>> is something that could be fixed now that we have DSA.
>
> You normally *do* want a limit imo. And given that query strings are now
> stored externally, I don't think there's a huge space concern anymore?

Well, that's probably true.  I guess you don't want your query
generator to run the system out of memory.

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


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

[HACKERS] why subplan is 10x faster then function?

[Pavel Stehule]

Hi

I have some strange slow queries based on usage "view" functions

one function looks like this:

CREATE OR REPLACE FUNCTION
ides_funcs.najdatsplt_cislo_exekuce(mid_najdatsplt bigint)
 RETURNS character varying
 LANGUAGE sql
 STABLE
AS $function$
select CISLOEXEKUCE
  from najzalobpr MT, najvzallok A1,
NAJZALOBST A2, NAJZALOBCE A3 where
MT.ID_NAJVZALLOK= A1.ID_NAJVZALLOK AND
A1.ID_NAJZALOBST=A2.ID_NAJZALOBST AND
A2.ID_NAJZALOBCE= A3.ID_NAJZALOBCE AND
MT.ID_NAJDATSPLT = mID_NAJDATSPLT  LIMIT 1;
$function$ cost 20
;

I know so using this kind of functions is not good idea - it is customer
old code generated from Oracle. I had idea about possible planner issues.
But this is a executor issue.

when this function is evaluated as function, then execution needs about 46
sec

->  Nested Loop Left Join  (cost=0.71..780360.31 rows=589657
width=2700) (actual time=47796.588..47796.588 rows=0 loops=1)
  ->  Nested Loop  (cost=0.29..492947.20 rows=589657 width=2559)
(actual time=47796.587..47796.587 rows=0 loops=1)
->  Seq Scan on najdatsplt mt  (cost=0.00..124359.24
rows=1106096 width=1013) (actual time=47796.587..47796.587 rows=0 loops=1)
  Filter: (najdatsplt_cislo_exekuce(id_najdatsplt) IS
NOT NULL)
  Rows Removed by Filter: 654

When I use correlated subquery, then

 ->  Nested Loop  (cost=0.29..19876820.11 rows=589657 width=2559) (actual
time=3404.154..3404.154 rows=0 loops=1)
  ->  Seq Scan on najdatsplt mt  (cost=0.00..19508232.15 rows=1106096
width=1013) (actual time=3404.153..3404.153 rows=0 loops=1)
  Filter: ((SubPlan 11) IS NOT NULL)
  Rows Removed by Filter: 654
  SubPlan 11
->  Limit  (cost=1.10..17.49 rows=1 width=144) (actual
time=0.002..0.002 rows=0 loops=654)
  ->  Nested Loop  (cost=1.10..17.49 rows=1 width=144) (actual
time=0.002..0.002 rows=0 loops=654)
->  Nested Loop  (cost=0.83..17.02 rows=1 width=8)
(actual time=0.002..0.002 rows=0 loops=654)
  ->  Nested Loop  (cost=0.56..16.61 rows=1
width=8) (actual time=0.002..0.002 rows=0 loops=654)

The execution plan is +/- same - the bottleneck is in function execution

Tested with same result on 9.6, 10.

Is known overhead of function execution?

Regards

Pavel

Re: [HACKERS] alter server for foreign table

[Tom Lane]

Andrew Dunstan  writes:
> I assume the proposal is to allow changing to a different server using
> the same FDW. I can see all sorts of odd things happening if we allow
> changing to a server of a different FDW.

As long as we check that the table's FDW options are acceptable to the
new FDW, where's the problem?  I could see there being an issue if we
provided any hook whereby FDW could have a say during CREATE FOREIGN
TABLE (or DROP FOREIGN TABLE), but we don't.  So as far as the FDWs
are concerned, this should be little different from dropping the foreign
table and then remaking it attached to the new server.

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] Causal reads take II

[Thomas Munro]

On Sun, Oct 1, 2017 at 9:05 AM, Dmitry Dolgov <9erthali...@gmail.com> wrote:
>>> LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211/47732":
>>> Directory not empty
>>> ...
>>
>> Hmm.  The first error ("could not remove directory") could perhaps be
>> explained by temporary files from concurrent backends.
>> ...
>> Perhaps in your testing you accidentally copied a pgdata directory over
>> the
>> top of it while it was running?  In any case I'm struggling to see how
>> anything in this patch would affect anything at the REDO level.
>
> Hmm...no, I don't think so. Basically what I was doing is just running
> `installcheck` against a primary instance (I assume there is nothing wrong
> with
> this approach, am I right?). This particular error was caused by
> `tablespace`
> test which was failed in this case:
>
> ```
> INSERT INTO testschema.foo VALUES(1);
> ERROR:  could not open file "pg_tblspc/16388/PG_11_201709191/16386/16390":
> No such file or directory
> ```
>
> I tried few more times, and I've got it two times from four attempts on a
> fresh
> installation (when all instances were on the same machine). But anyway I'll
> try
> to investigate, maybe it has something to do with my environment.
>
> ...
>
> 2017-09-30 15:47:55.154 CEST [6030] LOG:  invalid magic number  in log
> segment 00010020, offset 10092544

Hi Dmitry,

Thanks for testing.  Yeah, it looks like the patch may be corrupting
the WAL stream in some case that I didn't hit in my own testing
procedure.  I will try to reproduce these failures.

-- 
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] alter server for foreign table

[konstantin knizhnik]

On Sep 30, 2017, at 10:58 PM, Andrew Dunstan wrote:

> 
> 
> On 09/30/2017 05:14 AM, Derry Hamilton wrote:
>> Just to say, yes, this would be handy. I've been using a variant of
>> that hack on reporting servers, while migrating systems from
>> proprietary databases.  It behaves quite gracefully when there are
>> incompatible options, and it fixes up properly with DROPs as the first
>> options.
>> 
>> 
> 
> 
> I assume the proposal is to allow changing to a different server using
> the same FDW. I can see all sorts of odd things happening if we allow
> changing to a server of a different FDW.

Actually what I need is to handle a move of a shard (partition) to other node.
I can not use "alter server" to change connection string, because this server 
is used for many shards located at this node.
And I do not want to drop and recreate foreign table because it seems to be 
very complicated. 

So I need to change server of the same FDW.
But in theory I can imagine situation when partition is moved to another 
database (from MySQL to Postgres for example). In this case we need to change 
FDW.
What can be wrong with changing FDW? All checks that FDW is consistent with 
foreign table can be redone...




-- 
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] alter server for foreign table

[Nico Williams]

On Sat, Sep 30, 2017 at 03:58:04PM -0400, Andrew Dunstan wrote:
> On 09/30/2017 05:14 AM, Derry Hamilton wrote:
> > Just to say, yes, this would be handy. I've been using a variant of
> > that hack on reporting servers, while migrating systems from
> > proprietary databases.  It behaves quite gracefully when there are
> > incompatible options, and it fixes up properly with DROPs as the first
> > options.
> 
> I assume the proposal is to allow changing to a different server using
> the same FDW. I can see all sorts of odd things happening if we allow
> changing to a server of a different FDW.

Like what that could not happen without this feature anyways?

Suppose the foreign server becomes unreachable, changes its schema
completely, and becomes reachable again?  How would that be different
from changing the server name to one with a totally different schema?

Naturally one should shoot one's feet off, but the proposed feature
wouldn't exactly be a footgun.  To believe otherwise would be like
arguing that DROP TABLE (especially CASCASDE!) is a footgun, so better
not have it.

Nico
-- 


-- 
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] Incremental sort

[Alexander Korotkov]

On Sat, Sep 16, 2017 at 2:46 AM, Alexander Korotkov <
a.korot...@postgrespro.ru> wrote:

> On Thu, Sep 14, 2017 at 2:48 AM, Alexander Korotkov <
> a.korot...@postgrespro.ru> wrote:
>
>> Patch rebased to current master is attached.  I'm going to improve my
>> testing script and post new results.
>>
>
> New benchmarking script and results are attached.  There new dataset
> parameter is introduced: skew factor.  Skew factor defines skew in
> distribution of groups sizes.
> My idea of generating is just usage of power function where power is from
> 0 to 1.  Following formula is used to get group number for particular item
> number i.
> [((i / number_of_indexes) ^ power) * number_of_groups]
> For example, power = 1/6 gives following distribution of groups sizes:
> group numbergroup size
> 0   2
> 1   63
> 2   665
> 3   3367
> 4   11529
> 5   31031
> 6   70993
> 7   144495
> 8   269297
> 9   468558
>
> For convenience, instead of power itself, I use skew factor where power =
> 1.0 / (1.0 + skew).  Therefore, with skew = 0.0, distribution of groups
> sizes is uniform.  Larger skew gives more skewed distribution (and that
> seems to be quite intuitive).  For, negative skew, group sizes are mirrored
> as for corresponding positive skew.  For example, skew factor = -5.0 gives
> following groups sizes distribution:
> group numbergroup size
> 0   468558
> 1   269297
> 2   144495
> 3   70993
> 4   31031
> 5   11529
> 6   3367
> 7   665
> 8   63
> 9   2
>
> Results shows that between 2172 test cases, in 2113 incremental sort gives
> speedup while in 59 it causes slowdown.  The following 4 test cases show
> most significant slowdown (>10% of time).
>
> Table   GroupedCols GroupCount Skew PreorderedFrac
> FullSortMedian IncSortMedian TimeChangePercent
> int4|int4|numeric   1  100  -10  0
> 1.5688240528  2.0607631207 31.36
> text|int8|text|int4 110  0
> 1.7785198689 <(778)%20519-8689>  2.1816160679 22.66
> int8|int8|int4  1   10  -10  0
>  1.136412859  1.3166360855 15.86
> numeric|text|int4|int8  2   10  -10  1
> 0.4403841496  0.5070910454 15.15
>
> As you can see, 3 of this 4 test cases have skewed distribution while one
> of them is related to costly location-aware comparison of text.  I've no
> particular idea of how to cope these slowdowns.  Probably, it's OK to have
> slowdown in some cases while have speedup in majority of cases (assuming
> there is an option to turn off new behavior).  Probably, we should teach
> optimizer more about skewed distributions of groups, but that doesn't seem
> feasible for me.
>
> Any thoughts?
>

BTW, replacement selection sort was removed by 8b304b8b.  I think it worth
to rerun benchmarks after that, because results might be changed.  Will do.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] [PATCH] Tests for reloptions

[Nikolay Shaplov]

While working with reloptions refactoring patch, I've written series of tests 
that triggers reloptions related code in all access methods. (I needed it to 
make sure I did not break anything while coding)

I've included these tests to that patch.

Meanwhile Alvaro suggested to commit these tests before the main patch, in 
order to make sure, that this patch does not change usual behavior.

So these tests separated from reloptions patch is in the attachment.

I've removed tests that check functionality that were introduced only in my 
patch, and kept those that checks things that are already in postgres.

I also compared test coverage before and after applying this patch 
(You can also compare, I put coverage results online
http://lj.nataraj.su/2017/reloptions_fix/coverage-master/
http://lj.nataraj.su/2017/reloptions_fix/coverage-patched/ )

Tests adds almost 600 lines to the test covered code (but see note at the end 
of the letter)

src/backend/access/common/reloptions.c get only 7 lines, it was quite covered 
by existing test, but all most of the access methods gets some coverage 
increase:

src/backend/access/brin 1268 -> 1280 (+18)
src/backend/access/gin  2924 -> 2924 (0)
src/backend/access/gist 1673 -> 2108 (+435)
src/backend/access/hash 1580 -> 1638 (+58)
src/backend/access/heap 2863 -> 2866 (+3) 
src/backend/access/nbtree   2565 -> 2647 (+82)
src/backend/access/spgist   2066 -> 2068 (+2)

Though I should say that incredible coverage boost for gist, is the result of 
not steady results of test run. The real value should be much less...

Nevertheless tests touches the reloptions related code, checks for proper 
error handling, and it is good.

I think we should commit it.







  

 

-- 
Do code for fun. Can do it for money (Perl & C/C++ ~10h/week)diff --git a/contrib/bloom/expected/bloom.out b/contrib/bloom/expected/bloom.out
index cbc50f7..df9b7b9 100644
--- a/contrib/bloom/expected/bloom.out
+++ b/contrib/bloom/expected/bloom.out
@@ -210,3 +210,25 @@ ORDER BY 1;
  text_ops | t
 (2 rows)
 
+-- reloptions test
+DROP INDEX bloomidx;
+CREATE INDEX bloomidx ON tst USING bloom (i, t) WITH (length=7, col1 = 4);
+SELECT reloptions FROM pg_class WHERE oid = 'bloomidx'::regclass;
+reloptions 
+---
+ {length=7,col1=4}
+(1 row)
+
+-- check for min and max values
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (length=0);
+ERROR:  value 0 out of bounds for option "length"
+DETAIL:  Valid values are between "1" and "4096".
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (length=4097);
+ERROR:  value 4097 out of bounds for option "length"
+DETAIL:  Valid values are between "1" and "4096".
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (col1=0);
+ERROR:  value 0 out of bounds for option "col1"
+DETAIL:  Valid values are between "1" and "4095".
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (col1=4096);
+ERROR:  value 4096 out of bounds for option "col1"
+DETAIL:  Valid values are between "1" and "4095".
diff --git a/contrib/bloom/sql/bloom.sql b/contrib/bloom/sql/bloom.sql
index 2227460..39b0239 100644
--- a/contrib/bloom/sql/bloom.sql
+++ b/contrib/bloom/sql/bloom.sql
@@ -81,3 +81,14 @@ SELECT opcname, amvalidate(opc.oid)
 FROM pg_opclass opc JOIN pg_am am ON am.oid = opcmethod
 WHERE amname = 'bloom'
 ORDER BY 1;
+
+-- reloptions test
+
+DROP INDEX bloomidx;
+CREATE INDEX bloomidx ON tst USING bloom (i, t) WITH (length=7, col1 = 4);
+SELECT reloptions FROM pg_class WHERE oid = 'bloomidx'::regclass;
+-- check for min and max values
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (length=0);
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (length=4097);
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (col1=0);
+CREATE INDEX bloomidx2 ON tst USING bloom (i, t) WITH (col1=4096);
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ed03cb9..e03d72d 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -3551,3 +3551,10 @@ create table parted_validate_test_1 partition of parted_validate_test for values
 alter table parted_validate_test add constraint parted_validate_test_chka check (a > 0) not valid;
 alter table parted_validate_test validate constraint parted_validate_test_chka;
 drop table parted_validate_test;
+-- test alter column options
+CREATE TABLE tmp(i integer);
+INSERT INTO tmp VALUES (1);
+ALTER TABLE tmp ALTER COLUMN i SET (n_distinct = 1, n_distinct_inherited = 2);
+ALTER TABLE tmp ALTER COLUMN i RESET (n_distinct_inherited);
+ANALYZE tmp;
+DROP TABLE tmp;
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 064adb4..16a3b8b 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2337,7 +2337,7 @@ Options: fastupdate=on, gin_pending_list_limit=128
 CREATE INDEX hash_i4_index ON hash_i4_hea

Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)

[Alexander Korotkov]

On Sat, Sep 30, 2017 at 6:12 PM, Shubham Barai 
wrote:

> I have made changes according to your suggestions. Please have a look at
> the updated patch.
> I am also considering your suggestions for my other patches also. But, I
> will need some time to
> make changes as I am currently busy doing my master's.
>

I don't understand why sometimes you call PredicateLockPage() only when
fast update is off.  For example:

@@ -94,6 +101,9 @@ scanPostingTree(Relation index, GinScanEntry scanEntry,
>   break; /* no more pages */
>
>   buffer = ginStepRight(buffer, index, GIN_SHARE);
> +
> + if (!GinGetUseFastUpdate(index))
> + PredicateLockPage(index, BufferGetBlockNumber(buffer), snapshot);
>   }
>
>   UnlockReleaseBuffer(buffer);


But sometimes you call PredicateLockPage() unconditionally.

@@ -131,6 +141,8 @@ collectMatchBitmap(GinBtreeData *btree, GinBtreeStack
> *stack,
>   attnum = scanEntry->attnum;
>   attr = btree->ginstate->origTupdesc->attrs[attnum - 1];
>
> + PredicateLockPage(btree->index, stack->buffer, snapshot);
> +
>   for (;;)
>   {
>   Page page;


As I understand, all page-level predicate locking should happen only when
fast update is off.

Also, despite general idea is described in README-SSI, in-code comments
would be useful.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] [PATCH] Generic type subscripting

[Dmitry Dolgov]

> On Fri, Sep 22, 2017 at 3:51 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
> On 9/21/17 11:24, Dmitry Dolgov wrote:
>> One last thing that I need to clarify. Initially there was an idea to
>> minimize changes in `pg_type`
>
> I see, but there is no value in that if it makes everything else more
> complicated.

I'm still working on that, but obviously I'll not manage to finish it
within this CF,
so I'm going to move it to the next one.

Re: [HACKERS] [PATCH]make pg_rewind to not copy useless WAL files

[Alexander Korotkov]

On Sat, Sep 30, 2017 at 8:18 PM, chenhj  wrote:

> On 2017-09-30 02:17:54，"Alexander Korotkov" 
> wrote:
>
>
> Great.  Now code of this patch looks good for me.
> However, we forgot about documentation.
>
>   
>>The result is equivalent to replacing the target data directory with
>> the
>>source one. Only changed blocks from relation files are copied;
>>all other files are copied in full, including configuration files. The
>>advantage of pg_rewind over taking a new base backup,
>> or
>>tools like rsync, is that pg_rewind
>> does
>>not require reading through unchanged blocks in the cluster. This makes
>>it a lot faster when the database is large and only a small
>>fraction of blocks differ between the clusters.
>>   
>
>
> At least, this paragraph need to be adjusted, because it states whose
> files are copied.  And probably latter paragraphs whose state about WAL
> files.
>
>
>
> Your are rigth.
> I wrote a draft as following, but i'm afraid whether the english statement
> is accurate.
>

I'm not native english speaker too :(

Only the WAL files between the point of divergence and the current WAL
> insert location of the source server are copied, *for* other WAL files are
> useless for the target server.


I'm not sure about this usage of word *for*.  For me, it probably should be
just removed.  Rest of changes looks good for me.  Please, integrate them
into the patch.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] Causal reads take II

[Dmitry Dolgov]

> On 31 July 2017 at 07:49, Thomas Munro 
wrote:
>> On Sun, Jul 30, 2017 at 7:07 AM, Dmitry Dolgov <9erthali...@gmail.com>
wrote:
>>
>> I looked through the code of `synchronous-replay-v1.patch` a bit and ran
a few
>> tests. I didn't manage to break anything, except one mysterious error
that I've
>> got only once on one of my replicas, but I couldn't reproduce it yet.
>> Interesting thing is that this error did not affect another replica or
primary.
>> Just in case here is the log for this error (maybe you can see something
>> obvious, that I've not noticed):
>>
>> LOG:  could not remove directory "pg_tblspc/47733/PG_10_201707211/47732":
>> Directory not empty
>> ...
>
> Hmm.  The first error ("could not remove directory") could perhaps be
> explained by temporary files from concurrent backends.
> ...
> Perhaps in your testing you accidentally copied a pgdata directory over
the
> top of it while it was running?  In any case I'm struggling to see how
> anything in this patch would affect anything at the REDO level.

Hmm...no, I don't think so. Basically what I was doing is just running
`installcheck` against a primary instance (I assume there is nothing wrong
with
this approach, am I right?). This particular error was caused by
`tablespace`
test which was failed in this case:

```
INSERT INTO testschema.foo VALUES(1);
ERROR:  could not open file "pg_tblspc/16388/PG_11_201709191/16386/16390":
No such file or directory
```

I tried few more times, and I've got it two times from four attempts on a
fresh
installation (when all instances were on the same machine). But anyway I'll
try
to investigate, maybe it has something to do with my environment.

> > * Also I noticed that some time-related values are hardcoded (e.g.
50%/25%
> >   time shift when we're dealing with leases). Does it make sense to move
> >   them out and make them configurable?
>
> These numbers are interrelated, and I think they're best fixed in that
> ratio.  You could make it more adjustable, but I think it's better to
> keep it simple with just a single knob.

Ok, but what do you think about converting them to constants to make them
more
self explanatory? Like:

```
/*
+ * Since this timestamp is being sent to the standby where it will be
+ * compared against a time generated by the standby's system clock, we
+ * must consider clock skew.  We use 25% of the lease time as max
+ * clock skew, and we subtract that from the time we send with the
+ * following reasoning:
+ */
+int max_clock_skew = synchronous_replay_lease_time *
MAX_CLOCK_SKEW_PORTION;
```

Also I have another question. I tried to test this patch little bit more,
and
I've got some strange behaviour after pgbench (here is the full output [1]):

```
# primary

$ ./bin/pgbench -s 100 -i test

NOTICE:  table "pgbench_history" does not exist, skipping
NOTICE:  table "pgbench_tellers" does not exist, skipping
NOTICE:  table "pgbench_accounts" does not exist, skipping
NOTICE:  table "pgbench_branches" does not exist, skipping
creating tables...
10 of 1000 tuples (1%) done (elapsed 0.11 s, remaining 10.50 s)
20 of 1000 tuples (2%) done (elapsed 1.06 s, remaining 52.00 s)
30 of 1000 tuples (3%) done (elapsed 1.88 s, remaining 60.87 s)
2017-09-30 15:47:26.884 CEST [6035] LOG:  revoking synchronous replay lease
for standby "walreceiver"...
2017-09-30 15:47:26.900 CEST [6035] LOG:  standby "walreceiver" is no
longer available for synchronous replay
2017-09-30 15:47:26.903 CEST [6197] LOG:  revoking synchronous replay lease
for standby "walreceiver"...
40 of 1000 tuples (4%) done (elapsed 2.44 s, remaining 58.62 s)
2017-09-30 15:47:27.979 CEST [6197] LOG:  standby "walreceiver" is no
longer available for synchronous replay
```

```
# replica

2017-09-30 15:47:51.802 CEST [6034] FATAL:  could not receive data from WAL
stream: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
2017-09-30 15:47:55.154 CEST [6030] LOG:  invalid magic number  in log
segment 00010020, offset 10092544
2017-09-30 15:47:55.257 CEST [10508] LOG:  started streaming WAL from
primary at 0/2000 on timeline 1
2017-09-30 15:48:09.622 CEST [10508] FATAL:  could not receive data from
WAL stream: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
```

Is it something well known or unrelated to the patch itself?

[1]: https://gist.github.com/erthalion/cdc9357f7437171192348239eb4db764

Re: [HACKERS] 64-bit queryId?

[Alexander Korotkov]

On Sat, Sep 30, 2017 at 6:39 PM, Peter Geoghegan  wrote:

> On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas 
> wrote:
> > Assuming, however, that you don't manage to prove all known
> > mathematics inconsistent, what one might reasonably hope to do is
> > render collisions remote enough that one need not worry about them too
> > much in practice.
>
> Isn't that already true in the case of queryId? I've never heard any
> complaints about collisions. Most people don't change
> pg_stat_statements.max, so the probability of a collision is more like
> 1%. And, that's the probability of *any* collision, not the
> probability of a collision that the user actually cares about. The
> majority of entries in pg_stat_statements among those ten thousand
> will not be interesting. Often, 90%+ will be one-hit wonders. If that
> isn't true, then you're probably not going to find pg_stat_statements
> very useful, because you have nothing to focus on.
>

I heard from customers that they periodically dump contents of
pg_stat_statements and then build statistics over long period of time.  If
even they leave default pg_stat_statements.max unchanged, probability of
collision would be significantly higher.

I have heard complaints about a number of different things in
> pg_stat_statements, like the fact that we don't always manage to
> replace constants with '?'/'$n' characters in all cases. I heard about
> that quite a few times during my time at Heroku. But never this.
>

I'm not sure that we should be oriented only by users complaints in this
problem by couple reasons.

1) Some defects could leave unrevealed by users during long periods of
time.  We have examples of GiST picksplit algorithm to be bad resulting bad
query performance.  However, users never complained about that because they
didn't know what is real fair performance for this kind of queries.  In the
same way users may suffer from queryId collisions during long time without
noticing it.  They might have inaccurate statistics of queries execution,
but they don't notice it because they have nothing to compare.

2) Ideally, we should fix potential problems before they materialize, not
only after users complaints.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] alter server for foreign table

[Andrew Dunstan]

On 09/30/2017 05:14 AM, Derry Hamilton wrote:
> Just to say, yes, this would be handy. I've been using a variant of
> that hack on reporting servers, while migrating systems from
> proprietary databases.  It behaves quite gracefully when there are
> incompatible options, and it fixes up properly with DROPs as the first
> options.
>
>


I assume the proposal is to allow changing to a different server using
the same FDW. I can see all sorts of odd things happening if we allow
changing to a server of a different FDW.


cheers

andrew

-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



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

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Geoghegan]

On Sat, Sep 30, 2017 at 12:28 PM, Tom Lane  wrote:
> This suggests to me that arguing about canonicalization is moot so
> far as avoiding reindexing goes: if you change ICU library versions,
> you're screwed and will have to jump through all the reindexing hoops,
> no matter what we do or don't do.  (Maybe we are looking at the wrong
> information to populate collversion?)

Just to be clear, I was never arguing that canonicalization would
avoid reindexing, and I didn't think anyone else was. I believe that
the version string will change when the behavior of its collator
changes for any reason and in any way. This includes changes to how
binary sort keys are generated.

(We don't currently store binary keys on disk, so a change to that
representation doesn't really necessitate a REINDEX for us. The UCA
spec explicitly decouples compatibility for indexing with binary keys
from changes needed due to human requirements. ICU binary sort keys
are compressed, and this is sometimes improved, independently of the
evolution of how natural language experts say text should be sorted.)

> Now, it may still be worthwhile to argue about whether canonicalization
> will help the other component of the problem, which is whether you can
> dump and reload CREATE COLLATION commands into a new ICU version and
> expect to get more-or-less-the-same behavior as before.

Again, to be clear, that's what I was arguing all along. I think that
users will get very good results with this approach. When the sorting
behavior of a locale is refined by natural language experts, it's
almost certainly a very small change that most users that are affected
won't actually notice. For example, when en-us-x-icu is changed,
that's actually due to a general change in the inherited root collator
that doesn't really affect English speakers. There is no practical
question about how you're supposed to sort English text.

-- 
Peter Geoghegan


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

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Tom Lane]

Noah Misch  writes:
> On Sat, Sep 30, 2017 at 11:25:43AM -0400, Tom Lane wrote:
>> Sure, but dealing with that is mechanical: reindex the necessary indexes
>> and you're done.

> In the general case, one must revalidate CHECK constraints, re-partition
> tables, revalidate range values, and reindex.

True, but that is what it is: nothing we can do is going to affect the
consequences of a collation behavior change, if there is one.  What's more
useful for our immediate purposes is to ask whether we can reliably detect
a collation behavior change.  False negatives are bad, but so are false
positives, because those would force DBAs to jump through lots of hoops
unnecessarily.

So: are canonicalized locale descriptions any better or worse by that
metric than non-canonicalized descriptions?  In principle I think a
canonicalized description might be more likely to be recognized as
the "same" locale by another ICU version than one that isn't, but
I don't know whether there's any meaningful difference in practice.

Another point here is whether, even if a new ICU version recognizes
a locale description as being "the same" interpretation that an old
ICU version used, will it report the same collation version?  Limited
experimentation suggests that the collversions we're actually getting
out of ICU depend on little other than the libicu version.  "select
distinct collversion from pg_collation where collversion is not null"
produces this on ICU 4.2.1:

 49.192.5.41
 49.192.0.41

and this on 52.1:

 58.0.6.50
 58.0.0.50

and this on 57.1:

 153.64.29
 153.64

This suggests to me that arguing about canonicalization is moot so
far as avoiding reindexing goes: if you change ICU library versions,
you're screwed and will have to jump through all the reindexing hoops,
no matter what we do or don't do.  (Maybe we are looking at the wrong
information to populate collversion?)

Now, it may still be worthwhile to argue about whether canonicalization
will help the other component of the problem, which is whether you can
dump and reload CREATE COLLATION commands into a new ICU version and
expect to get more-or-less-the-same behavior as before.

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] 10RC1 crash testing MultiXact oddity

[Alvaro Herrera]

Jeff Janes wrote:
> On Fri, Sep 22, 2017 at 1:19 PM, Robert Haas  wrote:
> 
> > On Fri, Sep 22, 2017 at 3:39 PM, Jeff Janes  wrote:
> > > It turns out it is not new in pg10.  I spotted in the log file only by
> > > accident while looking for something else.  Now that I am looking for
> > it, I
> > > do see it in 9.6 as well.
> >
> > So I guess the next question is whether it also shows up if you initdb
> > with 9.4.latest and then run the same test.
> >
> 
> git bisect shows that it shows up in 9.5, at this commit:
> 
> commit bd7c348d83a4576163b635010e49dbcac7126f01
> Author: Andres Freund 
> Date:   Sat Sep 26 19:04:25 2015 +0200
> 
> Rework the way multixact truncations work.

Oh man.  And I thought we were done with that stuff :-(

> Not really sure what the next step is here.  I could promote the
> ereport(LOG...) to a PANIC to get a core dump, but I don't think that would
> help because presumably the problem occurred early, when the truncation was
> done, not when it was detected.

Probably the best way to track it down is to add some instrumentation
elog(LOG) to the multixact truncation mechanism.

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


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

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Noah Misch]

On Sat, Sep 30, 2017 at 11:25:43AM -0400, Tom Lane wrote:
> Noah Misch  writes:
> > On Mon, Sep 25, 2017 at 09:36:44AM -0700, Peter Geoghegan wrote:
> >>> I think it's inevitable that a certain number of users are going to
> >>> have to cope with ICU version changes breaking stuff.
> 
> >> Wasn't the main point of adopting ICU that that doesn't happen when it
> >> isn't essential?
> 
> > I wouldn't describe it that way.  I agree that few, if any, ICU upgrades 
> > will
> > remove country or language codes.  Overall, though, almost every ICU upgrade
> > will be difficult.  Each ICU release, even a minor release like 58.2, 
> > changes
> > the sorting rules in some tiny way.  You then see "Rebuild all objects
> > affected by this collation" messages.
> 
> Sure, but dealing with that is mechanical: reindex the necessary indexes
> and you're done.

In the general case, one must revalidate CHECK constraints, re-partition
tables, revalidate range values, and reindex.

> In the libc world,
> when you upgrade libc's locale definitions, you have no idea what the
> consequences are.

Yep.  It's strictly better than the libc case.


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

Re: [HACKERS] 10RC1 crash testing MultiXact oddity

[Jeff Janes]

On Fri, Sep 22, 2017 at 1:19 PM, Robert Haas  wrote:

> On Fri, Sep 22, 2017 at 3:39 PM, Jeff Janes  wrote:
> > It turns out it is not new in pg10.  I spotted in the log file only by
> > accident while looking for something else.  Now that I am looking for
> it, I
> > do see it in 9.6 as well.
>
> So I guess the next question is whether it also shows up if you initdb
> with 9.4.latest and then run the same test.
>

git bisect shows that it shows up in 9.5, at this commit:

commit bd7c348d83a4576163b635010e49dbcac7126f01
Author: Andres Freund 
Date:   Sat Sep 26 19:04:25 2015 +0200

Rework the way multixact truncations work.

The patches which enable the crashes and the rapid consumption of xid and
multixact both need a little adjustment from the 10rc1 versions, so I'm
attaching a combined patch that applies to bd7c348d83.

Not really sure what the next step is here.  I could promote the
ereport(LOG...) to a PANIC to get a core dump, but I don't think that would
help because presumably the problem occurred early, when the truncation was
done, not when it was detected.

Cheers,

Jeff


crash_instrument.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] [PATCH]make pg_rewind to not copy useless WAL files

[chenhj]

On 2017-09-30 02:17:54，"Alexander Korotkov"  wrote:


Great.  Now code of this patch looks good for me.
However, we forgot about documentation.


  
   The result is equivalent to replacing the target data directory with the
   source one. Only changed blocks from relation files are copied;
   all other files are copied in full, including configuration files. The
   advantage of pg_rewind over taking a new base backup, or
   tools like rsync, is that pg_rewind does
   not require reading through unchanged blocks in the cluster. This makes
   it a lot faster when the database is large and only a small
   fraction of blocks differ between the clusters.
  


At least, this paragraph need to be adjusted, because it states whose files are 
copied.  And probably latter paragraphs whose state about WAL files.






Your are rigth.
I wrote a draft as following, but i'm afraid whether the english statement is 
accurate.


--
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index d5430d4..bcd094b 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -50,9 +50,12 @@ PostgreSQL documentation
   
The result is equivalent to replacing the target data directory with the
source one. Only changed blocks from relation files are copied;
-   all other files are copied in full, including configuration files. The
-   advantage of pg_rewind over taking a new base backup, or
-   tools like rsync, is that pg_rewind does
+   all other files except WAL are copied in full, including configuration
+   files. Only the WAL files between the point of divergence and the current
+   WAL insert location of the source server are copied, for other WAL files
+   are useless for the target server. The advantage of
+   pg_rewind over taking a new base backup, or tools
+   like rsync, is that pg_rewind does
not require reading through unchanged blocks in the cluster. This makes
it a lot faster when the database is large and only a small
fraction of blocks differ between the clusters.
@@ -231,7 +234,7 @@ PostgreSQL documentation
  
   Copy all other files such as pg_xact and
   configuration files from the source cluster to the target cluster
-  (everything except the relation files).
+  (everything except the relation files and some WAL files).
  
 
 


--
Best Regars,
Chen Huajun

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Peter Geoghegan]

On Sat, Sep 30, 2017 at 8:25 AM, Tom Lane  wrote:
> I'd also argue that the point of adopting ICU was exactly so we *could*
> distinguish those cases, and limit the scope of a normal upgrade to
> "reindex these identifiable indexes and you're done".  In the libc world,
> when you upgrade libc's locale definitions, you have no idea what the
> consequences are.

Right. With libc, we think of collations as something that there is a
small, fixed number of on a system, that we cannot safely assume
anything about. But with ICU, all of the semantics of how natural
languages should be sorted are exposed via various APIs, and there are
literally more possible sets of collation behaviors than there are
grains of sand in the Sahara (there are hundreds of distinct scripts,
which we can change the overall ordering of arbitrarily, on top of all
the other customizations). Clearly the libc way of looking at things
doesn't really carry over.

BCP 47 is supposed to be universal -- it's an IETF standard. That's
where all the stability guarantees are. The officially recognized 'u'
extension that ICU uses is a CLDR/Unicode thing, not an ICU thing. The
same format could, in the future, be used by other collation
providers, since there actually are other CLDR consumers/UCA
implementations. And, ICU have said that they have deprecated the old
locale format, and have standardized on BCP 47. As of ICU 54, it is
recommended that ucol_open() be passed a string in BCP 47 format.

I'm surprised that this issue was not resolved earlier in the week. I
presumed that all of this was obvious to Peter E., but I seem to have
been wrong about that.

-- 
Peter Geoghegan


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

Re: [HACKERS] 64-bit queryId?

[Robert Haas]

On Sat, Sep 30, 2017 at 12:03 PM, Tom Lane  wrote:
> More to the point: with 32-bit IDs, it's apparent that you shouldn't
> really rely on them being unique, and should design your usage so that
> it will survive collisions.

But the point is precisely that we do not do that.  pg_stat_statements
"survives" collisions, but nothing good happens.

-- 
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] 64-bit queryId?

[Andres Freund]

On 2017-09-30 12:03:57 -0400, Tom Lane wrote:
> Peter Geoghegan  writes:
> > On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas  wrote:
> >> Assuming, however, that you don't manage to prove all known
> >> mathematics inconsistent, what one might reasonably hope to do is
> >> render collisions remote enough that one need not worry about them too
> >> much in practice.
> 
> > Isn't that already true in the case of queryId? I've never heard any
> > complaints about collisions.
> 
> More to the point: with 32-bit IDs, it's apparent that you shouldn't
> really rely on them being unique, and should design your usage so that
> it will survive collisions.  Robert seems to be arguing that if we
> merely made the IDs wider, it would be okay to design applications that
> don't allow for that and would fail hard on a collision.  I'm reminded
> of Weinberg's famous line "If builders built houses the way programmers
> build programs, the first woodpecker to come along would destroy
> civilization".

I think you're putting words and intent into Robert's mouth.  If you
design a hashtable you're concerned about unnecessary collisions, even
if you're aware of them.  Additionally, it's not clear you always can do
all that much about the collisions, without accepting undue overhead -
see e.g. pg_stat_statements.

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] 64-bit queryId?

[Tom Lane]

Peter Geoghegan  writes:
> On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas  wrote:
>> Assuming, however, that you don't manage to prove all known
>> mathematics inconsistent, what one might reasonably hope to do is
>> render collisions remote enough that one need not worry about them too
>> much in practice.

> Isn't that already true in the case of queryId? I've never heard any
> complaints about collisions.

More to the point: with 32-bit IDs, it's apparent that you shouldn't
really rely on them being unique, and should design your usage so that
it will survive collisions.  Robert seems to be arguing that if we
merely made the IDs wider, it would be okay to design applications that
don't allow for that and would fail hard on a collision.  I'm reminded
of Weinberg's famous line "If builders built houses the way programmers
build programs, the first woodpecker to come along would destroy
civilization".

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] Parallel Append implementation

[Robert Haas]

On Sat, Sep 30, 2017 at 12:20 AM, Amit Kapila  wrote:
> Okay, but the point is whether it will make any difference
> practically.  Let us try to see with an example, consider there are
> two children (just taking two for simplicity, we can extend it to
> many) and first having 1000 pages to scan and second having 900 pages
> to scan, then it might not make much difference which child plan
> leader chooses.  Now, it might matter if the first child relation has
> 1000 pages to scan and second has just 1 page to scan, but not sure
> how much difference will it be in practice considering that is almost
> the maximum possible theoretical difference between two non-partial
> paths (if we have pages greater than 1024 pages
> (min_parallel_table_scan_size) then it will have a partial path).

But that's comparing two non-partial paths for the same relation --
the point here is to compare across relations.  Also keep in mind
scenarios like this:

SELECT ... FROM relation UNION ALL SELECT ... FROM generate_series(...);

>> It's a lot fuzzier what is best when there are only partial plans.
>>
>
> The point that bothers me a bit is whether it is a clear win if we
> allow the leader to choose a different strategy to pick the paths or
> is this just our theoretical assumption.  Basically, I think the patch
> will become simpler if pick some simple strategy to choose paths.

Well, that's true, but is it really that much complexity?

And I actually don't see how this is very debatable.  If the only
paths that are reasonably cheap are GIN index scans, then the only
strategy is to dole them out across the processes you've got.  Giving
the leader the cheapest one seems to be to be clearly smarter than any
other strategy.  Am I missing something?

-- 
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] 64-bit queryId?

[Andres Freund]

On 2017-09-30 11:50:08 -0400, Robert Haas wrote:
> Well, I think that the fact that pg_stat_statements.max exists at all
> is something that could be fixed now that we have DSA.

You normally *do* want a limit imo. And given that query strings are now
stored externally, I don't think there's a huge space concern anymore?

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] 64-bit queryId?

[Robert Haas]

On Sat, Sep 30, 2017 at 11:39 AM, Peter Geoghegan  wrote:
> On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas  wrote:
>> Assuming, however, that you don't manage to prove all known
>> mathematics inconsistent, what one might reasonably hope to do is
>> render collisions remote enough that one need not worry about them too
>> much in practice.
>
> Isn't that already true in the case of queryId? I've never heard any
> complaints about collisions. Most people don't change
> pg_stat_statements.max, so the probability of a collision is more like
> 1%. And, that's the probability of *any* collision, not the
> probability of a collision that the user actually cares about. The
> majority of entries in pg_stat_statements among those ten thousand
> will not be interesting. Often, 90%+ will be one-hit wonders. If that
> isn't true, then you're probably not going to find pg_stat_statements
> very useful, because you have nothing to focus on.

Well, I think that the fact that pg_stat_statements.max exists at all
is something that could be fixed now that we have DSA.  It's hard to
tell right now whether the fact that we don't hear about collisions is
an artifact of that limit or of the underlying workload.  Also, if
someone did have collisions, how would they even know?

-- 
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] 64-bit queryId?

[Peter Geoghegan]

On Sat, Sep 30, 2017 at 8:39 AM, Peter Geoghegan  wrote:
> Isn't that already true in the case of queryId? I've never heard any
> complaints about collisions. Most people don't change
> pg_stat_statements.max, so the probability of a collision is more like
> 1%. And, that's the probability of *any* collision, not the
> probability of a collision that the user actually cares about. The
> majority of entries in pg_stat_statements among those ten thousand
> will not be interesting.

Correction: ten thousand is an example value of pg_stat_statements.max
in the docs, not the default value. The default is five thousand.

-- 
Peter Geoghegan


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

Re: [HACKERS] 64-bit queryId?

[Peter Geoghegan]

On Sat, Sep 30, 2017 at 7:34 AM, Robert Haas  wrote:
> Assuming, however, that you don't manage to prove all known
> mathematics inconsistent, what one might reasonably hope to do is
> render collisions remote enough that one need not worry about them too
> much in practice.

Isn't that already true in the case of queryId? I've never heard any
complaints about collisions. Most people don't change
pg_stat_statements.max, so the probability of a collision is more like
1%. And, that's the probability of *any* collision, not the
probability of a collision that the user actually cares about. The
majority of entries in pg_stat_statements among those ten thousand
will not be interesting. Often, 90%+ will be one-hit wonders. If that
isn't true, then you're probably not going to find pg_stat_statements
very useful, because you have nothing to focus on.

I have heard complaints about a number of different things in
pg_stat_statements, like the fact that we don't always manage to
replace constants with '?'/'$n' characters in all cases. I heard about
that quite a few times during my time at Heroku. But never this.

-- 
Peter Geoghegan


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

Re: [HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Tom Lane]

Noah Misch  writes:
> On Mon, Sep 25, 2017 at 09:36:44AM -0700, Peter Geoghegan wrote:
>>> I think it's inevitable that a certain number of users are going to
>>> have to cope with ICU version changes breaking stuff.

>> Wasn't the main point of adopting ICU that that doesn't happen when it
>> isn't essential?

> I wouldn't describe it that way.  I agree that few, if any, ICU upgrades will
> remove country or language codes.  Overall, though, almost every ICU upgrade
> will be difficult.  Each ICU release, even a minor release like 58.2, changes
> the sorting rules in some tiny way.  You then see "Rebuild all objects
> affected by this collation" messages.

Sure, but dealing with that is mechanical: reindex the necessary indexes
and you're done.  I think it's important to distinguish that from a case
where users have to change their collation definitions.  That is a
qualitatively different, and much harder, upgrade problem.

I'd also argue that the point of adopting ICU was exactly so we *could*
distinguish those cases, and limit the scope of a normal upgrade to
"reindex these identifiable indexes and you're done".  In the libc world,
when you upgrade libc's locale definitions, you have no idea what the
consequences are.

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] extension build issue with PostgreSQL 10 on Centos6

[Tom Lane]

Devrim =?ISO-8859-1?Q?G=FCnd=FCz?=  writes:
> On Sat, 2017-09-30 at 08:19 +0200, Pavel Stehule wrote:
>> probably rpm is created with --with-icu

> Sure, I compile all v10 RPMs with ICU support.

So the point is that postgresql-devel now needs to have a dependency
on icu-devel.

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] GSoC 2017: weekly progress reports (week 6)

[Shubham Barai]

 Sent with Mailtrack

<#>

On 28 September 2017 at 15:49, Alexander Korotkov  wrote:

> On Thu, Sep 28, 2017 at 12:45 AM, Alexander Korotkov <
> a.korot...@postgrespro.ru> wrote:
>
>> On Wed, Aug 9, 2017 at 6:30 PM, Shubham Barai 
>> wrote:
>>
>>> Please find the updated patch for predicate locking in gin index here.
>>>
>>> There was a small issue in the previous patch. I didn't consider the case
>>> where only root page exists in the tree, and there is a predicate lock
>>> on it,
>>> and it gets split.
>>>
>>> If we treat the original page as a left page and create a new root and
>>> right
>>> page, then we just need to copy a predicate lock from the left to the
>>> right
>>> page (this is the case in B-tree).
>>>
>>> But if we treat the original page as a root and create a new left and
>>> right
>>> page, then we need to copy a predicate lock on both new pages (in the
>>> case of rum and gin).
>>>
>>> link to updated code and tests: https://github.com/shub
>>> hambaraiss/postgres/commit/6172639a104785f051cb4aa0d511c58f2bae65a6
>>>
>>
>> I've assigned to review this patch.  First of all I'd like to understand
>> general idea of this patch.
>>
>> As I get, you're placing predicate locks to both entry tree leaf pages
>> and posting tree leaf pages.  But, GIN implements so called "fast scan"
>> technique which allows it to skip some leaf pages of posting tree when
>> these pages are guaranteed to not contain matching item pointers.  Wherein
>> the particular order of posting list scan and skip depends of their
>> estimated size (so it's a kind of coincidence).
>>
>> But thinking about this more generally, I found that proposed locking
>> scheme is redundant.  Currently when entry has posting tree, you're locking
>> both:
>> 1) entry tree leaf page containing pointer to posting tree,
>> 2) leaf pages of corresponding posting tree.
>> Therefore conflicting transactions accessing same entry would anyway
>> conflict while accessing the same entry tree leaf page.  So, there is no
>> necessity to lock posting tree leaf pages at all.  Alternatively, if entry
>> has posting tree, you can skip locking entry tree leaf page and lock
>> posting tree leaf pages instead.
>>
>
> I'd like to note that I had following warnings during compilation using
> clang.
>
> gininsert.c:519:47: warning: incompatible pointer to integer conversion
>> passing 'void *' to parameter of type 'Buffer' (aka 'int')
>> [-Wint-conversion]
>> CheckForSerializableConflictIn(index, NULL, NULL);
>> ^~~~
>> /Applications/Xcode.app/Contents/Developer/Toolchains/
>> XcodeDefault.xctoolchain/usr/bin/../lib/clang/8.0.0/include/stddef.h:105:16:
>> note: expanded from macro 'NULL'
>> #  define NULL ((void*)0)
>>^~
>> ../../../../src/include/storage/predicate.h:64:87: note: passing
>> argument to parameter 'buffer' here
>> extern void CheckForSerializableConflictIn(Relation relation, HeapTuple
>> tuple, Buffer buffer);
>>
>> ^
>> 1 warning generated.
>> ginvacuum.c:163:2: warning: implicit declaration of function
>> 'PredicateLockPageCombine' is invalid in C99 [-Wimplicit-function-
>> declaration]
>> PredicateLockPageCombine(gvs->index, deleteBlkno, rightlink);
>> ^
>> 1 warning generated.
>
>
> Also, I tried to remove predicate locks from posting tree leafs.  At least
> isolation tests passed correctly after this change.
>
> However, after telegram discussion with Andrew Borodin, we decided that it
> would be better to do predicate locking and conflict checking for posting
> tree leafs, but skip that for entry tree leafs (in the case when entry has
> posting tree).  That would give us more granular locking and less false
> positives.
>
>
>
>
Hi Alexander,

I have made changes according to your suggestions. Please have a look at
the updated patch.
I am also considering your suggestions for my other patches also. But, I
will need some time to
make changes as I am currently busy doing my master's.


Kind Regards,
Shubham


Predicate-locking-in-gin-index_2.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] 64-bit queryId?

[Robert Haas]

On Sat, Sep 30, 2017 at 12:34 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> How about widening the value to uint64?
>
> Doesn't really seem like that would guarantee no collisions.

Well, no duh.  If you come up with a hash function that maps an
infinite domain onto a finite range while guaranteeing no collisions,
I will look forward to reading the paper with interest.

Assuming, however, that you don't manage to prove all known
mathematics inconsistent, what one might reasonably hope to do is
render collisions remote enough that one need not worry about them too
much in practice.  From that point of view, reducing the probability
of a collision by several orders of magnitude seems worth doing if (1)
the cost isn't too much and (2) the probability of a collision as
things stand is significant.  I argue that both of those things are
probably true.

-- 
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] Parallel Append implementation

[Amit Kapila]

On Wed, Sep 20, 2017 at 10:59 AM, Amit Khandekar  wrote:
> On 16 September 2017 at 10:42, Amit Kapila  wrote:
>>
>> At a broader level, the idea is good, but I think it won't turn out
>> exactly like that considering your below paragraph which indicates
>> that it is okay if leader picks a partial path that is costly among
>> other partial paths as a leader won't be locked with that.
>> Considering this is a good design for parallel append, the question is
>> do we really need worker and leader to follow separate strategy for
>> choosing next path.  I think the patch will be simpler if we can come
>> up with a way for the worker and leader to use the same strategy to
>> pick next path to process.  How about we arrange the list of paths
>> such that first, all partial paths will be there and then non-partial
>> paths and probably both in decreasing order of cost.  Now, both leader
>> and worker can start from the beginning of the list. In most cases,
>> the leader will start at the first partial path and will only ever
>> need to scan non-partial path if there is no other partial path left.
>> This is not bulletproof as it is possible that some worker starts
>> before leader in which case leader might scan non-partial path before
>> all partial paths are finished, but I think we can avoid that as well
>> if we are too worried about such cases.
>
> If there are no partial subpaths, then again the leader is likely to
> take up the expensive subpath. And this scenario would not be
> uncommon.
>

While thinking about how common the case of no partial subpaths would
be, it occurred to me that as of now we always create a partial path
for the inheritance child if it is parallel-safe and the user has not
explicitly set the value of parallel_workers to zero (refer
compute_parallel_worker).  So, unless you are planning to change that,
I think it will be quite uncommon to have no partial subpaths.

Few nitpicks in your latest patch:
1.
@@ -298,6 +366,292 @@ ExecReScanAppend(AppendState *node)
  if (subnode->chgParam == NULL)
  ExecReScan(subnode);
  }
+

Looks like a spurious line.

2.
@@ -1285,7 +1291,11 @@ add_paths_to_append_rel(PlannerInfo *root,
RelOptInfo *rel,
..
+ if (chosen_path && chosen_path != cheapest_partial_path)
+ pa_all_partial_subpaths = false;

It will keep on setting pa_all_partial_subpaths as false for
non-partial paths which don't seem to be the purpose of this variable.
I think you want it to be set even when there is one non-partial path,
so isn't it better to write as below or something similar:
if (pa_nonpartial_subpaths && pa_all_partial_subpaths)
pa_all_partial_subpaths = false;


-- 
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] alter server for foreign table

[Derry Hamilton]

Just to say, yes, this would be handy. I've been using a variant of that
hack on reporting servers, while migrating systems from proprietary
databases.  It behaves quite gracefully when there are incompatible
options, and it fixes up properly with DROPs as the first options.

Derry

Re: [HACKERS] extension build issue with PostgreSQL 10 on Centos6

[Devrim Gündüz]

Hi,

On Sat, 2017-09-30 at 08:19 +0200, Pavel Stehule wrote:
> probably rpm is created with --with-icu

Sure, I compile all v10 RPMs with ICU support.

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


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

[HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Noah Misch]

On Mon, Sep 25, 2017 at 09:36:44AM -0700, Peter Geoghegan wrote:
> On Mon, Sep 25, 2017 at 9:06 AM, Robert Haas  wrote:
> >> The big concern I have here is that this feels a lot like something that
> >> we'll regret at leisure, if it's not right in the first release.  I'd
> >> much rather be restrictive in v10 and then loosen the rules later, than
> >> be lax in v10 and then have to argue about whether to break backwards
> >> compatibility in order to gain saner behavior.
> >
> > I think it's inevitable that a certain number of users are going to
> > have to cope with ICU version changes breaking stuff.
> 
> Wasn't the main point of adopting ICU that that doesn't happen when it
> isn't essential? There will be some risk with pg_dump across ICU
> versions, due rare to political changes. But, on pg_upgrade, the old
> collations will continue to work, even if they would not have appeared
> at initdb time on that ICU version, because ICU has all kinds of
> fallbacks.

I wouldn't describe it that way.  I agree that few, if any, ICU upgrades will
remove country or language codes.  Overall, though, almost every ICU upgrade
will be difficult.  Each ICU release, even a minor release like 58.2, changes
the sorting rules in some tiny way.  You then see "Rebuild all objects
affected by this collation" messages.


-- 
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] 64-bit queryId?

[Michael Paquier]

On Sat, Sep 30, 2017 at 1:34 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> How about widening the value to uint64?
>
> Doesn't really seem like that would guarantee no collisions.

This moves the possibility of a 25% collision from 50k queries 3.3
billion. Not zero, but safe enough as a minimal change for many years
to come.
-- 
Michael


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

[HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?

[Noah Misch]

On Mon, Sep 25, 2017 at 08:26:21AM +, Noah Misch wrote:
> On Tue, Sep 19, 2017 at 07:01:47PM -0700, Peter Geoghegan wrote:
> > On Tue, Sep 19, 2017 at 5:52 PM, Peter Eisentraut
> >  wrote:
> > > On 9/18/17 18:46, Peter Geoghegan wrote:
> > >> As I pointed out a couple of times already [1], we don't currently
> > >> sanitize ICU's BCP 47 language tags within CREATE COLLATION.
> > >
> > > There is no requirement that the locale strings for ICU need to be BCP
> > > 47.  ICU locale names like 'de@collation=phonebook' are also acceptable.
> > 
> > Right. But, we only document that BCP 47 is supported by Postgres.
> > Maybe we can use get_icu_language_tag()/uloc_toLanguageTag() to ensure
> > that we end up with BCP 47, even when the user happens to specify the
> > legacy syntax. Should we be "canonicalizing" legacy ICU locale strings
> > as BCP 47, too?
> > 
> > > The reason they are not validated is that, as you know, ICU accepts any
> > > locale string as valid.  You appear to have found a way to do some
> > > validation, but I would like to see that code.
> > 
> > As I mentioned, I'm simply calling
> > get_icu_language_tag()/uloc_toLanguageTag() to do that sanitization.
> > The code to get the extra sanitization is completely trivial.
> > 
> > I didn't post the patch that generates the errors in my opening e-mail
> > because I'm not confident it's correct just yet. And, I think that I
> > see a bigger problem: we pass a string that is almost certainly a BCP
> > 47 string to ucol_open() from within pg_newlocale_from_collation(). We
> > do so despite the fact that ucol_open() apparently doesn't accept BCP
> > 47 syntax locale strings until ICU 54 [1]. Seems entirely possible
> > that this accounts for the problems you saw on ICU 4.2, back when we
> > were still creating keyword variants (I guess that the keyword
> > variants seem very "BCP 47-ish" to me).
> > 
> > I really think we need to add some kind of debug mode that makes ICU
> > optionally spit out a locale display name at key points. We need this
> > to gain confidence that the behavior that ICU provides actually
> > matches what is expected across ICU different versions for different
> > locale formats. We talked about this as a user-facing feature before,
> > which can wait till v11; I just want this to debug problems like this
> > one.
> > 
> > [1] 
> > https://ssl.icu-project.org/apiref/icu4c/ucol_8h.html#a4721e4c0a519bb0139a874e191223590
> 
> [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

This PostgreSQL 10 open item is past due for your status update.  On the worst
week to be violating open item policies.  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