Re: [HACKERS] PostgreSQL 10 changes in exclusion constraints - did something change? CASE WHEN behavior oddity

[Regina Obe]

> Did something change with how exclusion constraints are handled?  I'm
trying to troubleshoot a regression we are having with PostGIS raster
support.

> As best I can guess, it's because exclusion constraints that used to work
in past versions are failing in PostgreSQL 10 with an error something like
> this:

> ERROR:  conflicting key value violates exclusion constraint
"enforce_spatially_unique_test_raster_columns_rast"
> ERROR:  new row for relation "test_raster_columns" violates check
constraint "enforce_coverage_tile_rast"

> Unfortunately I don't know how long this has been an issue since we had an
earlier test failing preventing the raster ones  from being tested.

> Thanks,
> Regina


I figured out the culprit was the change in CASE WHEN behavior with set
returning functions

Had a criteria something of the form:

CASE WHEN some_condition_dependent_on_sometable_that_resolves_to_false THEN
(regexp_matches(...))[1] ELSE ...  END
FROM sometable;


One thing that seems a little odd to me is why these return a record


SELECT CASE WHEN strpos('ABC', 'd') > 1 THEN (regexp_matches('a (b) c',
'd'))[1] ELSE 'a' END;

SELECT CASE WHEN false THEN (regexp_matches('a (b) c', 'd'))[1] ELSE 'a' END
FROM pg_tables;



And this doesn't - I'm guessing it has to do with this being a function of
the value of table, but it seems unintuitive 
>From a user perspective.

SELECT CASE WHEN  strpos(f.tablename, 'ANY (ARRAY[') > 1 THEN
(regexp_matches('a (b) c', 'd'))[1] ELSE 'a' END
FROM pg_tables AS f;


Pre-PostgreSQL 10 this would return a row for each record in pg_tables



Thanks,
Regina



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

Re: [HACKERS] pg_dump ignoring information_schema tables which used in Create Publication.

[Robert Haas]

On Thu, May 25, 2017 at 5:06 PM, Peter Eisentraut
 wrote:
> They are the same cases.
>
> a) Create object in information_schema.
>
> b) Create another object elsewhere that depends on it.
>
> c) pg_dump will dump (b) but not (a).
>
> So the fix, if any, would be to prevent (a), or prevent (b), or fix (c).

I guess I'm not convinced that it's really the same.  I think we want
to allow users to create views over system objects; our life might be
easier if we hadn't permitted that, but views over e.g. pg_locks are
common, and prohibiting them doesn't seem like a reasonable choice.
I'm less clear that we want to let them publish system objects.  Aside
from the pg_dump issues, does it work?

-- 
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: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)

[Amit Kapila]

On Thu, May 25, 2017 at 8:01 PM, Tom Lane  wrote:
> Amit Kapila  writes:
>> Yes, I also share this opinion, the shm attach failures are due to
>> randomization behavior, so sleep won't help much.  So, I will change
>> the patch to use 100 retries unless people have other opinions.
>
> Sounds about right to me.
>

Okay.  I have changed the retry count to 100 and modified few comments
in the attached patch.

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


win_shm_retry_reattach_v2.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: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)

[Tsunakawa, Takayuki]

From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit Kapila
> Yes, I also share this opinion, the shm attach failures are due to
> randomization behavior, so sleep won't help much.  So, I will change the
> patch to use 100 retries unless people have other opinions.

Perhaps I'm misunderstanding, but I thought it is not known yet whether the 
cause of the original problem is ASLR.  I remember someone referred to 
anti-virus software and something else.  I guessed that the reason Noah 
suggested 1 - 5 seconds of retry is based on the expectation that the address 
space might be freed by the anti-virus software.

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] Create subscription with `create_slot=false` and incorrect slot name

[Petr Jelinek]

On 25/05/17 23:26, Peter Eisentraut wrote:
> On 5/24/17 21:41, Robert Haas wrote:
>>> This came up in a previous thread.  It is up to the publishing end what
>>> slot names it accepts.  So running the validation locally is incorrect.
>>
>> That argument seems pretty tenuous; surely both ends are PostgreSQL,
>> and the rules for valid slot names aren't likely to change very often.
> 
> Remember that this could be used for upgrades and side-grades, so the
> local rules could change or be more restricted in the future or
> depending on compilation options.
> 
>> But even if we accept it as true, I still don't like the idea that a
>> DROP can just fail, especially with no real guidance as to how to fix
>> things so it doesn't fail.  Ideas:
>>
>> 1. If we didn't create the slot (and have never connected to it?),
>> don't try to drop it.
> 
> That would conceptually be nice, but it would probably create a bunch of
> problems of its own.  For one, we would need an interlock so that the
> first $anything that connects to the slot registers it in the local
> catalog as "it's mine now".
> 
>> 2. Emit some kind of a HINT telling people about ALTER SUBSCRIPTION ..
>> SET (slot_name = NONE).
> 
> The reported error is just one of many errors that can happen when DROP
> SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
> permission, etc.).  We don't want to give the hint that is effectively
> "just forget about the slot then" for all of them.  So we would need
> some way to distinguish "you can't do this right now" from "this would
> never work" (400 vs 500 errors).
> 

This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
could check for it and offer hint only for this case.



-- 
  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] No parameter values checking while creating Alter subscription...Connection

[Petr Jelinek]

On 25/05/17 23:18, Andres Freund wrote:
> On 2017-05-25 17:08:57 -0400, Peter Eisentraut wrote:
>> On 5/25/17 10:18, Masahiko Sawada wrote:
 postgres=# alter subscription c1 connection 'port=4000';
 ALTER SUBSCRIPTION
 postgres=# alter subscription c1 connection 'dbname=cc';
 ALTER SUBSCRIPTION

>>> CREATE SUBSCRIPTION tries to connect to publisher to create
>>> replication slot or to get table list for table synchronization, not
>>> to check the connection parameter value. So if you specify connect =
>>> false then CREATE SUBSCRIPTION doesn't try to connect.
>>
>> We don't make a connection attempt as part of ALTER SUBSCRIPTION.  I
>> guess we could just connect and disconnect to check that it works.
> 
> I think during reconfigurations it's quite useful to be able to do so
> even if the other hosts aren't reachable that second.
> 

Yes, it's intended behavior for this very reason, we want ability to
(re)configure downstream without any existing upstream. It's also reason
why we have the WITH (connect = false) in CREATE SUBSCRIPTION. I don't
see a nice way how to do something similar (ie make it optional) for
ALTER though.

-- 
  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] Create subscription with `create_slot=false` and incorrect slot name

[Peter Eisentraut]

On 5/24/17 21:41, Robert Haas wrote:
>> This came up in a previous thread.  It is up to the publishing end what
>> slot names it accepts.  So running the validation locally is incorrect.
> 
> That argument seems pretty tenuous; surely both ends are PostgreSQL,
> and the rules for valid slot names aren't likely to change very often.

Remember that this could be used for upgrades and side-grades, so the
local rules could change or be more restricted in the future or
depending on compilation options.

> But even if we accept it as true, I still don't like the idea that a
> DROP can just fail, especially with no real guidance as to how to fix
> things so it doesn't fail.  Ideas:
> 
> 1. If we didn't create the slot (and have never connected to it?),
> don't try to drop it.

That would conceptually be nice, but it would probably create a bunch of
problems of its own.  For one, we would need an interlock so that the
first $anything that connects to the slot registers it in the local
catalog as "it's mine now".

> 2. Emit some kind of a HINT telling people about ALTER SUBSCRIPTION ..
> SET (slot_name = NONE).

The reported error is just one of many errors that can happen when DROP
SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
permission, etc.).  We don't want to give the hint that is effectively
"just forget about the slot then" for all of them.  So we would need
some way to distinguish "you can't do this right now" from "this would
never work" (400 vs 500 errors).

Another way to fix this particular issue is to not verify the
replication slot name before doing the drop.  After all, if the name is
not valid, then you can also just report that it doesn't exist.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Renaming a table to an array's autogenerated name

[Tom Lane]

Vik Fearing  writes:
> In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo
> when an array type of that name already existed would make the array
> type change its name to get out of the way. But it missed a trick in
> that renaming a table would still cause a conflict.

Good catch.

> One interesting thing to note however, is that if you rename a table to
> its own array's name (which was my case when I found this bug), the new
> array name will be ___foo instead of just __foo.  I don't know if it's
> worth worrying about that.

This seemed weird to me.  Stepping through the logic, I see that what's
happening is that the moveArrayTypeName call you added renames the
array type from "_foo" to "__foo", and then at the bottom of
RenameTypeInternal we call makeArrayTypeName/RenameTypeInternal again
on that array type.  makeArrayTypeName sees that "__foo" exists and
assumes the name is in use, even though it's really this same type and
no more renaming is needed.  So it ends up picking the next choice
"___foo".

After some experimentation, I came up with the attached, which simply
skips the "recursive" step if it would apply to the same array type we
already moved.

I added some queries to the regression test case to show exactly what
happens to the array type names, and in that, you can see that the
behavior for the "normal" case with distinct array types is that neither
array type ends up with a name that is just the unsurprising case of
an underscore followed by its element type's name; they *both* have an
extra underscore compared to that.  Maybe that's okay.  We could possibly
rejigger the order of renaming so that one of them ends with an
unsurprising name, but I failed to make that happen without considerably
more surgery.

regards, tom lane

diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 04c10c6347b81183aa5c5ec3906bc0367672da29..74eeba6e0ab3aa127c1ba01e2c37b770f92629cd 100644
*** a/src/backend/catalog/pg_type.c
--- b/src/backend/catalog/pg_type.c
*** RenameTypeInternal(Oid typeOid, const ch
*** 695,700 
--- 695,701 
  	HeapTuple	tuple;
  	Form_pg_type typ;
  	Oid			arrayOid;
+ 	Oid			oldTypeOid;
  
  	pg_type_desc = heap_open(TypeRelationId, RowExclusiveLock);
  
*** RenameTypeInternal(Oid typeOid, const ch
*** 708,720 
  
  	arrayOid = typ->typarray;
  
! 	/* Just to give a more friendly error than unique-index violation */
! 	if (SearchSysCacheExists2(TYPENAMENSP,
! 			  CStringGetDatum(newTypeName),
! 			  ObjectIdGetDatum(typeNamespace)))
! 		ereport(ERROR,
! (errcode(ERRCODE_DUPLICATE_OBJECT),
!  errmsg("type \"%s\" already exists", newTypeName)));
  
  	/* OK, do the rename --- tuple is a copy, so OK to scribble on it */
  	namestrcpy(&(typ->typname), newTypeName);
--- 709,736 
  
  	arrayOid = typ->typarray;
  
! 	/* Check for a conflicting type name. */
! 	oldTypeOid = GetSysCacheOid2(TYPENAMENSP,
!  CStringGetDatum(newTypeName),
!  ObjectIdGetDatum(typeNamespace));
! 
! 	/*
! 	 * If there is one, see if it's an autogenerated array type, and if so
! 	 * rename it out of the way.  (But we must skip that for a shell type
! 	 * because moveArrayTypeName will do the wrong thing in that case.)
! 	 * Otherwise, we can at least give a more friendly error than unique-index
! 	 * violation.
! 	 */
! 	if (OidIsValid(oldTypeOid))
! 	{
! 		if (get_typisdefined(oldTypeOid) &&
! 			moveArrayTypeName(oldTypeOid, newTypeName, typeNamespace))
! 			/* successfully dodged the problem */ ;
! 		else
! 			ereport(ERROR,
! 	(errcode(ERRCODE_DUPLICATE_OBJECT),
! 	 errmsg("type \"%s\" already exists", newTypeName)));
! 	}
  
  	/* OK, do the rename --- tuple is a copy, so OK to scribble on it */
  	namestrcpy(&(typ->typname), newTypeName);
*** RenameTypeInternal(Oid typeOid, const ch
*** 726,733 
  	heap_freetuple(tuple);
  	heap_close(pg_type_desc, RowExclusiveLock);
  
! 	/* If the type has an array type, recurse to handle that */
! 	if (OidIsValid(arrayOid))
  	{
  		char	   *arrname = makeArrayTypeName(newTypeName, typeNamespace);
  
--- 742,753 
  	heap_freetuple(tuple);
  	heap_close(pg_type_desc, RowExclusiveLock);
  
! 	/*
! 	 * If the type has an array type, recurse to handle that.  But we don't
! 	 * need to do anything more if we already renamed that array type above
! 	 * (which would happen when, eg, renaming "foo" to "_foo").
! 	 */
! 	if (OidIsValid(arrayOid) && arrayOid != oldTypeOid)
  	{
  		char	   *arrname = makeArrayTypeName(newTypeName, typeNamespace);
  
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c88fd768482792bd8376dd21cc5b63be927f7727..8aadbb88a348571dd2fe6e22d1e2a2f72380d35b 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*** SELECT * FROM tmp_new2;
*** 128,133 

Re: [HACKERS] No parameter values checking while creating Alter subscription...Connection

[Andres Freund]

On 2017-05-25 17:08:57 -0400, Peter Eisentraut wrote:
> On 5/25/17 10:18, Masahiko Sawada wrote:
> >> postgres=# alter subscription c1 connection 'port=4000';
> >> ALTER SUBSCRIPTION
> >> postgres=# alter subscription c1 connection 'dbname=cc';
> >> ALTER SUBSCRIPTION
> >>
> > CREATE SUBSCRIPTION tries to connect to publisher to create
> > replication slot or to get table list for table synchronization, not
> > to check the connection parameter value. So if you specify connect =
> > false then CREATE SUBSCRIPTION doesn't try to connect.
> 
> We don't make a connection attempt as part of ALTER SUBSCRIPTION.  I
> guess we could just connect and disconnect to check that it works.

I think during reconfigurations it's quite useful to be able to do so
even if the other hosts aren't reachable that second.

- Andres


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

[HACKERS] Channel binding support for SCRAM-SHA-256

[Michael Paquier]

Hi all,

Please find attached a patch to add support for channel binding for
SCRAM, to mitigate MITM attacks using this protocol. Per RFC5802
(https://tools.ietf.org/html/rfc5802), servers supporting channel
binding need to add support for tls-unique, and this is what this
patch does.

As defined in RFC5056 (exactly here =>
https://tools.ietf.org/html/rfc5056#section-4.1), servers can use the
TLS finish message to determine if the client is actually the same as
the one who initiated the connection, to eliminate the risk of MITM
attacks. OpenSSL offers two undocumented APIs for this purpose:
- SSL_get_finished() to get the latest TLS finish message that the
client has sent.
- SSL_get_peer_finished(), to get the latest TLS finish message
expected by the server.
So basically what is needed here is saving the latest message
generated by client in libpq using get_finished(), send it to the
server using the SASL exchange messages (value sent with the final
client message), and then compare it with what the server expected.
Connection is successful if what the client has sent and what the
server expects match.

There is also a clear documentation about what is expected from the
client and the server in terms of how both should behave using the
first client message https://tools.ietf.org/html/rfc5802#section-6. So
I have tried to follow it, reviews are welcome particularly regarding
that. The implementation is done in such a way that channel binding is
used in the context of an SSL connection, which is what the RFCs
expect from an implementation.

Before even that, the server needs to send to the client the list of
SASL mechanisms that are supported. This adds SCRAM-SHA-256-PLUS if
the server has been built with SSL support to the list.

Comments are welcome, I am parking that in the next CF for integration in PG11.
Thanks,
-- 
Michael


scram-channel-binding.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] No parameter values checking while creating Alter subscription...Connection

[Peter Eisentraut]

On 5/25/17 10:18, Masahiko Sawada wrote:
>> postgres=# alter subscription c1 connection 'port=4000';
>> ALTER SUBSCRIPTION
>> postgres=# alter subscription c1 connection 'dbname=cc';
>> ALTER SUBSCRIPTION
>>
> CREATE SUBSCRIPTION tries to connect to publisher to create
> replication slot or to get table list for table synchronization, not
> to check the connection parameter value. So if you specify connect =
> false then CREATE SUBSCRIPTION doesn't try to connect.

We don't make a connection attempt as part of ALTER SUBSCRIPTION.  I
guess we could just connect and disconnect to check that it works.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] pg_dump ignoring information_schema tables which used in Create Publication.

[Peter Eisentraut]

On 5/25/17 09:55, Robert Haas wrote:
> On Thu, May 25, 2017 at 8:32 AM, Peter Eisentraut
>  wrote:
>>> Well, I think if it's not going to work, it should be prohibited,
>>> rather than seeming to work but then not actually working.
>>
>> Here is a similar case that pg_dump fails on:
>>
>> create table information_schema.test1 (a int);
>> create view public.test2 as select * from information_schema.test1;
>>
>> It's not clear how to address that, or whether it's worth it.
> 
> Sure, that case is hard to address.  But why is *this* case hard to address?

They are the same cases.

a) Create object in information_schema.

b) Create another object elsewhere that depends on it.

c) pg_dump will dump (b) but not (a).

So the fix, if any, would be to prevent (a), or prevent (b), or fix (c).

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

[HACKERS] PostgreSQL 10 changes in exclusion constraints - did something change?

[Regina Obe]

Did something change with how exclusion constraints are handled?  I'm trying
to troubleshoot a regression we are having with PostGIS raster support.

As best I can guess, it's because exclusion constraints that used to work in
past versions are failing in PostgreSQL 10 with an error something like
this:

ERROR:  conflicting key value violates exclusion constraint
"enforce_spatially_unique_test_raster_columns_rast"
ERROR:  new row for relation "test_raster_columns" violates check constraint
"enforce_coverage_tile_rast"

Unfortunately I don't know how long this has been an issue since we had an
earlier test failing preventing the raster ones  from being tested.

Thanks,
Regina





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

[HACKERS] fix side-effect in get_qual_for_list()

[Jeevan Ladhe]

Hi,

While working on one of the crash reported on default partition for list
partitioning table[1] I found some strange behavior in get_qual_for_list()
while
I tried to call it from the new code I wrote for default partition.

After debugging, I noticed that the function get_qual_for_list() is
implicitly
manipulating the (PartitionBoundSpec) spec->listdatums list. AFAICU, this
manipulation is needed just to construct a list of datums to be passed to
ArrayExpr, and this should be done without manipulating the original list.
The function name is get_qual_for_list(), which implies that this function
returns something and does not modify anything.

I have made this change in attached patch, as I think this is useful for
future
developments, as there may be a need in future to call get_qual_for_list()
from
other places, and the caller might not expect that PartitionBoundSpec gets
modified.

PFA.

[1] 
*https://www.postgresql.org/message-id/flat/CAOgcT0PLPge%3D5U6%3DGU5SnC3_8yutCbWWOiUva3Cw94M9zpbvgQ%40mail.gmail.com
*

Regards,
Jeevan Ladhe


fix_listdatums_get_qual_for_list.patch
Description: Binary data

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

[HACKERS] Renaming a table to an array's autogenerated name

[Vik Fearing]

In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo
when an array type of that name already existed would make the array
type change its name to get out of the way. But it missed a trick in
that renaming a table would still cause a conflict.

Steps to reproduce:

postgres=# create table foo (id int);
CREATE TABLE
postgres=# create table bar (id int);
CREATE TABLE
postgres=# alter table bar rename to _foo;
ERROR:  type "_foo" already exists

Attached is a patch that fixes this bug.

One interesting thing to note however, is that if you rename a table to
its own array's name (which was my case when I found this bug), the new
array name will be ___foo instead of just __foo.  I don't know if it's
worth worrying about that.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 04c10c6347..9e4e46f9f5 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -695,6 +695,7 @@ RenameTypeInternal(Oid typeOid, const char *newTypeName, Oid typeNamespace)
 	HeapTuple	tuple;
 	Form_pg_type typ;
 	Oid			arrayOid;
+	Oid			typoid;
 
 	pg_type_desc = heap_open(TypeRelationId, RowExclusiveLock);
 
@@ -708,10 +709,22 @@ RenameTypeInternal(Oid typeOid, const char *newTypeName, Oid typeNamespace)
 
 	arrayOid = typ->typarray;
 
+	typoid = GetSysCacheOid2(TYPENAMENSP,
+			 CStringGetDatum(newTypeName),
+			 ObjectIdGetDatum(typeNamespace));
+
+	/*
+	 * See if it's an autogenerated array type, and if so
+	 * rename it out of the way.
+	 */
+	if (OidIsValid(typoid) && get_typisdefined(typoid))
+	{
+		if (moveArrayTypeName(typoid, newTypeName, typeNamespace))
+			typoid = InvalidOid;
+	}
+
 	/* Just to give a more friendly error than unique-index violation */
-	if (SearchSysCacheExists2(TYPENAMENSP,
-			  CStringGetDatum(newTypeName),
-			  ObjectIdGetDatum(typeNamespace)))
+	if (OidIsValid(typoid))
 		ereport(ERROR,
 (errcode(ERRCODE_DUPLICATE_OBJECT),
  errmsg("type \"%s\" already exists", newTypeName)));
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c88fd76848..142a782bff 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -128,6 +128,15 @@ SELECT * FROM tmp_new2;
 
 DROP TABLE tmp_new;
 DROP TABLE tmp_new2;
+-- renaming to an array's autogenerated name (the array's name should get out of the way)
+CREATE TABLE tmp_array (id int);
+CREATE TABLE tmp_array2 (id int);
+ALTER TABLE tmp_array2 RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
+DROP TABLE tmp_array;
+CREATE TABLE tmp_array (id int);
+ALTER TABLE tmp_array RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
 -- ALTER TABLE ... RENAME on non-table relations
 -- renaming indexes (FIXME: this should probably test the index's functionality)
 ALTER INDEX IF EXISTS __onek_unique1 RENAME TO tmp_onek_unique1;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index c0e29720dc..1a893e9e2c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -165,6 +165,16 @@ SELECT * FROM tmp_new2;
 DROP TABLE tmp_new;
 DROP TABLE tmp_new2;
 
+-- renaming to an array's autogenerated name (the array's name should get out of the way)
+CREATE TABLE tmp_array (id int);
+CREATE TABLE tmp_array2 (id int);
+ALTER TABLE tmp_array2 RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
+DROP TABLE tmp_array;
+
+CREATE TABLE tmp_array (id int);
+ALTER TABLE tmp_array RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
 
 -- ALTER TABLE ... RENAME on non-table relations
 -- renaming indexes (FIXME: this should probably test the index's functionality)

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

Re: [HACKERS] CREATE STATISTICS statistic_type documentation

[Alvaro Herrera]

Tom Lane wrote:
> Jeff Janes  writes:
> > On Thu, May 25, 2017 at 9:28 AM, Tom Lane  wrote:

> > If we invent more types in the future, would we expect those to be
> > defaulted to as well?
> 
> I might be wrong, but my impression is that the plan is to default
> to gathering all possible stats types.  So I'd probably write the
> addendum that way rather than naming those two types specifically.

Yes -- that plan has been hardcoded in ruleutils.c.

I'll patch.

-- 
Á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] Surjective functional indexes

[Konstantin Knizhnik]

On 25.05.2017 19:37, Tom Lane wrote:

Konstantin Knizhnik  writes:

My proposal is to check value of function for functional indexes instead
of just comparing set of effected attributes.
Obviously, for some complex functions it may  have negative effect on
update speed.
This is why I have added "surjective" option to index.

This seems overcomplicated.  We would have to compute the function
value at some point anyway.  Can't we refactor to do that earlier?

regards, tom lane



Check for affected indexes/applicability of HOT update and update of 
indexes themselves is done in two completely different parts of code.
And if we find out that values of indexed expressions are not changed, 
then we can use HOT update and indexes should not be updated
(so calculated value of function is not needed). And it is expected to 
be most frequent case.


Certainly, if value of indexed expression is changed, then we can avoid 
redundant calculation of function by storing result of calculations 
somewhere.
But it will greatly complicate all logic of updating indexes. Please 
notice, that if we have several functional indexes and only one of them 
is actually changed,
then in any case we can not use HOT and have to update all indexes. So 
we do not need to evaluate values of all indexed expressions. We just 
need to find first
changed one. So we should somehow keep track values of which expression 
are calculated and which not.


One more argument. Originally Postgres evaluates index expression only 
once (when inserting new version of tuple to the index).
Now (with this patch) Postgres has to evaluate expression three times in 
the worst case: calculate the value of expression for old and new tuples 
to make a decision bout hot update,
and the evaluate it once again when performing index update itself. Even 
if I managed to store somewhere calculated value of the expression, we 
still have to perform
twice more evaluations than before. This is why for expensive functions 
or for functions defined for frequently updated attributes (in case of 
JSON) such policy should be disabled.
And for non-expensive functions extra overhead is negligible. Also there 
is completely no overhead if indexed expression is not actually changed. 
And it is expected to be most frequent case.


At least at the particular example with YCSB benchmark, our first try 
was just to disable index update by commenting correspondent check of 
updated fields mask.
Obviously there are no extra function calculations in this case. Then I 
have implemented this patch. And performance is almost the same.
This is why I think that simplicity and modularity of code is more 
important here than elimination of redundant function calculation.



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



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

Re: [HACKERS] Surjective functional indexes

[Andres Freund]

On 2017-05-25 12:37:40 -0400, Tom Lane wrote:
> Konstantin Knizhnik  writes:
> > My proposal is to check value of function for functional indexes instead 
> > of just comparing set of effected attributes.
> > Obviously, for some complex functions it may  have negative effect on 
> > update speed.
> > This is why I have added "surjective" option to index.
> 
> This seems overcomplicated.  We would have to compute the function
> value at some point anyway.  Can't we refactor to do that earlier?

Yea, that'd be good. Especially if we were to compute the expressions
for all indexes in one go - doing that in other places (e.g. aggregate
transition values) yielded a good amount of speedup.  It'd be even
larger if we get JITing of expressions.  It seems feasible to do so for
at least the nodeModifyTable case.

I wonder if there's a chance to use such logic alsofor HOT update
considerations, but that seems harder to do without larger layering
violations.

- Andres


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

Re: [HACKERS] CREATE STATISTICS statistic_type documentation

[Tom Lane]

Jeff Janes  writes:
> On Thu, May 25, 2017 at 9:28 AM, Tom Lane  wrote:
>> Where would you expect to find that?

> Probably at the end of the paragraph:
> "A statistic type to be computed in this statistics object. Currently
> supported types are ndistinct, which enables n-distinct statistics, and
> dependencies, which enables functional dependency statistics. For more
> information, see Section 14.2.2 and Section 68.2."

Fair enough.

> Something like "If omitted, both ndistinct and dependencies will be
> included."

> If we invent more types in the future, would we expect those to be
> defaulted to as well?

I might be wrong, but my impression is that the plan is to default
to gathering all possible stats types.  So I'd probably write the
addendum that way rather than naming those two types specifically.

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] CREATE STATISTICS statistic_type documentation

[Jeff Janes]

On Thu, May 25, 2017 at 9:28 AM, Tom Lane  wrote:

> Jeff Janes  writes:
> > The docs for CREATE STATISTICS does not say what happens if the
> > statistic_type clause is omitted.  It should probably say that the
> default
> > action is to create both ndistinct and dependencies.
>
> Hmm, I coulda sworn that it did say that somewhere.
>
> Where would you expect to find that?
>

Probably at the end of the paragraph:

"A statistic type to be computed in this statistics object. Currently
supported types are ndistinct, which enables n-distinct statistics, and
dependencies, which enables functional dependency statistics. For more
information, see Section 14.2.2 and Section 68.2."

Something like "If omitted, both ndistinct and dependencies will be
included."

If we invent more types in the future, would we expect those to be
defaulted to as well?

Cheers,

Jeff

Re: [HACKERS] Surjective functional indexes

[Tom Lane]

Konstantin Knizhnik  writes:
> My proposal is to check value of function for functional indexes instead 
> of just comparing set of effected attributes.
> Obviously, for some complex functions it may  have negative effect on 
> update speed.
> This is why I have added "surjective" option to index.

This seems overcomplicated.  We would have to compute the function
value at some point anyway.  Can't we refactor to do that earlier?

regards, tom lane


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

[HACKERS] Surjective functional indexes

[Konstantin Knizhnik]

Right now Postgres determines whether update operation touch index or 
not based only on set of the affected columns.
But in case of functional indexes such policy quite frequently leads to 
unnecessary index updates.
For example, functional index are widely use for indexing JSON data: 
info->>'name'.


JSON data may contain multiple attributes and only few of them may be 
affected by update.
Moreover, index is used to build for immutable attributes (like "id", 
"isbn", "name",...).


Functions like (info->>'name') are named "surjective" ni mathematics.
I have strong feeling that most of functional indexes are based on 
surjective functions.
For such indexes current Postgresql index update policy is very 
inefficient.  It cause disabling of hot updates

and so leads to significant degrade of performance.

Without this patch Postgres is slower than Mongo on YCSB benchmark with 
(50% update,50 % select)  workload.

And after applying this patch Postgres beats Mongo at all workloads.

My proposal is to check value of function for functional indexes instead 
of just comparing set of effected attributes.
Obviously, for some complex functions it may  have negative effect on 
update speed.
This is why I have added "surjective" option to index. By default it is 
switched on for all functional indexes (based on my assumption
that most functions used in functional indexes are surjective). But it 
is possible to explicitly disable it and make decision weather index

needs to be updated or not only based on set of effected attributes.


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

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 6d1f22f..37fc407 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -130,6 +130,15 @@ static relopt_bool boolRelOpts[] =
 	},
 	{
 		{
+			"surjective",
+			"Reevaluate functional index expression on update to check if its values is changed",
+			RELOPT_KIND_INDEX,
+			AccessExclusiveLock
+		},
+		true
+	},
+	{
+		{
 			"security_barrier",
 			"View acts as a row security barrier",
 			RELOPT_KIND_VIEW,
@@ -1301,7 +1310,7 @@ fillRelOptions(void *rdopts, Size basesize,
 break;
 			}
 		}
-		if (validate && !found)
+		if (validate && !found && options[i].gen->kinds != RELOPT_KIND_INDEX)
 			elog(ERROR, "reloption \"%s\" not found in parse table",
  options[i].gen->name);
 	}
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e890e08..3525e3c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -56,6 +56,7 @@
 #include "access/xlogutils.h"
 #include "catalog/catalog.h"
 #include "catalog/namespace.h"
+#include "catalog/index.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "storage/bufmgr.h"
@@ -73,7 +74,9 @@
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/tqual.h"
-
+#include "utils/memutils.h"
+#include "nodes/execnodes.h"
+#include "executor/executor.h"
 
 /* GUC variable */
 bool		synchronize_seqscans = true;
@@ -4199,6 +4202,7 @@ l2:
 
 	if (use_hot_update)
 	{
+		elog(DEBUG1, "Use hot update");
 		/* Mark the old tuple as HOT-updated */
 		HeapTupleSetHotUpdated();
 		/* And mark the new tuple as heap-only */
@@ -4436,6 +4440,73 @@ HeapDetermineModifiedColumns(Relation relation, Bitmapset *interesting_cols,
 attnum - FirstLowInvalidHeapAttributeNumber);
 	}
 
+	if (hot_result && relation->rd_surjective)
+	{
+		ListCell   *l;
+		List	   *indexoidlist = RelationGetIndexList(relation);
+		EState *estate = CreateExecutorState();
+		ExprContext*econtext = GetPerTupleExprContext(estate);
+		TupleTableSlot *slot = MakeSingleTupleTableSlot(RelationGetDescr(relation));
+		Datum	   	old_values[INDEX_MAX_KEYS];
+		bool		old_isnull[INDEX_MAX_KEYS];
+		Datum	   	new_values[INDEX_MAX_KEYS];
+		bool		new_isnull[INDEX_MAX_KEYS];
+
+		econtext->ecxt_scantuple = slot;
+
+		foreach(l, indexoidlist)
+		{
+			Oid		indexOid = lfirst_oid(l);
+			RelationindexDesc = index_open(indexOid, AccessShareLock);
+			IndexInfo  *indexInfo = BuildIndexInfo(indexDesc);
+			int i;
+
+			if (indexInfo->ii_Expressions && indexInfo->ii_Surjective)
+			{
+ResetExprContext(econtext);
+ExecStoreTuple(oldtup, slot, InvalidBuffer, false);
+FormIndexDatum(indexInfo,
+			   slot,
+			   estate,
+			   old_values,
+			   old_isnull);
+
+ExecStoreTuple(newtup, slot, InvalidBuffer, false);
+FormIndexDatum(indexInfo,
+			   slot,
+			   estate,
+			   new_values,
+			   new_isnull);
+
+for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)
+{
+	if (old_isnull[i] != new_isnull[i])
+	{
+		hot_result = false;
+		break;
+	}
+	else if (!old_isnull[i])
+	{
+		Form_pg_attribute att = RelationGetDescr(indexDesc)->attrs[i];
+		if 

Re: [HACKERS] CREATE STATISTICS statistic_type documentation

[Tom Lane]

Jeff Janes  writes:
> The docs for CREATE STATISTICS does not say what happens if the
> statistic_type clause is omitted.  It should probably say that the default
> action is to create both ndistinct and dependencies.

Hmm, I coulda sworn that it did say that somewhere.

Where would you expect to find that?

regards, tom lane


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

[HACKERS] CREATE STATISTICS statistic_type documentation

[Jeff Janes]

The docs for CREATE STATISTICS does not say what happens if the
statistic_type clause is omitted.  It should probably say that the default
action is to create both ndistinct and dependencies.

Cheers,

Jeff

Re: [HACKERS] Cached plans and statement generalization

[Konstantin Knizhnik]

On 10.05.2017 19:11, Konstantin Knizhnik wrote:


Based on the Robert's feedback and Tom's proposal I have implemented 
two new versions of autoprepare patch.


First version is just refactoring of my original implementation: I 
have extracted common code into prepare_cached_plan and 
exec_prepared_plan
function to avoid code duplication. Also I rewrote assignment of 
values to parameters. Now types of parameters are inferred from types 
of literals, so there may be several
prepared plans which are different only by types of parameters. Due to 
the problem with type coercion for parameters, I have to catch errors 
in parse_analyze_varparams.




Attached please find rebased version of the autoprepare patch based on 
Tom's proposal (perform analyze for tree with constant literals and then 
replace them with parameters).

Also I submitted this patch for the Autum commitfest.

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

diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 95c1d3e..0b0642b 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3710,6 +3710,454 @@ raw_expression_tree_walker(Node *node,
 }
 
 /*
+ * raw_expression_tree_mutator --- transform raw parse tree.
+ *
+ * This function is implementing slightly different approach for tree update than expression_tree_mutator().
+ * Callback is given pointer to pointer to the current node and can update this field instead of returning reference to new node.
+ * It makes it possible to remember changes and easily revert them without extra traversal of the tree.
+ *
+ * This function do not need QTW_DONT_COPY_QUERY flag: it never implicitly copy tree nodes, doing in-place update.
+ *
+ * Like raw_expression_tree_walker, there is no special rule about query
+ * boundaries: we descend to everything that's possibly interesting.
+ *
+ * Currently, the node type coverage here extends only to DML statements
+ * (SELECT/INSERT/UPDATE/DELETE) and nodes that can appear in them, because
+ * this is used mainly during analysis of CTEs, and only DML statements can
+ * appear in CTEs. If some other node is visited, iteration is immediately stopped and true is returned.
+ */
+bool
+raw_expression_tree_mutator(Node *node,
+			bool (*mutator) (),
+			void *context)
+{
+	ListCell   *temp;
+
+	/*
+	 * The walker has already visited the current node, and so we need only
+	 * recurse into any sub-nodes it has.
+	 */
+	if (node == NULL)
+		return false;
+
+	/* Guard against stack overflow due to overly complex expressions */
+	check_stack_depth();
+
+	switch (nodeTag(node))
+	{
+		case T_SetToDefault:
+		case T_CurrentOfExpr:
+		case T_Integer:
+		case T_Float:
+		case T_String:
+		case T_BitString:
+		case T_Null:
+		case T_ParamRef:
+		case T_A_Const:
+		case T_A_Star:
+			/* primitive node types with no subnodes */
+			break;
+		case T_Alias:
+			/* we assume the colnames list isn't interesting */
+			break;
+		case T_RangeVar:
+			return mutator(&((RangeVar *) node)->alias, context);
+		case T_GroupingFunc:
+			return mutator(&((GroupingFunc *) node)->args, context);
+		case T_SubLink:
+			{
+SubLink	   *sublink = (SubLink *) node;
+
+if (mutator(>testexpr, context))
+	return true;
+/* we assume the operName is not interesting */
+if (mutator(>subselect, context))
+	return true;
+			}
+			break;
+		case T_CaseExpr:
+			{
+CaseExpr   *caseexpr = (CaseExpr *) node;
+
+if (mutator(>arg, context))
+	return true;
+/* we assume mutator(& doesn't care about CaseWhens, either */
+foreach(temp, caseexpr->args)
+{
+	CaseWhen   *when = (CaseWhen *) lfirst(temp);
+
+	Assert(IsA(when, CaseWhen));
+	if (mutator(>expr, context))
+		return true;
+	if (mutator(>result, context))
+		return true;
+}
+if (mutator(>defresult, context))
+	return true;
+			}
+			break;
+		case T_RowExpr:
+			/* Assume colnames isn't interesting */
+			return mutator(&((RowExpr *) node)->args, context);
+		case T_CoalesceExpr:
+			return mutator(&((CoalesceExpr *) node)->args, context);
+		case T_MinMaxExpr:
+			return mutator(&((MinMaxExpr *) node)->args, context);
+		case T_XmlExpr:
+			{
+XmlExpr	   *xexpr = (XmlExpr *) node;
+
+if (mutator(>named_args, context))
+	return true;
+/* we assume mutator(& doesn't care about arg_names */
+if (mutator(>args, context))
+	return true;
+			}
+			break;
+		case T_NullTest:
+			return mutator(&((NullTest *) node)->arg, context);
+		case T_BooleanTest:
+			return mutator(&((BooleanTest *) node)->arg, context);
+		case T_JoinExpr:
+			{
+JoinExpr   *join = (JoinExpr *) node;
+
+if (mutator(>larg, context))
+	return true;
+if (mutator(>rarg, context))
+	return true;
+if (mutator(>quals, context))
+	return true;
+if (mutator(>alias, context))
+	return true;
+/* using list is deemed uninteresting */
+			}
+			

[HACKERS] Walsender timeouts and large transactions

[Petr Jelinek]

Hi,

We have had issue with walsender timeout when used with logical decoding
and the transaction is taking long time to be decoded (because it
contains many changes)

I was looking today at the walsender code and realized that it's because
if the network and downstream are fast enough, we'll always take fast
path in WalSndWriteData which does not do reply or keepalive processing
and is only reached once the transaction has finished by other code. So
paradoxically we die of timeout because everything was fast enough to
never fall back to slow code path.

I propose we only use fast path if the last processed reply is not older
than half of walsender timeout, if it is then we'll force the slow code
path to process the replies again. This is similar logic that we use to
determine if to send keepalive message. I also added CHECK_INTERRUPRS
call to fast code path because otherwise walsender might ignore them for
too long on large transactions.

Thoughts?

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


0001-Fix-walsender-timeouts-when-decoding-large-transacti.patch
Description: binary/octet-stream

-- 
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] Server ignores contents of SASLInitialResponse

[Michael Paquier]

On Thu, May 25, 2017 at 10:52 AM, Michael Paquier
 wrote:
> Actually, I don't think that we are completely done here. Using the
> patch of upthread to enforce a failure on SASLInitialResponse, I see
> that connecting without SSL causes the following error:
> psql: FATAL:  password authentication failed for user "mpaquier"
> But connecting with SSL returns that:
> psql: duplicate SASL authentication request
>
> I have not looked at that in details yet, but it seems to me that we
> should not take pg_SASL_init() twice in the scram authentication code
> path in libpq for a single attempt.

Gotcha. This happens because of sslmode=prefer, on which
pqDropConnection is used to clean up the connection state. So it seems
to me that the correct fix is to move the cleanup of sasl_state to
pqDropConnection() instead of closePGconn(). Once I do so the failures
are correct, showing to the user two FATAL errors because of the two
attempts:
psql: FATAL:  password authentication failed for user "mpaquier"
FATAL:  password authentication failed for user "mpaquier"
-- 
Michael


libpq-sasl-cleanup.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] [POC] hash partitioning

[Robert Haas]

On Mon, May 22, 2017 at 1:49 AM, Ashutosh Bapat
 wrote:
> The prologue is arranged as one paragraph (with a new line) per
> member. Within each paragraph explanation for each partitioning
> strategy starts on its own line. One paragraph per member is more
> readable than separate paragraphs for each member and strategy.

The point is that you can either make it a separate paragraph or you
can make it a single paragraph, but you can't leave it halfway in
between.  If it's one paragraph, every line should end at around the
80 character mark, without any short lines.  If it's multiple
paragraphs, they should be separated by blank lines.  The only line of
a paragraph that can be short is the last one.

-- 
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] Fix performance of generic atomics

[Sokolov Yura]

Hello, Tom.

I agree that lonely semicolon looks bad.
Applied your suggestion for empty loop body (/* skip */).

Patch in first letter had while(true), but I removed it cause
I think it is uglier:
- `while(true)` was necessary for grouping read with `if`,
- but now there is single statement in a loop body and it is
  condition for loop exit, so it is clearly just a loop.

Optimization is valid cause compare_exchange always store old value
in `old` variable in a same atomic manner as atomic read.

Tom Lane wrote 2017-05-25 17:39:

Sokolov Yura  writes:
@@ -382,12 +358,8 @@ static inline uint64
 pg_atomic_fetch_and_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 
and_)

 {
uint64 old;
-   while (true)
-   {
-   old = pg_atomic_read_u64_impl(ptr);
-   if (pg_atomic_compare_exchange_u64_impl(ptr, , old & and_))
-   break;
-   }
+   old = pg_atomic_read_u64_impl(ptr);
+   while (!pg_atomic_compare_exchange_u64_impl(ptr, , old & and_));
return old;
 }
 #endif

FWIW, I do not think that writing the loops like that is good style.
It looks like a typo and will confuse readers.  You could perhaps
write the same code with better formatting, eg

while (!pg_atomic_compare_exchange_u64_impl(ptr, , old & and_))
/* skip */ ;

but why not leave the formulation with while(true) and a break alone?

(I take no position on whether moving the read of "old" outside the
loop is a valid optimization.)

regards, tom lane


With regards,
--
Sokolov Yura aka funny_falcon
Postgres Professional: https://postgrespro.ru
The Russian Postgres CompanyFrom d377e36da652ade715b868a6fc7164736f38d1df Mon Sep 17 00:00:00 2001
From: Sokolov Yura 
Date: Thu, 25 May 2017 14:54:45 +0300
Subject: [PATCH] Fix performance of Atomics generic implementation

pg_atomic_compare_exchange_*_impl stores old value into `old` variable,
so there is no need to reread value in loop body.

Also add gcc specific __sync_fetch_and_(or|and), cause looks like
compiler may optimize code around intrinsic better than around assembler,
although intrinsic is compiled to almost same CAS loop.
---
 src/include/port/atomics/generic-gcc.h | 36 +
 src/include/port/atomics/generic.h | 72 --
 2 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h
index 7efc0861e7..79ada94047 100644
--- a/src/include/port/atomics/generic-gcc.h
+++ b/src/include/port/atomics/generic-gcc.h
@@ -185,6 +185,24 @@ pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
 }
 #endif
 
+#if !defined(PG_HAVE_ATOMIC_FETCH_AND_U32) && defined(HAVE_GCC__SYNC_INT32_CAS)
+#define PG_HAVE_ATOMIC_FETCH_AND_U32
+static inline uint32
+pg_atomic_fetch_and_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 and_)
+{
+	return __sync_fetch_and_and(>value, and_);
+}
+#endif
+
+#if !defined(PG_HAVE_ATOMIC_FETCH_OR_U32) && defined(HAVE_GCC__SYNC_INT32_CAS)
+#define PG_HAVE_ATOMIC_FETCH_OR_U32
+static inline uint32
+pg_atomic_fetch_or_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 or_)
+{
+	return __sync_fetch_and_or(>value, or_);
+}
+#endif
+
 
 #if !defined(PG_DISABLE_64_BIT_ATOMICS)
 
@@ -223,6 +241,24 @@ pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_)
 }
 #endif
 
+#if !defined(PG_HAVE_ATOMIC_FETCH_AND_U64) && defined(HAVE_GCC__SYNC_INT64_CAS)
+#define PG_HAVE_ATOMIC_FETCH_AND_U64
+static inline uint64
+pg_atomic_fetch_and_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 and_)
+{
+	return __sync_fetch_and_and(>value, and_);
+}
+#endif
+
+#if !defined(PG_HAVE_ATOMIC_FETCH_OR_U64) && defined(HAVE_GCC__SYNC_INT64_CAS)
+#define PG_HAVE_ATOMIC_FETCH_OR_U64
+static inline uint64
+pg_atomic_fetch_or_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 or_)
+{
+	return __sync_fetch_and_or(>value, or_);
+}
+#endif
+
 #endif /* !defined(PG_DISABLE_64_BIT_ATOMICS) */
 
 #endif /* defined(HAVE_ATOMICS) */
diff --git a/src/include/port/atomics/generic.h b/src/include/port/atomics/generic.h
index 424543604a..75ffaf6e87 100644
--- a/src/include/port/atomics/generic.h
+++ b/src/include/port/atomics/generic.h
@@ -170,12 +170,9 @@ static inline uint32
 pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 xchg_)
 {
 	uint32 old;
-	while (true)
-	{
-		old = pg_atomic_read_u32_impl(ptr);
-		if (pg_atomic_compare_exchange_u32_impl(ptr, , xchg_))
-			break;
-	}
+	old = pg_atomic_read_u32_impl(ptr);
+	while (!pg_atomic_compare_exchange_u32_impl(ptr, , xchg_))
+		/* skip */;
 	return old;
 }
 #endif
@@ -186,12 +183,9 @@ static inline uint32
 pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
 {
 	uint32 old;
-	while (true)
-	{
-		old = pg_atomic_read_u32_impl(ptr);
-		if (pg_atomic_compare_exchange_u32_impl(ptr, , old + add_))
-			break;
-	}
+	old = 

Re: [HACKERS] Fix performance of generic atomics

[Aleksander Alekseev]

Hi Yura,

> Attached patch contains patch for all generic atomic
> functions, and also __sync_fetch_and_(or|and) for gcc, cause
> I believe GCC optimize code around intrinsic better than
> around inline assembler.
> (final performance is around 86000tps, but difference between
> 83000tps and 86000tps is not so obvious in NUMA system).

I don't see any patch in my email client or pgsql-hackers@ archive. I
would also recommend to add your patch to the nearest commitfest [1].

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

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

Re: [HACKERS] Server ignores contents of SASLInitialResponse

[Michael Paquier]

On Thu, May 25, 2017 at 9:32 AM, Michael Paquier
 wrote:
> On Thu, May 25, 2017 at 8:51 AM, Heikki Linnakangas  wrote:
>> On 05/24/2017 11:33 PM, Michael Paquier wrote:
>>> I have noticed today that the server ignores completely the contents
>>> of SASLInitialResponse. ... Attached is a patch to fix the problem.
>>
>> Fixed, thanks!
>
> Thanks for the commit.

Actually, I don't think that we are completely done here. Using the
patch of upthread to enforce a failure on SASLInitialResponse, I see
that connecting without SSL causes the following error:
psql: FATAL:  password authentication failed for user "mpaquier"
But connecting with SSL returns that:
psql: duplicate SASL authentication request

I have not looked at that in details yet, but it seems to me that we
should not take pg_SASL_init() twice in the scram authentication code
path in libpq for a single attempt.
-- 
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 performance of generic atomics

[Tom Lane]

Sokolov Yura  writes:
@@ -382,12 +358,8 @@ static inline uint64
 pg_atomic_fetch_and_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 and_)
 {
uint64 old;
-   while (true)
-   {
-   old = pg_atomic_read_u64_impl(ptr);
-   if (pg_atomic_compare_exchange_u64_impl(ptr, , old & and_))
-   break;
-   }
+   old = pg_atomic_read_u64_impl(ptr);
+   while (!pg_atomic_compare_exchange_u64_impl(ptr, , old & and_));
return old;
 }
 #endif

FWIW, I do not think that writing the loops like that is good style.
It looks like a typo and will confuse readers.  You could perhaps
write the same code with better formatting, eg

while (!pg_atomic_compare_exchange_u64_impl(ptr, , old & and_))
/* skip */ ;

but why not leave the formulation with while(true) and a break alone?

(I take no position on whether moving the read of "old" outside the
loop is a valid optimization.)

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: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)

[Tom Lane]

Amit Kapila  writes:
> Yes, I also share this opinion, the shm attach failures are due to
> randomization behavior, so sleep won't help much.  So, I will change
> the patch to use 100 retries unless people have other opinions.

Sounds about right to me.

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] No parameter values checking while creating Alter subscription...Connection

[Masahiko Sawada]

On Thu, May 25, 2017 at 9:43 AM, tushar  wrote:
> Hi,
>
> We usually check connection  parameter values while creating create
> subscription
>
> \\port is WRONG
>
> postgres=# create subscription c1 connection 'port=4000 ' publication pub;
> ERROR:  could not connect to the publisher: could not connect to server: No
> such file or directory
> Is the server running locally and accepting
> connections on Unix domain socket "/tmp/.s.PGSQL.4000"?
> postgres=#
>
> \\when database doesn't exist
>
> postgres=# create subscription c1 connection 'dbname=postgre ' publication
> pub;
> ERROR:  could not connect to the publisher: FATAL:  database "postgre" does
> not exist
> postgres=#
>
> but such checking is not done at the time of alter subscription ..
> connection
>
> postgres=# alter subscription c1 connection 'port=4000';
> ALTER SUBSCRIPTION
> postgres=# alter subscription c1 connection 'dbname=cc';
> ALTER SUBSCRIPTION
>

CREATE SUBSCRIPTION tries to connect to publisher to create
replication slot or to get table list for table synchronization, not
to check the connection parameter value. So if you specify connect =
false then CREATE SUBSCRIPTION doesn't try to connect.

Regards,

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


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

[HACKERS] Fix performance of generic atomics

[Sokolov Yura]

Good day, everyone.

I've been played with pgbench on huge machine.
(72 cores, 56 for postgresql, enough memory to fit base
both into shared_buffers and file cache)
(pgbench scale 500, unlogged tables, fsync=off,
synchronous commit=off, wal_writer_flush_after=0).

With 200 clients performance is around 76000tps and main
bottleneck in this dumb test is LWLockWaitListLock.

I added gcc specific implementation for pg_atomic_fetch_or_u32_impl
(ie using __sync_fetch_and_or) and performance became 83000tps.

It were a bit strange at a first look, cause __sync_fetch_and_or
compiles to almost same CAS loop.

Looking closely, I noticed that intrinsic performs doesn't do
read in the loop body, but at loop initialization. It is correct
behavior cause `lock cmpxchg` instruction stores old value in EAX
register.

It is expected behavior, and pg_compare_and_exchange_*_impl does
the same in all implementations. So there is no need to re-read
value in the loop body:

Example diff for pg_atomic_exchange_u32_impl:

 static inline uint32
 pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 
xchg_)

 {
uint32 old;
+   old = pg_atomic_read_u32_impl(ptr);
while (true)
{
-   old = pg_atomic_read_u32_impl(ptr);
if (pg_atomic_compare_exchange_u32_impl(ptr, , xchg_))
break;
}
return old;
 }

After applying this change to all generic atomic functions
(and for pg_atomic_fetch_or_u32_impl ), performance became
equal to __sync_fetch_and_or intrinsic.

Attached patch contains patch for all generic atomic
functions, and also __sync_fetch_and_(or|and) for gcc, cause
I believe GCC optimize code around intrinsic better than
around inline assembler.
(final performance is around 86000tps, but difference between
83000tps and 86000tps is not so obvious in NUMA system).

With regards,
--
Sokolov Yura aka funny_falcon
Postgres Professional: https://postgrespro.ru
The Russian Postgres Company


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

Re: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)

[Amit Kapila]

On Thu, May 25, 2017 at 8:41 AM, Noah Misch  wrote:
> On Thu, May 25, 2017 at 11:41:19AM +0900, Michael Paquier wrote:
>
>> Indeed, pgrename() does so with a 100ms sleep time between each
>> iteration. Perhaps we could do that and limit to 50 iterations?
>
> pgrename() is polling for an asynchronous event, hence the sleep.  To my
> knowledge, time doesn't heal shm attach failures; therefore, a sleep is not
> appropriate here.
>

Yes, I also share this opinion, the shm attach failures are due to
randomization behavior, so sleep won't help much.  So, I will change
the patch to use 100 retries unless people have other opinions.

-- 
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] pg_dump ignoring information_schema tables which used in Create Publication.

[Robert Haas]

On Thu, May 25, 2017 at 8:32 AM, Peter Eisentraut
 wrote:
>> Well, I think if it's not going to work, it should be prohibited,
>> rather than seeming to work but then not actually working.
>
> Here is a similar case that pg_dump fails on:
>
> create table information_schema.test1 (a int);
> create view public.test2 as select * from information_schema.test1;
>
> It's not clear how to address that, or whether it's worth it.

Sure, that case is hard to address.  But why is *this* case hard to address?

-- 
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] No parameter values checking while creating Alter subscription...Connection

[tushar]

Hi,

We usually check connection  parameter values while creating create 
subscription


\\port is WRONG

postgres=# create subscription c1 connection 'port=4000 ' publication pub;
ERROR:  could not connect to the publisher: could not connect to server: 
No such file or directory

Is the server running locally and accepting
connections on Unix domain socket "/tmp/.s.PGSQL.4000"?
postgres=#

\\when database doesn't exist

postgres=# create subscription c1 connection 'dbname=postgre ' 
publication pub;
ERROR:  could not connect to the publisher: FATAL:  database "postgre" 
does not exist

postgres=#

but such checking is not done at the time of alter subscription .. 
connection


postgres=# alter subscription c1 connection 'port=4000';
ALTER SUBSCRIPTION
postgres=# alter subscription c1 connection 'dbname=cc';
ALTER SUBSCRIPTION

--
regards,tushar
EnterpriseDB  https://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] Fix performance of generic atomics

[Sokolov Yura]

A bit cleaner version of a patch.

Sokolov Yura писал 2017-05-25 15:22:

Good day, everyone.

I've been played with pgbench on huge machine.
(72 cores, 56 for postgresql, enough memory to fit base
both into shared_buffers and file cache)
(pgbench scale 500, unlogged tables, fsync=off,
synchronous commit=off, wal_writer_flush_after=0).

With 200 clients performance is around 76000tps and main
bottleneck in this dumb test is LWLockWaitListLock.

I added gcc specific implementation for pg_atomic_fetch_or_u32_impl
(ie using __sync_fetch_and_or) and performance became 83000tps.

It were a bit strange at a first look, cause __sync_fetch_and_or
compiles to almost same CAS loop.

Looking closely, I noticed that intrinsic performs doesn't do
read in the loop body, but at loop initialization. It is correct
behavior cause `lock cmpxchg` instruction stores old value in EAX
register.

It is expected behavior, and pg_compare_and_exchange_*_impl does
the same in all implementations. So there is no need to re-read
value in the loop body:

Example diff for pg_atomic_exchange_u32_impl:

 static inline uint32
 pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 
xchg_)

 {
uint32 old;
+   old = pg_atomic_read_u32_impl(ptr);
while (true)
{
-   old = pg_atomic_read_u32_impl(ptr);
if (pg_atomic_compare_exchange_u32_impl(ptr, , xchg_))
break;
}
return old;
 }

After applying this change to all generic atomic functions
(and for pg_atomic_fetch_or_u32_impl ), performance became
equal to __sync_fetch_and_or intrinsic.

Attached patch contains patch for all generic atomic
functions, and also __sync_fetch_and_(or|and) for gcc, cause
I believe GCC optimize code around intrinsic better than
around inline assembler.
(final performance is around 86000tps, but difference between
83000tps and 86000tps is not so obvious in NUMA system).

With regards,


--
Sokolov Yura aka funny_falcon
Postgres Professional: https://postgrespro.ru
The Russian Postgres CompanyFrom 2c58a32ca89867e7d244e7037a38aa9ccc2b92c2 Mon Sep 17 00:00:00 2001
From: Sokolov Yura 
Date: Thu, 25 May 2017 14:54:45 +0300
Subject: [PATCH] Fix performance of Atomics generic implementation

pg_atomic_compare_exchange_*_impl modifies value of `old` pointer,
so there is no need to reread value in loop body.

Also add gcc specific __sync_fetch_and_(or|and), cause looks like
compiler may optimize code around intrisic better than around assembler,
although intrisic is compiled to almost same CAS loop.
---
 src/include/port/atomics/generic-gcc.h | 36 +++
 src/include/port/atomics/generic.h | 64 +-
 2 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/src/include/port/atomics/generic-gcc.h b/src/include/port/atomics/generic-gcc.h
index 7efc0861e7..79ada94047 100644
--- a/src/include/port/atomics/generic-gcc.h
+++ b/src/include/port/atomics/generic-gcc.h
@@ -185,6 +185,24 @@ pg_atomic_fetch_add_u32_impl(volatile pg_atomic_uint32 *ptr, int32 add_)
 }
 #endif
 
+#if !defined(PG_HAVE_ATOMIC_FETCH_AND_U32) && defined(HAVE_GCC__SYNC_INT32_CAS)
+#define PG_HAVE_ATOMIC_FETCH_AND_U32
+static inline uint32
+pg_atomic_fetch_and_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 and_)
+{
+	return __sync_fetch_and_and(>value, and_);
+}
+#endif
+
+#if !defined(PG_HAVE_ATOMIC_FETCH_OR_U32) && defined(HAVE_GCC__SYNC_INT32_CAS)
+#define PG_HAVE_ATOMIC_FETCH_OR_U32
+static inline uint32
+pg_atomic_fetch_or_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 or_)
+{
+	return __sync_fetch_and_or(>value, or_);
+}
+#endif
+
 
 #if !defined(PG_DISABLE_64_BIT_ATOMICS)
 
@@ -223,6 +241,24 @@ pg_atomic_fetch_add_u64_impl(volatile pg_atomic_uint64 *ptr, int64 add_)
 }
 #endif
 
+#if !defined(PG_HAVE_ATOMIC_FETCH_AND_U64) && defined(HAVE_GCC__SYNC_INT64_CAS)
+#define PG_HAVE_ATOMIC_FETCH_AND_U64
+static inline uint64
+pg_atomic_fetch_and_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 and_)
+{
+	return __sync_fetch_and_and(>value, and_);
+}
+#endif
+
+#if !defined(PG_HAVE_ATOMIC_FETCH_OR_U64) && defined(HAVE_GCC__SYNC_INT64_CAS)
+#define PG_HAVE_ATOMIC_FETCH_OR_U64
+static inline uint64
+pg_atomic_fetch_or_u64_impl(volatile pg_atomic_uint64 *ptr, uint64 or_)
+{
+	return __sync_fetch_and_or(>value, or_);
+}
+#endif
+
 #endif /* !defined(PG_DISABLE_64_BIT_ATOMICS) */
 
 #endif /* defined(HAVE_ATOMICS) */
diff --git a/src/include/port/atomics/generic.h b/src/include/port/atomics/generic.h
index 424543604a..6d2671beab 100644
--- a/src/include/port/atomics/generic.h
+++ b/src/include/port/atomics/generic.h
@@ -170,12 +170,8 @@ static inline uint32
 pg_atomic_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 xchg_)
 {
 	uint32 old;
-	while (true)
-	{
-		old = pg_atomic_read_u32_impl(ptr);
-		if (pg_atomic_compare_exchange_u32_impl(ptr, , xchg_))
-			break;
-	}
+	old = pg_atomic_read_u32_impl(ptr);
+	while 

Re: [HACKERS] Server ignores contents of SASLInitialResponse

[Michael Paquier]

On Thu, May 25, 2017 at 8:51 AM, Heikki Linnakangas  wrote:
> On 05/24/2017 11:33 PM, Michael Paquier wrote:
>> I have noticed today that the server ignores completely the contents
>> of SASLInitialResponse. ... Attached is a patch to fix the problem.
>
> Fixed, thanks!

Thanks for the commit.
-- 
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: Alter subscription..SET - NOTICE message is coming for table which is already removed

[tushar]

On 05/25/2017 03:38 PM, tushar wrote:
While performing - Alter subscription..SET  , I found that NOTICE 
message is coming duplicate next time , which  is not needed anymore. 
There is an another example - where i am getting "ERROR: subscription 
table 16435 in subscription 16684 does not exist" in standby log file


2017-05-25 13:51:48.825 BST [32138] NOTICE:  removed subscription for 
table public.t96
2017-05-25 13:51:48.825 BST [32138] NOTICE:  removed subscription for 
table public.t97
2017-05-25 13:51:48.826 BST [32138] NOTICE:  removed subscription for 
table public.t98
2017-05-25 13:51:48.826 BST [32138] NOTICE:  removed subscription for 
table public.t99
2017-05-25 13:51:48.826 BST [32138] NOTICE:  removed subscription for 
table public.t100
2017-05-25 13:51:48.827 BST [32138] LOG:  duration: 35.404 ms statement: 
alter subscription c1 set publication p1 refresh;
2017-05-25 13:51:49.192 BST [32347] LOG:  starting logical replication 
worker for subscription "c1"
2017-05-25 13:51:49.198 BST [32368] LOG:  logical replication sync for 
subscription c1, table t16 started
2017-05-25 13:51:49.198 BST [32368] ERROR:  subscription table 16429 in 
subscription 16684 does not exist
2017-05-25 13:51:49.199 BST [32347] LOG:  starting logical replication 
worker for subscription "c1"
2017-05-25 13:51:49.200 BST [32065] LOG:  worker process: logical 
replication worker for subscription 16684 sync 16429 (PID 32368) exited 
with exit code 1
2017-05-25 13:51:49.204 BST [32369] LOG:  logical replication sync for 
subscription c1, table t17 started
2017-05-25 13:51:49.204 BST [32369] ERROR:  subscription table 16432 in 
subscription 16684 does not exist
2017-05-25 13:51:49.205 BST [32347] LOG:  starting logical replication 
worker for subscription "c1"
2017-05-25 13:51:49.205 BST [32065] LOG:  worker process: logical 
replication worker for subscription 16684 sync 16432 (PID 32369) exited 
with exit code 1
2017-05-25 13:51:49.209 BST [32370] LOG:  logical replication sync for 
subscription c1, table t18 started
2017-05-25 13:51:49.209 BST [32370] ERROR:  subscription table 16435 in 
subscription 16684 does not exist
2017-05-25 13:51:49.210 BST [32347] LOG:  starting logical replication 
worker for subscription "c1"
2017-05-25 13:51:49.210 BST [32065] LOG:  worker process: logical 
replication worker for subscription 16684 sync 16435 (PID 32370) exited 
with exit code 1
2017-05-25 13:51:49.213 BST [32371] LOG:  logical replication sync for 
subscription c1, table t19 started
2017-05-25 13:51:49.213 BST [32371] ERROR:  subscription table 16438 in 
subscription 16684 does not exist
2017-05-25 13:51:49.214 BST [32347] LOG:  starting logical replication 
worker for subscription "c1"



Steps to reproduce -
X cluster ->
create 100 tables , publish all tables (create publication pub for table 
t1,t2,t2..t100;)
create one more table (create table t101(n int), create publication , 
publish only that table (create publication p1 for table t101;)


Y Cluster ->
create subscription (create subscription c1 connection 'host=localhost  
port=5432 user=centos ' publication pub;

alter subscription c1 set publication p1 refresh;
alter subscription c1 set publication pub refresh;
alter subscription c1 set publication p1 refresh;

check the log file.

--
regards,tushar
EnterpriseDB  https://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] Server ignores contents of SASLInitialResponse

[Heikki Linnakangas]

On 05/24/2017 11:33 PM, Michael Paquier wrote:

I have noticed today that the server ignores completely the contents
of SASLInitialResponse. ... Attached is a patch to fix the problem.


Fixed, 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] pg_dump ignoring information_schema tables which used in Create Publication.

[Peter Eisentraut]

On 5/24/17 21:36, Robert Haas wrote:
> On Wed, May 24, 2017 at 7:16 PM, Peter Eisentraut
>  wrote:
>> On 5/22/17 07:42, Kuntal Ghosh wrote:
>>> pg_dump ignores anything created under object name "pg_*" or
>>> "information_schema".
>>
>> Publications have a slightly different definition of what tables to
>> ignore/prohibit than pg_dump, partly because they have more built-in
>> knowledge.  I'm not sure whether it's worth fixing this.
> 
> Well, I think if it's not going to work, it should be prohibited,
> rather than seeming to work but then not actually working.

Here is a similar case that pg_dump fails on:

create table information_schema.test1 (a int);
create view public.test2 as select * from information_schema.test1;

It's not clear how to address that, or whether it's worth it.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

[HACKERS] Allow GiST opcalsses without compress\decompres functions

[Andrew Borodin]

Hi, hackers!

Currently, GiST stores each attribute in a compressed form.
That is, each time attribute is written it's calling compress
function, and when the attribute is accessed the decompress functions
is called.
Some types can't get any advantage out of this technique since the
context of one value is not enough for seeding effective compression
algorithm.
And we have some of the compress functions like this
Datum
gist_box_compress(PG_FUNCTION_ARGS)
{
PG_RETURN_POINTER(PG_GETARG_POINTER(0));
}

https://github.com/postgres/postgres/blob/master/src/backend/access/gist/gistproc.c#L195
https://github.com/postgres/postgres/blob/master/src/backend/utils/adt/rangetypes_gist.c#L221
https://github.com/postgres/postgres/blob/master/contrib/seg/seg.c#L255
https://github.com/postgres/postgres/blob/master/contrib/cube/cube.c#L384

Maybe we should make compress\decompress functions optional?
Also, this brings some bit of performance.
For the attached test I observe 6% faster GiST build and 4% faster scans.
Not a big deal, but something. How do you think?

In some cases, there are strange things in the code of
compress\decompress. E.g. cube's decompress function is detoasting
entry twice, to be very sure :)



Best regards, Andrey Borodin, Octonica.
CREATE EXTENSION IF NOT EXISTS CUBE ;

DROP TABLE IF EXISTS TESTTABLE ;
DROP OPERATOR CLASS IF EXISTS gist_cube_ops_nocompress USING GIST;

CREATE OPERATOR CLASS gist_cube_ops_nocompress
FOR TYPE CUBE USING GIST AS
OPERATOR3   && ,
OPERATOR6   = ,
OPERATOR7   @> ,
OPERATOR8   <@ ,
OPERATOR13  @ ,
OPERATOR14  ~ ,

FUNCTION1   g_cube_consistent (INTERNAL, CUBE, SMALLINT, 
OID, INTERNAL),
FUNCTION2   g_cube_union (INTERNAL, INTERNAL),
--we do not have functions 3 and for (compress and decompress)
FUNCTION5   g_cube_penalty (INTERNAL, INTERNAL, INTERNAL),
FUNCTION6   g_cube_picksplit (INTERNAL, INTERNAL),
FUNCTION7   g_cube_same (CUBE, CUBE, INTERNAL),
FUNCTION8   g_cube_distance (INTERNAL, CUBE, SMALLINT, OID, 
INTERNAL),
FUNCTION9   g_cube_decompress (INTERNAL);--fetch function, 
not for compression



BEGIN;
SELECT SETSEED(0.5);

CREATE TABLE TESTTABLE AS SELECT CUBE(RANDOM()) C FROM 
GENERATE_SERIES(1,50) I;

\timing

--testing time to create compressed index
CREATE INDEX COMPRESSED ON TESTTABLE USING GIST(C gist_cube_ops);
--testing time to create uncompressed index
CREATE INDEX UNCOMPRESSED ON TESTTABLE USING GIST(C gist_cube_ops_nocompress);

\timing
set enable_bitmapscan = false;

UPDATE PG_INDEX SET INDISVALID = FALSE WHERE INDEXRELID = 
'UNCOMPRESSED'::REGCLASS;
UPDATE PG_INDEX SET INDISVALID = TRUE WHERE INDEXRELID = 'COMPRESSED'::REGCLASS;

EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) 
from generate_series(1,50) b;
EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) 
from generate_series(1,50) b;

UPDATE PG_INDEX SET INDISVALID = TRUE WHERE INDEXRELID = 
'UNCOMPRESSED'::REGCLASS;
UPDATE PG_INDEX SET INDISVALID = FALSE WHERE INDEXRELID = 
'COMPRESSED'::REGCLASS;

EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) 
from generate_series(1,50) b;
EXPLAIN ANALYZE select (SELECT COUNT(*) FROM TESTTABLE WHERE C <@ CUBE(-b,1)) 
from generate_series(1,50) b;

COMMIT;





0001-Allow-uncompressed-GiST.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] Alter subscription..SET - NOTICE message is coming for table which is already removed

[tushar]

On 05/25/2017 04:40 PM, Masahiko Sawada wrote:

I think you did ALTER SUBSCRIPTION while table sync for 100 tables is
running, right?

Yes, i didn't wait too much while executing the commands.

--
regards,tushar
EnterpriseDB  https://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] Alter subscription..SET - NOTICE message is coming for table which is already removed

[Masahiko Sawada]

On Thu, May 25, 2017 at 6:08 AM, tushar  wrote:
> Hi,
>
> While performing - Alter subscription..SET  , I found that NOTICE message is
> coming duplicate next time , which  is not needed anymore.
>
> X cluster=
> create 100 tables
> create publication ( create publication pub for all tables;)
>
> Y cluster=
> create 100 tables
> create subscription ( create subscription sub connection 'dbname=postgres
> user=centos host=localhost ) publication pub;
>
> X cluster =
> create 1 more table (create table r(n int));
> create publication for this above table only (create publication pub1 for
> table r;)
>
> Y cluster=
> create table r - create table r(n int);
> alter publication -alter subscription sub set publication pub1 refresh;
>
> in the notice message -> table 'r' added / 100 tables removed from the
> subscription

I think you did ALTER SUBSCRIPTION while table sync for 100 tables is
running, right?

>
> postgres=# alter subscription sub set publication pub1 refresh;
> NOTICE:  added subscription for table public.r
> NOTICE:  removed subscription for table public.t1
> NOTICE:  removed subscription for table public.t2
> NOTICE:  removed subscription for table public.t3
> NOTICE:  removed subscription for table public.t4
> NOTICE:  removed subscription for table public.t5
> NOTICE:  removed subscription for table public.t6
> --
> --
> --
> ALTER SUBSCRIPTION
>
> now again fire the same sql query
>
> postgres=# alter subscription sub set publication pub1 refresh;
> NOTICE:  removed subscription for table public.t78
> ALTER SUBSCRIPTION
>
> This notice message should not  come  as t.78 is already removed from
> earlier same command.
>

I'm investigating the cause of this issue but it seems to me that
first ALTER SUBSCRIPTION surely removed t78 record from
pg_subscription_rel but table sync worker inserts new record of t78
when updating its state. SetSubscriptionRelState inserts a new record
if the table doesn't exist otherwise updates the record but we can
divide it into two different functions and call appropriate function
in each place.

Regards,

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


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

Re: [HACKERS] TPC-H Q20 from 1 hour to 19 hours!

[Tomas Vondra]

Hi,

On 5/25/17 6:03 AM, Robert Haas wrote:

On Thu, Apr 6, 2017 at 4:37 PM, Tomas Vondra
 wrote:

Which brings me to the slightly suspicious bit. On 9.5, there's no
difference between GROUP and GROUP+LIKE cases - the estimates are exactly
the same in both cases. This is true too, but only without the foreign key
between "partsupp" and "part", i.e. the two non-grouped relations in the
join. And what's more, the difference (1737 vs. 16) is pretty much exactly
100x, which is the estimate for the LIKE condition.


I don't follow this.  How does the foreign key between partsupp and
part change the selectivity of LIKE?


So it kinda seems 9.5 does not apply this condition for semi-joins, while

=9.6 does that.




Well, get_foreign_key_join_selectivity() does handle restrictions when 
calculating joinrel size estimate in calc_joinrel_size_estimate(), so 
assuming there's some thinko it might easily cause this.


I haven't found any such thinko, but I don't dare to claim I fully 
understand what the current version of get_foreign_key_join_selectivity 
does :-/



If 9.5 and prior are ignoring some of the quals, that's bad, but I
don't think I understand from your analysis why
7fa93eec4e0c9c3e801e3c51aa4bae3a38aaa218 regressed anything.



It's been quite a bit of time since I looked into this, but I think my 
main point was that it's hard to say it's a regression when both the old 
and new estimates are so utterly wrong.


I mean, 9.5 estimates 160, 9.6 estimates 18. We might fix the post-9.6 
estimate to return the same value as 9.5, and it might fix this 
particular query with this particular scale. But in the end it's just 
noise considering that the actual value is 120k (so 3 or 4 orders of 
magnitude off).



regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

[HACKERS] Alter subscription..SET - NOTICE message is coming for table which is already removed

[tushar]

Hi,

While performing - Alter subscription..SET  , I found that NOTICE 
message is coming duplicate next time , which  is not needed anymore.


X cluster=
create 100 tables
create publication ( create publication pub for all tables;)

Y cluster=
create 100 tables
create subscription ( create subscription sub connection 
'dbname=postgres user=centos host=localhost ) publication pub;


X cluster =
create 1 more table (create table r(n int));
create publication for this above table only (create publication pub1 
for table r;)


Y cluster=
create table r - create table r(n int);
alter publication -alter subscription sub set publication pub1 refresh;

in the notice message -> table 'r' added / 100 tables removed from the 
subscription


postgres=# alter subscription sub set publication pub1 refresh;
NOTICE:  added subscription for table public.r
NOTICE:  removed subscription for table public.t1
NOTICE:  removed subscription for table public.t2
NOTICE:  removed subscription for table public.t3
NOTICE:  removed subscription for table public.t4
NOTICE:  removed subscription for table public.t5
NOTICE:  removed subscription for table public.t6
--
--
--
ALTER SUBSCRIPTION

now again fire the same sql query

postgres=# alter subscription sub set publication pub1 refresh;
NOTICE:  removed subscription for table public.t78
ALTER SUBSCRIPTION

This notice message should not  come  as t.78 is already removed from 
earlier same command.


--
regards,tushar
EnterpriseDB  https://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] Adding support for Default partition in partitioning

[Jeevan Ladhe]

Forgot to attach the patch.
PFA.

On Thu, May 25, 2017 at 3:02 PM, Jeevan Ladhe  wrote:

> Hi Rajkumar,
>
> postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a);
>> CREATE TABLE
>> postgres=# CREATE TEMP TABLE temp_def_part (a int);
>> CREATE TABLE
>> postgres=# ALTER TABLE temp_list_part ATTACH PARTITION temp_def_part
>> DEFAULT;
>> server closed the connection unexpectedly
>> This probably means the server terminated abnormally
>> before or while processing the request.
>> The connection to the server was lost. Attempting reset: Failed.
>> !>
>>
>
> Thanks for reporting.
> PFA patch that fixes above issue.
>
> Regards,
> Jeevan Ladhe
>


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

[Jeevan Ladhe]

Hi Rajkumar,

postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a);
> CREATE TABLE
> postgres=# CREATE TEMP TABLE temp_def_part (a int);
> CREATE TABLE
> postgres=# ALTER TABLE temp_list_part ATTACH PARTITION temp_def_part
> DEFAULT;
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> !>
>

Thanks for reporting.
PFA patch that fixes above issue.

Regards,
Jeevan Ladhe

Re: [HACKERS] wal_level > WAL_LEVEL_LOGICAL

[Neha Khatri]

On Wed, 24 May 2017 at 10:29 pm, Robert Haas  wrote:

> On Mon, May 22, 2017 at 9:08 AM, Neha Khatri 
> wrote:
> > As per my understabding, current postgres server supports only three
> > values for wal_level i.e. 'minimal' , 'replica' or 'logical'. But
> > following error message brought to notice that there are various code
> > spots that try to look for wal_level >= WAL_LEVEL_LOGICAL:
>
> I suspect that this was intended as future-proofing.  I think it's
> actually very reasonable to write the internal tests that way,


Agreed. Share the same thought and also started another thread just for the
user visible error message improvement [1]. In that thread the error
message is perceived to be correct.

but it
> does seem strange that it's crept into the user-visible error
> messages.


Yep, this seems useful for developer but not the end user.

[1]
https://www.postgresql.org/message-id/CAFO0U%2B_y8AyAcQLiF3S1i6yCNuYrcLNEd-BbzCuHiGOSejW%3D2A%40mail.gmail.com

Regards,
Neha
-- 
Cheers,
Neha

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

[tushar]

On 05/25/2017 12:44 AM, Petr Jelinek wrote:

There is still outstanding issue that sync worker will keep running
inside the long COPY because the invalidation messages are also not
processed until it finishes but all the original issues reported here
disappear for me with the attached patches applied.
After applying all your patches, drop subscription no  more hangs while 
dropping  subscription but there is an error   "ERROR:  tuple 
concurrently updated" in the log file of

standby.

---
logical replication synchronization worker finished processing
2017-05-25 09:15:52.654 BST [18575] LOG:  logical replication 
synchronization worker finished processing
2017-05-25 09:15:52.656 BST [18563] LOG:  starting logical replication 
worker for subscription "sub"
2017-05-25 09:15:52.662 BST [18577] LOG:  logical replication sync for 
subscription sub, table t14 started
2017-05-25 09:15:53.657 BST [18563] LOG:  starting logical replication 
worker for subscription "sub"
2017-05-25 09:15:53.663 BST [18579] LOG:  logical replication sync for 
subscription sub, table t15 started
2017-05-25 09:15:53.724 BST [18563] FATAL:  terminating logical 
replication worker due to administrator command
2017-05-25 09:15:53.725 BST [18521] LOG:  worker process: logical 
replication worker for subscription 16684 (PID 18563) exited with exit 
code 1

2017-05-25 09:15:54.734 BST [18579] ERROR:  tuple concurrently updated
2017-05-25 09:15:54.735 BST [18577] ERROR:  tuple concurrently updated
2017-05-25 09:15:54.736 BST [18521] LOG:  worker process: logical 
replication worker for subscription 16684 sync 16426 (PID 18579) exited 
with exit code 1
2017-05-25 09:15:54.736 BST [18521] LOG:  worker process: logical 
replication worker for subscription 16684 sync 16423 (PID 18577) exited 
with exit code 1

~
~
~

Steps to reproduce -
X cluster -> create 100 tables , publish all tables  (create publication 
pub for all tables);
Y Cluster -> create 100 tables ,create subscription(create subscription 
sub connection 'user=centos host=localhost' publication pub;

Y cluster ->drop subscription - drop subscription sub;

check the log file on Y cluster.

Sometime , i have seen this error on psql prompt and drop subscription 
operation got failed at first attempt.


postgres=# drop subscription sub;
ERROR:  tuple concurrently updated
postgres=# drop subscription sub;
NOTICE:  dropped replication slot "sub" on publisher
DROP SUBSCRIPTION

--
regards,tushar
EnterpriseDB  https://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] Adding support for Default partition in partitioning

[Rajkumar Raghuwanshi]

On Thu, May 25, 2017 at 12:10 PM, Jeevan Ladhe <
jeevan.la...@enterprisedb.com> wrote:

> PFA.
>

Hi

I have applied v13 patch, got a crash when trying to attach default temp
partition.

postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a);
CREATE TABLE
postgres=# CREATE TEMP TABLE temp_def_part (a int);
CREATE TABLE
postgres=# ALTER TABLE temp_list_part ATTACH PARTITION temp_def_part
DEFAULT;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>

Thanks & Regards,
Rajkumar Raghuwanshi
QMG, EnterpriseDB Corporation

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

[Jeevan Ladhe]

Hi,

I started looking into Rahila's default_partition_v11.patch, and reworked on
few things as below:

- I tried to cover all the review comments posted on the thread. Do let
me know if something is missing.

- Got rid of the functions get_qual_for_default() and
generate_qual_for_defaultpart().
There is no need of collecting boundspecs of all the partitions in case of
list
partition, the list is available in boundinfo->ndatums, an expression for
default can be created from the information that is available in boundinfo.

- Got rid of variable has_default, and added a macro for it.

- Changed the logic of checking the overlapping of existing rows in default
partition. Earlier version of patch used to build new constraints for
default
partition table and then was checking if any of existing rows violate those
constraints. However, current version of patch just checks if any of the
rows in
default partition satisfy the new partition's constraint and fail if there
exists any.
This logic can also be used as it is for default partition in case of RANGE
partitioning.

- Simplified grammar rule.

- Got rid of PARTITION_DEFAULT since DEFAULT is not a different partition
strategy, the applicable logic is also revised:

- There are few other code adjustments like: indentation, commenting, code
simplification etc.

- Added regression tests.

TODO:
Documentation, I am working on it. Will updated the patch soon.

PFA.

Regards,
Jeevan

On Mon, May 22, 2017 at 7:31 AM, Beena Emerson 
wrote:

> Hello,
>
> Patch for default range partition has been added. PFA the rebased v12
> patch for the same.
> I have not removed the has_default variable yet.
>
> Default range partition: https://www.postgresql.org/message-id/
> CAOG9ApEYj34fWMcvBMBQ-YtqR9fTdXhdN82QEKG0SVZ6zeL1xg%40mail.gmail.com
> --
>
> Beena Emerson
>
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>


default_partition_v13.patch
Description: Binary data

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

Re: [HACKERS] Broken hint bits (freeze)

[Vladimir Borodin]

> 24 мая 2017 г., в 15:44, Robert Haas  написал(а):
> 
> On Wed, May 24, 2017 at 7:27 AM, Dmitriy Sarafannikov
>  wrote:
>> It seems like replica did not replayed corresponding WAL records.
>> Any thoughts?
> 
> heap_xlog_freeze_page() is a pretty simple function.  It's not
> impossible that it could have a bug that causes it to incorrectly skip
> records, but it's not clear why that wouldn't affect many other replay
> routines equally, since the pattern of using the return value of
> XLogReadBufferForRedo() to decide what to do is widespread.
> 
> Can you prove that other WAL records generated around the same time as
> the freeze record *were* replayed on the master?  If so, that proves
> that this isn't just a case of the WAL never reaching the standby.
> Can you look at the segment that contains the relevant freeze record
> with pg_xlogdump?  Maybe that record is messed up somehow.

Not yet. Most of such cases are long before our recovery window so 
corresponding WALs have been deleted. We have already tuned retention policy 
and we are now looking for a fresh case.

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


--
May the force be with you…
https://simply.name