[HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[David G. Johnston]

On Thursday, June 15, 2017, Tom Lane  wrote:

> Michael Paquier  writes:
> > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> >> (2) My inclination would be not to back-patch.  This change could break
> >> configurations that worked before, and the lack of prior complaints
> >> says that not many people are having a problem with it.
>
> > That's fourteen years without complains, still I cannot imagine any
> > cases where it would be a problem as people who would have faced this
> > problem but not reported it have likely just enforced the FS to handle
> > case-insensitivity for paths.
>
> Well, it's not just about case sensitivity.  The comment for
> SplitDirectoriesString points out that it deals with embedded spaces
> differently, and it also applies canonicalize_path().  I'm too tired
> to think hard about what that part might mean for compatibility, but
> it probably isn't nothing.
>
> Anyway, I agree that this is an appropriate change for HEAD.  Just
> not convinced that we should shove it into minor releases.
>
>
If it's downcasing then careless use of mixed case when the true path on a
case sensitive filesystem is all lower case (not unlikely at all given
default packaging for deb and rpm packages, iirc) would indeed be a
problem.  Those paths are accidentally working right now.  I vote for no
back-patch.

David J.

Re: [HACKERS] Decimal64 and Decimal128

[Craig Ringer]

On 16 June 2017 at 05:42, Thomas Munro  wrote:
> On Fri, Sep 25, 2015 at 5:06 PM, Pavel Stehule  
> wrote:
>> 2015-09-25 0:25 GMT+02:00 Jim Nasby :
>>>
>>> On 9/24/15 3:35 PM, Peter Geoghegan wrote:

 I would worry about the implicit casts you've added. They might cause
 problems.
>>>
>>>
>>> Given the cycle created between numeric->decimal and decimal->numeric, I
>>> can pretty much guarantee they will. In any case, I don't think implicit
>>> casting from numeric->decimal is a good idea since it can overflow. I'm not
>>> sure that the other direction is safe either... I can't remember offhand if
>>> casting correctly obeys typmod or not.
>>>
>>> BTW, have you talked to Pavel about making these changes to his code?
>>> Seems a shame to needlessly fork it. :/
>>
>>
>> yes, he talked with me, and I gave a agreement to continue/enhance/fork this
>> project how will be necessary
>
> Bumping this ancient thread to say that DECFLOAT appears to have
> landed in the SQL standard.  I haven't looked at SQL:2016 myself by I
> just saw this on Markus Winand's Modern SQL blog:
>
> "There is a new type decfloat[()] (T076)."
>
> http://modern-sql.com/blog/2017-06/whats-new-in-sql-2016
>
> So far it's supported only by DB2 (inventor) and FirebirdSQL has just
> announced support in the next release.

I was pretty excited by decimal floating point initially, but the lack
of support for its use in hardware in commonplace CPUs makes me less
thrilled. IIRC Intel was talking about adding it, but I can't find any
references to that anymore. POWER6 and POWER7 has it, which is great,
but hardly justifies a push for getting it into the core Pg.

Some of the discussion on
https://software.intel.com/en-us/articles/intel-decimal-floating-point-math-library?page=1
suggests that doing it fully in hardware is very expensive, so a mixed
software/microcode implementation with some hardware assistance is
likely if/when it comes.

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


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

Re: [HACKERS] Broken hint bits (freeze)

[Sergey Burladyan]

Bruce Momjian  writes:

> !  against the old primary and standby clusters.  Verify that the
> !  Latest checkpoint location values match in all clusters.

For "Log-Shipping only" standby server this cannot be satisfied, because
last WAL from master (with shutdown checkpoint) never archived. For
example (git master):
 postgresql.conf ===
port = 5430
shared_buffers = 32MB
wal_level = hot_standby
archive_mode = on
archive_command = 'test ! -f "$ARH/%f" && ( echo "arch %p"; cp %p "$ARH/%f"; )'
max_wal_senders = 5
hot_standby = on
log_line_prefix = '%t '
log_checkpoints = on
lc_messages = C


 pg_control 
pg_control version number:1002
Catalog version number:   201705301
Database system identifier:   6432034080221219745
Database cluster state:   shut down
pg_control last modified: Fri Jun 16 03:57:22 2017
Latest checkpoint location:   0/D28
Prior checkpoint location:0/1604878
Latest checkpoint's REDO location:0/D28
Latest checkpoint's REDO WAL file:0001000D


 WALs archive 
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010003
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010004
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010005
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010006
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010007
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010008
-rw--- 1 sergey users 16777216 Jun 16 03:57 00010009
-rw--- 1 sergey users 16777216 Jun 16 03:57 0001000A
-rw--- 1 sergey users 16777216 Jun 16 03:57 0001000B
-rw--- 1 sergey users 16777216 Jun 16 03:57 0001000C
==

 logfile 
arch pg_wal/0001000A
arch pg_wal/0001000B
2017-06-16 00:57:21 GMT LOG:  received fast shutdown request
2017-06-16 00:57:21 GMT LOG:  aborting any active transactions
2017-06-16 00:57:21 GMT LOG:  shutting down
arch pg_wal/0001000C
2017-06-16 00:57:21 GMT LOG:  checkpoint starting: shutdown immediate
2017-06-16 00:57:22 GMT LOG:  checkpoint complete: wrote 4058 buffers (99.1%); 
0 WAL file(s) added, 0 removed, 0 recycled; write=0.033 s, sync=0.949 s, 
total=1.144 s; sync files=32, longest=0.598 s, average=0.029 s; distance=190445 
kB, estimate=190445 kB
2017-06-16 00:57:22 GMT LOG:  database system is shut down
=

There is no 0001000D in archive and after shutdown,
standby can only be at it previous restartpoint (0/1604878) because it
does not receive latest checkpoint (0/D28) from master.

So, after shutdown master and "Log-Shipping only" standby, it always "one
checkpoint early" then master and "Latest checkpoint location" never
match for it.

I think this must be mentioned somehow in documentation. 


> ! 
> !  Also, if upgrading standby servers, change wal_level
> !  to replica in the postgresql.conf file on
> !  the new cluster.
>   
>  

I am not sure how this help.

wal_level is reset by pg_resetxlog during pg_upgrade, so it does not
depend on postgresql.conf. After pg_upgrade wal_level always is
'minimal', that is why you must start and stop new master before rsync:

 output 
$ "$bin"/pg_controldata "$ver" | grep wal_level
wal_level setting:replica

$ "$bin"/pg_resetwal "$ver"
Write-ahead log reset

$ "$bin"/pg_controldata "$ver" | grep wal_level
wal_level setting:minimal


If you rsync standby now (without start/stop new master after pg_upgrade)
you will send pg_control with wal_level=minimal into it and after that
standby abort on startup:
 standby logfile 
2017-06-16 01:22:14 GMT LOG:  entering standby mode
2017-06-16 01:22:14 GMT WARNING:  WAL was generated with wal_level=minimal, 
data may be missing
2017-06-16 01:22:14 GMT HINT:  This happens if you temporarily set 
wal_level=minimal without taking a new base backup.
2017-06-16 01:22:14 GMT FATAL:  hot standby is not possible because wal_level 
was not set to "replica" or higher on the master server
2017-06-16 01:22:14 GMT HINT:  Either set wal_level to "replica" on the master, 
or turn off hot_standby here.
2017-06-16 01:22:14 GMT LOG:  startup process (PID 27916) exited with exit code 
1
=

PS: Thank you for answer, Bruce!

-- 
Sergey Burladyan


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

[HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[QL Zhuo]

I just put this line in my postgresql.conf:

```
shared_preload_libraries = '/Path/Contains/UpCaseWords/an_ext.so'
```

Then the server couldn't start. It tried to load the file
"/path/contains/upcasewords/an_ext.so" and failed.

After few digging, I found there's a wrong use of `SplitIdentifierString`
in function `load_libraries` in /src/backend/utils/init/miscinit.c, and the
attached patch fixes it.


--
This email address (zhuo.devgmail.com) is only for development affairs,
e.g. mail list, please mail to zhuohexoasis.com or zhuoqlzoho.com
for other purpose.

ZHUO QL (KDr2), http://kdr2.com


0001-Don-t-downcase-filepath-in-load_libraries.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] Something is rotten in publication drop

[Peter Eisentraut]

On 6/15/17 12:23, Tom Lane wrote:
> It strikes me that you could rewrite psql's query to just do its own
> catalog search and not bother with the function at all.  It would have
> to know a bit more about the catalog structure than it does now, but not
> that much:
> 
> select pub.pubname from pg_catalog.pg_publication pub
> where puballtables or
>   exists(select 1 from pg_catalog.pg_publication_rel r
>  where r.prpubid = pub.oid and r.prrelid = '%s');

We used to do something like that, but then people complained that that
was not absolutely accurate, because it did not exclude catalog tables
and related things properly.  See commit
2d460179baa8744e9e2a183a5121306596c53fba.  To do this properly, you need
to filter pg_class using is_publishable_class() (hitherto internal C
function).

The way this was originally written was for use by subscriptioncmds.c
fetch_table_list(), which generally only deals with a small number of
publications as the search key and wants to find all the relations.  The
psql use case is exactly the opposite: We start with a relation and want
to find all the publications.  The third use case is that we document
the view pg_publication_tables for general use, so depending on which
search key you start with, you might get terrible performance if you
have a lot of tables.

An academically nice way to write a general query for this would be:

CREATE VIEW pg_publication_tables AS
 SELECT
 p.pubname AS pubname,
 n.nspname AS schemaname,
 c.relname AS tablename,
 c.oid AS relid
 FROM pg_publication p
  JOIN pg_publication_rel pr ON p.oid = pr.prpubid
  JOIN pg_class c ON pr.prrelid = c.oid
  JOIN pg_namespace n ON c.relnamespace = n.oid
 UNION
 SELECT
 p.pubname AS pubname,
 n.nspname AS schemaname,
 c.relname AS tablename,
 c.oid AS relid
 FROM pg_publication p
  JOIN pg_class c ON p.puballtables AND
pg_is_relation_publishable(c.oid)
  JOIN pg_namespace n ON c.relnamespace = n.oid;

But looking at the plans this generates, it will do a sequential scan of
pg_class even if you look for a publication that is not puballtables,
which would suck for the subscriptioncmds.c use case.

We could use the above definition for the documented view and the psql
use case.

We could then create second view that uses the existing definition

CREATE VIEW pg_publication_tables AS
SELECT
P.pubname AS pubname,
N.nspname AS schemaname,
C.relname AS tablename
FROM pg_publication P, pg_class C
 JOIN pg_namespace N ON (N.oid = C.relnamespace)
WHERE C.oid IN (SELECT relid FROM pg_get_publication_tables(P.pubname));

for use in subscriptioncmds.c.  Or don't use a view for that.  But the
view is useful because we should preserve this interface across versions.

Or we throw away all the views and use custom code everywhere.

Patch for experimentation attached.

Any ideas?


-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 855aff09b862fab7bf8ca49b3b653b296a124484 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Thu, 15 Jun 2017 23:18:39 -0400
Subject: [PATCH] WIP Tweak publication fetching in psql

---
 src/backend/catalog/pg_publication.c | 16 
 src/backend/catalog/system_views.sql | 23 +--
 src/bin/psql/describe.c  |  5 ++---
 src/include/catalog/pg_proc.h|  2 ++
 4 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/backend/catalog/pg_publication.c 
b/src/backend/catalog/pg_publication.c
index 17105f4f2c..fccf642605 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -105,6 +105,22 @@ is_publishable_class(Oid relid, Form_pg_class reltuple)
relid >= FirstNormalObjectId;
 }
 
+Datum
+pg_is_relation_publishable(PG_FUNCTION_ARGS)
+{
+   Oid relid = PG_GETARG_OID(0);
+   HeapTuple tuple;
+   bool result;
+
+   tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+   if (!tuple)
+   PG_RETURN_NULL();
+   result = is_publishable_class(relid, (Form_pg_class) GETSTRUCT(tuple));
+   ReleaseSysCache(tuple);
+   PG_RETURN_BOOL(result);
+}
+
+
 /*
  * Insert new publication / relation mapping.
  */
diff --git a/src/backend/catalog/system_views.sql 
b/src/backend/catalog/system_views.sql
index 0fdad0c119..895b4001b7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -255,12 +255,23 @@ CREATE VIEW pg_stats WITH (security_barrier) AS
 
 CREATE VIEW pg_publication_tables AS
 SELECT
-P.pubname AS pubname,
-N.nspname AS schemaname,
-C.relname AS tablename
-FROM pg_publication P, pg_class C
- JOIN pg_namespace N ON (N.oid = C.relnamespace)
-WHERE C.oid IN (SELECT relid FROM 

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

[Ashutosh Bapat]

On Fri, Jun 16, 2017 at 12:48 AM, Robert Haas  wrote:
> On Thu, Jun 15, 2017 at 12:54 PM, Ashutosh Bapat
>  wrote:
>> Some more comments on the latest set of patches.
>>
>> In heap_drop_with_catalog(), we heap_open() the parent table to get the
>> default partition OID, if any. If the relcache doesn't have an entry for the
>> parent, this means that the entry will be created, only to be invalidated at
>> the end of the function. If there is no default partition, this all is
>> completely unnecessary. We should avoid heap_open() in this case. This also
>> means that get_default_partition_oid() should not rely on the relcache entry,
>> but should growl through pg_inherit to find the default partition.
>
> I am *entirely* unconvinced by this line of argument.  I think we want
> to open the relation the first time we touch it and pass the Relation
> around thereafter.

If this would be correct, why heap_drop_with_catalog() without this
patch just locks the parent and doesn't call a heap_open(). I am
missing something.

> Anything else is prone to accidentally failing to
> have the relation locked early enough,

We are locking the parent relation even without this patch, so this
isn't an issue.

> or looking up the OID in the
> relcache multiple times.

I am not able to understand this in the context of default partition.
After that nobody else is going to change its partitions and their
bounds (since both of those require heap_open on parent which would be
stuck on the lock we hold.). So, we have to check only once if the
table has a default partition. If it doesn't, it's not going to
acquire one unless we release the lock on the parent i.e at the end of
transaction. If it has one, it's not going to get dropped till the end
of the transaction for the same reason. I don't see where we are
looking up OIDs multiple times.


>
>> +defaultPartOid = get_default_partition_oid(rel);
>> +if (OidIsValid(defaultPartOid))
>> +ereport(ERROR,
>> +(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
>> + errmsg("there exists a default partition for table
>> \"%s\", cannot attach a new partition",
>> +RelationGetRelationName(rel;
>> +
>> Should be done before heap_open on the table being attached. If we are not
>> going to attach the partition, there's no point in instantiating its 
>> relcache.
>
> No, because we should take the lock before examining any properties of
> the table.

There are three tables involved here. "rel" which is the partitioned
table. "attachrel" is the table being attached as a partition to "rel"
and defaultrel, which is the default partition table. If there exists
a default partition in "rel" we are not allowing "attachrel" to be
attached to "rel". If that's the case, we don't need to examine any
properties of "attachrel" and hence we don't need to instantiate
relcache of "attachrel". That's what the comment is about.
ATExecAttachPartition() receives "rel" as an argument which has been
already locked and opened. So, we can check the existence of default
partition right at the beginning of the function.

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


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

Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[Tom Lane]

Michael Paquier  writes:
> On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
>> (2) My inclination would be not to back-patch.  This change could break
>> configurations that worked before, and the lack of prior complaints
>> says that not many people are having a problem with it.

> That's fourteen years without complains, still I cannot imagine any
> cases where it would be a problem as people who would have faced this
> problem but not reported it have likely just enforced the FS to handle
> case-insensitivity for paths.

Well, it's not just about case sensitivity.  The comment for
SplitDirectoriesString points out that it deals with embedded spaces
differently, and it also applies canonicalize_path().  I'm too tired
to think hard about what that part might mean for compatibility, but
it probably isn't nothing.

Anyway, I agree that this is an appropriate change for HEAD.  Just
not convinced that we should shove it into minor releases.

regards, tom lane


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

Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[QL Zhuo]

"That's fourteen years without complains", so maybe not to back-patch is
good choice, until someone complains this :)

And, the attached new patch fixes the memory leaks.

--
This email address (zhuo.devgmail.com) is only for development affairs,
e.g. mail list, please mail to zhuohexoasis.com or zhuoqlzoho.com
for other purpose.

ZHUO QL (KDr2), http://kdr2.com

On Fri, Jun 16, 2017 at 12:27 PM, Tom Lane  wrote:

> Michael Paquier  writes:
> > On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> >> (2) My inclination would be not to back-patch.  This change could break
> >> configurations that worked before, and the lack of prior complaints
> >> says that not many people are having a problem with it.
>
> > That's fourteen years without complains, still I cannot imagine any
> > cases where it would be a problem as people who would have faced this
> > problem but not reported it have likely just enforced the FS to handle
> > case-insensitivity for paths.
>
> Well, it's not just about case sensitivity.  The comment for
> SplitDirectoriesString points out that it deals with embedded spaces
> differently, and it also applies canonicalize_path().  I'm too tired
> to think hard about what that part might mean for compatibility, but
> it probably isn't nothing.
>
> Anyway, I agree that this is an appropriate change for HEAD.  Just
> not convinced that we should shove it into minor releases.
>
> regards, tom lane
>


0001-Don-t-downcase-filepath-in-load_libraries.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] logical replication read-only slave

[Craig Ringer]

On 15 June 2017 at 23:12, Maeldron T.  wrote:

> I could send an explicit command for each session to make it read-only
> I could use a read-only role (let’s ignore now I don’t use rules)

You can also set the GUC default_transaction_read_only = on.

But apps can easily clobber that with explicit read/write begin.
Setting it in combination with a role that doesn't have any write
permissions would be sufficient for most practical situations IMO.

> The DDL could be applied in a specific session as whitelisting is safer than
> blacklisting. I think the only missing part is if the subscription could
> turn on the writes for itself.
>
> If you think this would make sense, please consider it.

BDR has the option of marking a node as read-only, which is
implemented using an ExecutorStart_hook. It probably wouldn't be
overly hard to do the same thing as a standalone extension. You'd want
to detect when you were running within a logical replication apply
worker and permit changes then, but I don't expect that'd be unduly
hard.

It'd be nice to have a built-in way to do this, so maybe you could
pursue that for postgresql 11, raising a firm design idea here and
following up with a patch if you get a reasonable approximation of
consensus.

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


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

Re: [HACKERS] WIP: Data at rest encryption

[Tatsuo Ishii]

> Yeah, I guess we will just have to wait to see it since other people are
> excited about it.  My concern is code complexity and usability
> challenges, vs punting the problem to the operating system, though
> admittedly there are some cases where that is not possible.

Oracle sells this feature only with the expensive enterprise
edition. And people actually buy it. I guess the feature is pretty
important for some users.

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


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

Re: [HACKERS] Decimal64 and Decimal128

[Thomas Munro]

On Fri, Jun 16, 2017 at 1:24 PM, Craig Ringer  wrote:
> On 16 June 2017 at 05:42, Thomas Munro  wrote:
>> Bumping this ancient thread to say that DECFLOAT appears to have
>> landed in the SQL standard.  I haven't looked at SQL:2016 myself by I
>> just saw this on Markus Winand's Modern SQL blog:
>>
>> "There is a new type decfloat[()] (T076)."
>>
>> http://modern-sql.com/blog/2017-06/whats-new-in-sql-2016
>>
>> So far it's supported only by DB2 (inventor) and FirebirdSQL has just
>> announced support in the next release.
>
> I was pretty excited by decimal floating point initially, but the lack
> of support for its use in hardware in commonplace CPUs makes me less
> thrilled. IIRC Intel was talking about adding it, but I can't find any
> references to that anymore. POWER6 and POWER7 has it, which is great,
> but hardly justifies a push for getting it into the core Pg.
>
> Some of the discussion on
> https://software.intel.com/en-us/articles/intel-decimal-floating-point-math-library?page=1
> suggests that doing it fully in hardware is very expensive, so a mixed
> software/microcode implementation with some hardware assistance is
> likely if/when it comes.

There are considerations other than raw arithmetic performance though:

1.  They are fixed size, and DECFLOAT(9) [= 32 bit] and DECFLOAT(17)
[= 64 bit] could in theory be passed by value.  Of course we don't
have a way to make those pass-by-value and yet pass DECFLOAT(34) [=
128 bit] by reference!  That is where I got stuck last time I was
interested in this subject, because that seems like the place where we
would stand to gain a bunch of performance, and yet the limited
technical factors seems to be very well baked into Postgres.

2.  They may be increasingly used as 'vocabulary' datatypes as more
languages and databases adopt them.  That's probably not a huge
semantic problem since DECIMAL can represent most useful DECFLOAT
values exactly (but not some IEEE 754 quirks like -0, -inf, +inf, NaN,
and I haven't checked what SQL:2016 says about that anyway but if it's
based directly on IEEE 754:2008 then I guess they'll be in there).

I don't understand these things but it looks like the support in C
(originally proposed as N1312 and implemented by IBM, HP, GCC and
Intel compilers) has reached the next stage and been published as
ISO/IEC TS 18661-2:2015, and now N2079 proposes that TS 18661-2 be
absorbed into C2x (!).  As glacial as ISO processes may be, it's
encouraging that there is now a TS even though I'm not allowed to
download it without paying CHF178.  Meanwhile Python and others just
did it (albeit vastly less efficiently).

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


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

Re: [HACKERS] PATCH: Don't downcase filepath/filename while loading libraries

[Michael Paquier]

On Fri, Jun 16, 2017 at 1:01 PM, Tom Lane  wrote:
> (2) My inclination would be not to back-patch.  This change could break
> configurations that worked before, and the lack of prior complaints
> says that not many people are having a problem with it.

That's fourteen years without complains, still I cannot imagine any
cases where it would be a problem as people who would have faced this
problem but not reported it have likely just enforced the FS to handle
case-insensitivity for paths. Or they just relied on the default: on
Windows the default is to be case-insensitive, as much as OSX. So the
proposed patch handles things better with case-sensitive paths,
without really impacting users with case-insensive FS.

I see the point of not back-patching the change though.
-- 
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] logical replication: \dRp+ and "for all tables"

[Peter Eisentraut]

On 6/10/17 02:02, Jeff Janes wrote:
> On Fri, Jun 9, 2017 at 10:20 PM, Masahiko Sawada  > wrote:
> 
> On Sat, Jun 10, 2017 at 7:29 AM, Jeff Janes  > wrote:
> > If I create a publication "for all tables", \dRp+ doesn't indicate it 
> is for
> > all tables, it just gives a list of the tables.
> >
> > So it doesn't distinguish between a publication specified to be for all
> > tables (which will be dynamic regarding future additions), and one which
> > just happens to include all the table which currently exist.
> >
> > That seems unfortunate.  Should the "for all tables" be included as 
> another
> > column in \dRp and \dRp+, or at least as a footnote tag in \dRp+ ?
> >
> 
> +1. I was thinking the same. Attached patch adds "All Tables" column
> to both \dRp and \dRp+.
> 
> 
> Looks good to me.  Attached with regression test expected output  changes.

I have committed your patch and removed the "Tables" footer for
all-tables publications, as was discussed later in the thread.

-- 
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] memory fields from getrusage()

[Justin Pryzby]

On Thu, Jun 15, 2017 at 10:29:21AM -0400, Robert Haas wrote:
> On Wed, Jun 14, 2017 at 6:28 PM, Justin Pryzby  wrote:
> > On Tue, Jun 13, 2017 at 12:16:00PM -0400, Robert Haas wrote:
> >> It might be worth adding platform-specific code for common platforms.
> >
> > All I care (which linux happily/happens to support) is maxrss; I was 
> > probably
> > originally interested in this while digging into an issue with hash agg.
> 
> I don't think it needs to go in a separate file.  I'd just patch ShowUsage().

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f99dd0a..7f57a84 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4467,6 +4467,21 @@ ShowUsage(const char *title)
 r.ru_nvcsw - Save_r.ru_nvcsw,
 r.ru_nivcsw - Save_r.ru_nivcsw,
 r.ru_nvcsw, r.ru_nivcsw);
+
+#if defined(__linux__)
+   appendStringInfo(,
+"!\t%ld max resident (kB)\n",
+r.ru_maxrss);
+#elif defined(BSD)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss, r.ru_ixrss, r.ru_idrss, 
r.ru_isrss);
+#elif defined(__darwin__)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss/1024, r.ru_ixrss/1024, 
r.ru_idrss/1024, r.ru_isrss/1024);
+#endif /* __linux__ */
+
 #endif   /* HAVE_GETRUSAGE */
 
/* remove trailing newline */

Comments ?

Testing or suggestions on !linux would be useful.

Justin
Add memory fields (sometimes) available from getrusage()

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f99dd0a..7f57a84 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4467,6 +4467,21 @@ ShowUsage(const char *title)
 r.ru_nvcsw - Save_r.ru_nvcsw,
 r.ru_nivcsw - Save_r.ru_nivcsw,
 r.ru_nvcsw, r.ru_nivcsw);
+
+#if defined(__linux__)
+   appendStringInfo(,
+"!\t%ld max resident (kB)\n",
+r.ru_maxrss);
+#elif defined(BSD)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss, r.ru_ixrss, r.ru_idrss, 
r.ru_isrss);
+#elif defined(__darwin__)
+   appendStringInfo(,
+"!\t%ld max resident, %ld shared, %ld unshared data, 
%ld unshared stack (kB)\n",
+r.ru_maxrss/1024, r.ru_ixrss/1024, 
r.ru_idrss/1024, r.ru_isrss/1024);
+#endif /* __linux__ */
+
 #endif   /* HAVE_GETRUSAGE */
 
/* remove trailing newline */

-- 
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] Getting server crash on Windows when using ICU collation

[Peter Eisentraut]

On 6/12/17 00:38, Ashutosh Sharma wrote:
> PFA patch that fixes the issue described in above thread. As mentioned
> in the above thread, the crash is basically happening in varstr_cmp()
> function and  it's  only happening on Windows because in varstr_cmp(),
> if the collation provider is ICU, we don't even think of calling ICU
> functions to compare the string. Infact, we directly attempt to call
> the OS function wsccoll*() which is not expected. Thanks.

Maybe just

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a0dd391f09..2506f4eeb8 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1433,7 +1433,7 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, 
Oid collid)
 
 #ifdef WIN32
/* Win32 does not have UTF-8, so we need to map to UTF-16 */
-   if (GetDatabaseEncoding() == PG_UTF8)
+   if (GetDatabaseEncoding() == PG_UTF8 && (!mylocale || 
mylocale->provider == COLLPROVIDER_LIBC))
{
int a1len;
int a2len;


-- 
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] logical replication read-only slave

[Peter Eisentraut]

On 6/15/17 11:12, Maeldron T. wrote:
> However, it provides me a safety net that I could not execute writes on
> the slave by accident. Not only I couldn’t do it, I would also receive a
> notification from the software about the attempt as it would throw an
> exception.
> 
> Let’s say I would switch to logical replication of all tables
> Safety net is gone

You can change the permissions on your slave so that you cannot write to
the tables.

-- 
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] Refreshing subscription relation state inside a transaction block

[Petr Jelinek]

On 15/06/17 15:52, Peter Eisentraut wrote:
> On 6/15/17 02:41, Petr Jelinek wrote:
>> Hmm, forcibly stopping currently running table sync is not what was
>> intended, I'll have to look into it. We should not be forcibly stopping
>> anything except the main apply worker during drop subscription (and we
>> do that only because we can't drop the remote replication slot otherwise).
> 
> The change being complained about was specifically to address the
> problem described in the commit message:
> 
> Stop table sync workers when subscription relation entry is removed
> 
> When a table sync worker is in waiting state and the subscription table
> entry is removed because of a concurrent subscription refresh, the
> worker could be left orphaned.  To avoid that, explicitly stop the
> worker when the pg_subscription_rel entry is removed.
> 
> 
> Maybe that wasn't the best solution.  Alternatively, the tablesync
> worker has to check itself whether the subscription relation entry has
> disappeared, or we need a post-commit check to remove orphaned workers.
> 

Ah I missed that commit/discussion, otherwise I would have probably
complained. I don't like killing workers in the DDL command, as I said
the only reason we do it in DropSubscription is to free the slot for
dropping.

I think that either of the options you suggested now would be better.
We'll need that for stopping the tablesync which is in progress during
DropSubscription as well as those will currently still keep running. I
guess we could simply just register syscache callback, the only problem
with that is we'd need to AcceptInvalidationMessages regularly while we
do the COPY which is not exactly free so maybe killing at the end of
transaction would be better (both for refresh and drop)?

-- 
  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] intermittent failures in Cygwin from select_parallel tests

[Tom Lane]

Robert Haas  writes:
> On Thu, Jun 15, 2017 at 10:38 AM, Tom Lane  wrote:
>> ... er, -ENOCAFFEINE.  Nonetheless, there are no checks of
>> EXEC_FLAG_EXPLAIN_ONLY in any parallel-query code, so I think
>> a bet is being missed somewhere.

> ExecGather() is where workers get launched, and that ain't happening
> if EXEC_FLAG_EXPLAIN_ONLY is set, unless I am *very* confused about
> how this works.

Ah, you're right, it's done during first execution not during InitPlan.
Nevermind ...

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] Bug in ExecModifyTable function and trigger issues for foreign tables

[Ildus Kurbangaliev]

On Mon, 5 Jun 2017 17:25:27 +0900
Etsuro Fujita  wrote:

> On 2017/06/02 18:10, Etsuro Fujita wrote:
> > On 2017/05/16 21:36, Etsuro Fujita wrote:  
> >> One approach I came up with to fix this issue is to rewrite the 
> >> targetList entries of an inherited UPDATE/DELETE properly using 
> >> rewriteTargetListUD, when generating a plan for each child table
> >> in inheritance_planner.  Attached is a WIP patch for that.  Maybe
> >> I am missing something, though.  
> > 
> > While updating the patch, I noticed the patch rewrites the UPDATE 
> > targetList incorrectly in some cases; rewrite_inherited_tlist I
> > added to adjust_appendrel_attrs (1) removes all junk items from the
> > targetList and (2) adds junk items for the child table using
> > rewriteTargetListUD, but it's wrong to drop all junk items in cases
> > where there are junk items for some other reasons than
> > rewriteTargetListUD.  Consider junk items containing MULTIEXPR
> > SubLink.  One way I came up with to fix this is to change (1) to
> > only remove junk items with resname; since junk items added by
> > rewriteTargetListUD should have resname (note: we would need
> > resname to call ExecFindJunkAttributeInTlist at execution time!)
> > while other junk items wouldn't have resname (see
> > transformUpdateTargetList), we could correctly replace junk items
> > added by rewriteTargetListUD for the parent with ones for the
> > child, by that change.  I might be missing something, though.
> > Comments welcome.  
> 
> I updated the patch that way.  Please find attached an updated
> version.
> 
> Other changes:
> * Moved the initialization for "tupleid" I added in ExecModifyTable
> as discussed before, which I think is essentially the same as
> proposed by Ildus in [1], since I think that would be more consistent
> with "oldtuple".
> * Added regression tests.
> 
> Anyway I'll add this to the next commitfest.
> 
> Best regards,
> Etsuro Fujita
> 
> [1] 
> https://www.postgresql.org/message-id/20170514150525.0346ba72%40postgrespro.ru

Checked the latest patch. Looks good.
Shouldn't this patch be backported to 9.6 and 10beta? The bug
affects them too.

-- 
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
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] memory fields from getrusage()

[Tom Lane]

Justin Pryzby  writes:
> Comments ?

I was wondering whether it'd be better to drive this off of configure
probes for the existence of the struct fields.  However, in view of
the same fields having different contents on some platforms, such
a probe wouldn't really help much --- you'd still need platform-aware
logic.

Please add to next commitfest so we remember about this when the time
is ripe.

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] intermittent failures in Cygwin from select_parallel tests

[Robert Haas]

On Thu, Jun 15, 2017 at 10:21 AM, Amit Kapila  wrote:
> Yes, I think it is for next query.  If you refer the log below from lorikeet:
>
> 2017-06-13 16:44:57.179 EDT [59404ec6.2758:63] LOG:  statement:
> EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM
> tenk1;
> 2017-06-13 16:44:57.247 EDT [59404ec9.2e78:1] ERROR:  could not map
> dynamic shared memory segment
> 2017-06-13 16:44:57.248 EDT [59404dec.2d9c:5] LOG:  worker process:
> parallel worker for PID 10072 (PID 11896) exited with exit code 1
> 2017-06-13 16:44:57.273 EDT [59404ec6.2758:64] LOG:  statement: select
> stringu1::int2 from tenk1 where unique1 = 1;
> TRAP: FailedAssertion("!(BackgroundWorkerData->parallel_register_count
> - BackgroundWorkerData->parallel_terminate_count <= 1024)", File:
> "/home/andrew/bf64/root/HEAD/pgsql.build/../pgsql/src/backend/postmaster/bgworker.c",
> Line: 974)
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:6] LOG:  server process
> (PID 10072) was terminated by signal 6: Aborted
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:7] DETAIL:  Failed process
> was running: select stringu1::int2 from tenk1 where unique1 = 1;
> 2017-06-13 16:45:02.652 EDT [59404dec.2d9c:8] LOG:  terminating any
> other active server processes
>
> Error "could not map dynamic shared memory segment" is due to query
> "EXPLAIN .. SELECT * FROM tenk1" and Assertion failure is due to
> another statement "select stringu1::int2 from tenk1 where unique1 =
> 1;".

I think you're right.  So here's a theory:

1. The ERROR mapping the DSM segment is just a case of the worker the
losing a race, and isn't a bug.

2. But when that happens, parallel_terminate_count is getting bumped
twice for some reason.

3. So then the leader process fails that assertion when it tries to
launch the parallel workers for the next query.

-- 
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] logical replication read-only slave

[Maeldron T.]

Hello,

I played around a bit with the logical replication in 10.0 beta 1.

My first question was: is it possible to set the "slave" server to run in
(almost) read-only mode?

The current setup is the following:

There is a Rails application running on multiple servers
Two PostgreSQL servers, stream replication
Writes are executed on the master
Some of the reads are executed on the slave
(Nothing new here)

However, it provides me a safety net that I could not execute writes on the
slave by accident. Not only I couldn’t do it, I would also receive a
notification from the software about the attempt as it would throw an
exception.


Let’s say I would switch to logical replication of all tables
Safety net is gone

I could send an explicit command for each session to make it read-only
I could use a read-only role (let’s ignore now I don’t use rules)

But the main attribute of a safety net is the safety. As soon as there
would be a bug, and a session would not send the "set session ..." command,
or the wrong role would be used, the application could write to the
"slave", and that’s not great.

As far as I see, the only solution which provides the same safety level as
the stream replication does would be starting up the "slave" in read-only
mode. In this case, writes would be needed for:

* The replication
* DDL

The DDL could be applied in a specific session as whitelisting is safer
than blacklisting. I think the only missing part is if the subscription
could turn on the writes for itself.

If you think this would make sense, please consider it.

M

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

[Peter Eisentraut]

On 6/15/17 12:22, Petr Jelinek wrote:
> On 15/06/17 17:53, Peter Eisentraut wrote:
>> On 6/14/17 18:35, Petr Jelinek wrote:
>>> Attached fixes it (it was mostly about order of calls).
>>
>> So do I understand this right that the actual fix is just moving up the
>> logicalrep_worker_stop() call in DropSubscription().
>>
> 
> No the fix is heap_open before SearchSysCache().

Right.  Is there a reason for moving the _stop() call then?

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

[Ashutosh Bapat]

Some more comments on the latest set of patches.

In heap_drop_with_catalog(), we heap_open() the parent table to get the
default partition OID, if any. If the relcache doesn't have an entry for the
parent, this means that the entry will be created, only to be invalidated at
the end of the function. If there is no default partition, this all is
completely unnecessary. We should avoid heap_open() in this case. This also
means that get_default_partition_oid() should not rely on the relcache entry,
but should growl through pg_inherit to find the default partition.

In get_qual_for_list(), if the table has only default partition, it won't have
any boundinfo. In such a case the default partition's constraint would come out
as (NOT ((a IS NOT NULL) AND (a = ANY (ARRAY[]::integer[]. The empty array
looks odd and may be we spend a few CPU cycles executing ANY on an empty array.
We have the same problem with a partition containing only NULL value. So, may
be this one is not that bad.

Please add a testcase to test addition of default partition as the first
partition.

get_qual_for_list() allocates the constant expressions corresponding to the
datums in CacheMemoryContext while constructing constraints for a default
partition. We do not do this for other partitions. We may not be constructing
the constraints for saving in the cache. For example, ATExecAttachPartition
constructs the constraints for validation. In such a case, this code will
unnecessarily clobber the cache memory. generate_partition_qual() copies the
partition constraint in the CacheMemoryContext.

+   if (spec->is_default)
+   {
+   result = list_make1(make_ands_explicit(result));
+   result = list_make1(makeBoolExpr(NOT_EXPR, result, -1));
+   }

If the "result" is an OR expression, calling make_ands_explicit() on it would
create AND(OR(...)) expression, with an unnecessary AND. We want to avoid that?

+   if (cur_index < 0 && (partition_bound_has_default(partdesc->boundinfo)))
+   cur_index = partdesc->boundinfo->default_index;
+
The partition_bound_has_default() check is unnecessary since we check for
cur_index < 0 next anyway.

+ *
+ * Given the parent relation checks if it has default partition, and if it
+ * does exist returns its oid, otherwise returns InvalidOid.
+ */
May be reworded as "If the given relation has a default partition, this
function returns the OID of the default partition. Otherwise it returns
InvalidOid."

+Oid
+get_default_partition_oid(Relation parent)
+{
+   PartitionDesc partdesc = RelationGetPartitionDesc(parent);
+
+   if (partdesc->boundinfo && partition_bound_has_default(partdesc->boundinfo))
+   return partdesc->oids[partdesc->boundinfo->default_index];
+
+   return InvalidOid;
+}
An unpartitioned table would not have partdesc set set. So, this function will
segfault if we pass an unpartitioned table. Either Assert that partdesc should
exist or check for its NULL-ness.


+defaultPartOid = get_default_partition_oid(rel);
+if (OidIsValid(defaultPartOid))
+ereport(ERROR,
+(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("there exists a default partition for table
\"%s\", cannot attach a new partition",
+RelationGetRelationName(rel;
+
Should be done before heap_open on the table being attached. If we are not
going to attach the partition, there's no point in instantiating its relcache.

The comment in heap_drop_with_catalog() should mention why we lock the default
partition before locking the table being dropped.

 extern List *preprune_pg_partitions(PlannerInfo *root, RangeTblEntry *rte,
Index rti, Node *quals, LOCKMODE lockmode);
-
 #endif   /* PARTITION_H */
Unnecessary hunk.

On Thu, Jun 15, 2017 at 12:31 PM, Amit Langote
 wrote:
> Oops, I meant to send one more comment.
>
> On 2017/06/15 15:48, Amit Langote wrote:
>> BTW, I noticed the following in 0002
> +errmsg("there exists a default 
> partition for table \"%s\", cannot
> add a new partition",
>
> This error message style seems novel to me.  I'm not sure about the best
> message text here, but maybe: "cannot add new partition to table \"%s\"
> with default partition"
>
> Note that the comment applies to both DefineRelation and
> ATExecAttachPartition.
>
> Thanks,
> Amit
>



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


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