Re: Postgres 11 release notes

[Amit Kapila]

On Tue, May 15, 2018 at 11:43 AM, David Rowley
 wrote:
> On 12 May 2018 at 03:08, Bruce Momjian  wrote:
>> I have committed the first draft of the Postgres 11 release notes.  I
>> will add more markup soon.  You can view the most current version here:
>>
>> http://momjian.us/pgsql_docs/release-11.html
>>
>> I expect a torrent of feedback.  ;-)
>
> I wonder if 3cda10f41bfed -- "Use atomic ops to hand out pages to scan
> in parallel scan." should be listed in the notes.
>
> If so, I'd suggest something like:
>
> Improve performance of Parallel Seq Scans with many parallel workers
> (David Rowley).
>

+1 to add this in Release Notes.


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

log_min_messages shows debug instead of debug2

[Ideriha, Takeshi]

Hi,

I noticed that if log_min_messages is set to ‘debug2’, it shows ‘debug’ instead 
of debug2.
Showing other debug options like debug1 work normally.
This is same for client_min_messages.

According to a033daf56 and baaad2330, debug is an alias for debug2 and for 
backward-compatibility.
And also these debug options are mainly used for PostgreSQL hackers, so I think 
this is a trivial issue.

Patch attached.
The output of ‘show log_min_messages’ becomes ‘debug2’ if it’s set to either 
debug or debug2.
It passed make installcheck.

I just changed the order of server_message_level_options[]
because in this case SHOW command linearly searches GUC option by 
config_enum_lookup_by_value().

What do you think about it?
If it’s worth committing, do I need test? I couldn’t find the relevant test set.


Takeshi Ideriha
Fujitsu Limited



debug2_log_level.patch
Description: debug2_log_level.patch

Re: libpq compression

[Dmitry Dolgov]

> On 30 March 2018 at 14:53, Konstantin Knizhnik 
wrote:
> Hi hackers,
> One of our customers was managed to improve speed about 10 times by using
SSL compression for the system where client and servers are located in
different geographical regions
> and query results are very large because of JSON columns. Them actually
do not need encryption, just compression.
> I expect that it is not the only case where compression of libpq protocol
can be useful. Please notice that Postgres replication is also using libpq
protocol.
>
> Taken in account that vulnerability was found in SSL compression and so
SSLComppression is considered to be deprecated and insecure (
http://www.postgresql-archive.org/disable-SSL-compression-td6010072.html),
it will be nice to have some alternative mechanism of reducing  libpq
traffic.
>
> I have implemented some prototype implementation of it (patch is
attached).
> To use zstd compression, Postgres should be configured with --with-zstd.
Otherwise compression will use zlib unless it is disabled by --without-zlib
option.
> I have added compression=on/off parameter to connection string and -Z
option to psql and pgbench utilities.

I'm a bit confused why there was no reply to this. I mean, it wasn't sent on
1st April, the patch still can be applied on top of the master branch and
looks
like it even works.

I assume the main concern her is that it's implemented in a rather not
extensible way. Also, if I understand correctly, it compresses the data
stream
in both direction server <-> client, not sure if there is any value in
compressing what a client sends to a server. But still I'm wondering why it
didn't start at least a discussion about how it can be implemented. Do I
miss
something?

Re: [HACKERS] Planning counters in pg_stat_statements

[Dmitry Ivanov]

Hi,

Is anybody still working on this? Are there any plans to add this to 
commitfest?


I'd like to add planning time to auto_explain, and it turns out that 
this patch is somewhat relevant to that feature.


The current approach here is to set planning_time in PlannedStmt via 
planner_hook, which (in my opinion) has several flaws:


1. Depending on the order of extensions in shared_preload_libraries, it 
might not measure time spent on preceding planner hooks.


2. Provided that there are multiple users of this metric, it might 
become a little too costy to register several hooks with identical 
purpose.


3. [Bikeshedding] Although planning time is stored in PlannedStmt, it's 
definitely not an inherent property of a plan. You could have two 
machines with identical settings but quite different planning times due 
to various circumstances (raw CPU power, I/O etc).


I'd argue that it might be better to add a new argument to 
pg_plan_query() and pg_plan_queries() and a new field to QueryDesc, 
i.e.:


PlannedStmt *
pg_plan_query(Query *querytree,
  int cursorOptions,
  ParamListInfo boundParams,
  double *planningTime)

List *
pg_plan_queries(List *querytrees,
int cursorOptions,
ParamListInfo boundParams,
double *planningTime) /* total time as in 
BuildCachedPlan() */

The measured time can later be passed to QueryDesc via 
PortalDefineQuery(). Of course, this requires more changes, but the 
result might be worth it.


What do you think?

--
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] asynchronous execution

[Kyotaro HORIGUCHI]

This gets further refactoring.

At Fri, 11 May 2018 17:45:20 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20180511.174520.188681124.horiguchi.kyot...@lab.ntt.co.jp>
> But, this is not just a rebased version. On the way fixing
> serious conflicts, I refactored patch and I believe this becomes
> way readable than the previous shape.
> 
> - Waiting queue manipulation is moved into new functions. It had
>   a bug that the same node can be inserted in the queue more than
>   once and it is fixed.
> 
> - postgresIterateForeginScan had somewhat a tricky strcuture to
>   merge similar procedures thus it cannot be said easy-to-read at
>   all. Now it is far simpler and straight-forward looking.
> 
> - Still this works only on Append/ForeignScan.

I performed almost the same test (again) as before but with some
new things:

- partition tables (There should be no difference with
  inheritance and it actually looks so.)

- added test for fetch_size of 200 and 1000 as long as 100.

  Fetch size of 100 seems unreasonably magnifies the lag by
  context switching on single poor box for the test D/F
  below. They became faster by about twice by *adding* a small
  delay (1000 times of clock_gettime()(*1)) just before
  epoll_wait. Things would be different on separate machines but
  I'm not sure it really is. I don't find the exact cause nor how
  to avoid it.

*1: The reason for the function is that I found at first that the
queries get way faster by just prefixing by "explain
analyze"..

Async append (theoretically) no longer affects non-async path at
all so B is expected to get no degradation. It seems within
error.

C and F are the gain when all foreign tables share one connection
and D and G are the gain when every foreign tables has dedicate
connection.

(previous numbers)
>patched(ms)  unpatched(ms)   gain(%)
> A: simple table scan :  3562.32  3444.81-3.4
> B: local partitioning:  1451.25  1604.38 9.5
> C: single remote table   :  8818.92  9297.76 5.1
> D: sharding (single con) :  5966.14  6646.7310.2
> E: sharding (multi con)  :  1802.25  6515.4972.3

fetch_size = 100
patched(ms)  unpatched(ms)   gain(%)
A: simple table scan :   3065.82   3046.82-0.62
B: local partitioning:   1393.98   1378.00-1.16
C: single remote table   :   8499.73   8595.66 1.12
D: sharding (single con) :   9267.85   9251.59-0.18
E: sharding (multi con)  :   2567.02   9295.2272.38
F: partition (single con):   9241.08   9060.19-2.00
G: partition (multi con) :   2548.86   9419.1872.94

fetch_size = 200
patched(ms)  unpatched(ms)   gain(%)
A: simple table scan :   3067.08   2999.23-2.3 
B: local partitioning:   1392.07   1384.49-0.5 
C: single remote table   :   8521.72   8505.48-0.2 
D: sharding (single con) :   6752.81   7076.02 4.6  
E: sharding (multi con)  :   1958.27188.0272.8 
F: partition (single con):   6756.72   7000.72 3.5  
G: partition (multi con) :   1969.87228.8572.8 

fetch_size = 1000
patched(ms)  unpatched(ms)   gain(%)
A: simple table scan :   4547.44   4519.34-0.62
B: local partitioning:   2880.66   2739.43-5.16
C: single remote table   :   8448.04   8572.15 1.45
D: sharding (single con) :   2405.01   5919.3159.37
E: sharding (multi con)  :   1872.15   5963.0468.60
F: partition (single con):   2369.08   5960.8160.26
G: partition (multi con) :   1854.69   5893.6568.53


regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 54f85c159f3feee5ee2dac6daacc7330ec101ed5 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Mon, 22 May 2017 12:42:58 +0900
Subject: [PATCH 1/3] Allow wait event set to be registered to resource owner

WaitEventSet needs to be released using resource owner for a certain
case. This change adds WaitEventSet reowner and allow the creator of a
WaitEventSet to specify a resource owner.
---
 src/backend/libpq/pqcomm.c|  2 +-
 src/backend/storage/ipc/latch.c   | 18 ++-
 src/backend/storage/lmgr/condition_variable.c |  2 +-
 src/backend/utils/resowner/resowner.c | 67 +++
 src/include/storage/latch.h   |  4 +-
 src/include/utils/resowner_private.h  |  8 
 6 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index a4f6d4deeb..890972b9b8 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -220,7 +220,7 @@ pq_init(void)
 (errmsg("could not set socket to nonblocking mode: %m")));
 #endif
 
-	FeBeWaitSet = CreateWaitEventSet(TopMemoryConte

Re: libpq compression

[Konstantin Knizhnik]

On 15.05.2018 13:23, Dmitry Dolgov wrote:
> On 30 March 2018 at 14:53, Konstantin Knizhnik 
mailto:k.knizh...@postgrespro.ru>> wrote:

> Hi hackers,
> One of our customers was managed to improve speed about 10 times by 
using SSL compression for the system where client and servers are 
located in different geographical regions
> and query results are very large because of JSON columns. Them 
actually do not need encryption, just compression.
> I expect that it is not the only case where compression of libpq 
protocol can be useful. Please notice that Postgres replication is 
also using libpq protocol.

>
> Taken in account that vulnerability was found in SSL compression and 
so SSLComppression is considered to be deprecated and insecure 
(http://www.postgresql-archive.org/disable-SSL-compression-td6010072.html), 
it will be nice to have some alternative mechanism of reducing  libpq 
traffic.

>
> I have implemented some prototype implementation of it (patch is 
attached).
> To use zstd compression, Postgres should be configured with 
--with-zstd. Otherwise compression will use zlib unless it is disabled 
by --without-zlib option.
> I have added compression=on/off parameter to connection string and 
-Z option to psql and pgbench utilities.


I'm a bit confused why there was no reply to this. I mean, it wasn't 
sent on
1st April, the patch still can be applied on top of the master branch 
and looks

like it even works.

I assume the main concern her is that it's implemented in a rather not
extensible way. Also, if I understand correctly, it compresses the 
data stream

in both direction server <-> client, not sure if there is any value in
compressing what a client sends to a server. But still I'm wondering 
why it
didn't start at least a discussion about how it can be implemented. Do 
I miss

something?


Implementation of libpq compression will be included in next release of 
PgProEE.
Looks like community is not so interested in this patch. Frankly 
speaking I do not understand why.

Compression of libpq traffic can significantly increase speed of:
1. COPY
2. Replication (both streaming and logical)
3. Queries returning large results sets (for example JSON) through slow 
connections.


It is possible to compress libpq traffic using SSL compression.  But SSL 
compression is unsafe and deteriorated feature.


Yes, this patch is not extensible: it can use either zlib either zstd. 
Unfortunately internal Postgres compression pglz doesn't provide 
streaming API.
May be it is good idea to combine it with Ildus patch (custom 
compression methods): https://commitfest.postgresql.org/18/1294/
In this case it will be possible to use any custom compression 
algorithm. But we need to design and implement streaming API for pglz 
and other compressors.



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

Re: Global snapshots

[Robert Haas]

On Mon, May 14, 2018 at 7:20 AM, Stas Kelvich  wrote:
> Summarising, I think, that introducing some permanent connections to
> postgres_fdw node will put too much burden on this patch set and that it will
> be possible to address that later (in a long run such connection will be 
> anyway
> needed at least for a deadlock detection). However, if you think that current
> behavior + STO analog isn't good enough, then I'm ready to pursue that track.

I don't think I'd be willing to commit to a particular approach at
this point.  I think the STO analog is an interesting idea and worth
more investigation, and I think the idea of a permanent connection
with chatter that can be used to resolve deadlocks, coordinate shared
state, etc. is also an interesting idea.  But there are probably lots
of ideas that somebody could come up with in this area that would
sound interesting but ultimately not work out.  Also, an awful lot
depends on quality of implementation.  If you come up with an
implementation of a permanent connection for coordination "chatter",
and the patch gets rejected, it's almost certainly not a sign that we
don't want that thing in general.  It means we don't want yours.  :-)

Actually, I think if we're going to pursue that approach, we ought to
back off a bit from thinking about global snapshots and think about
what kind of general mechanism we want.  For example, maybe you can
imagine it like a message bus, where there are a bunch of named
channels on which the server publishes messages and you can listen to
the ones you care about.  There could, for example, be a channel that
publishes the new system-wide globalxmin every time it changes, and
another channel that publishes the wait graph every time the deadlock
detector runs, and so on.  In fact, perhaps we should consider
implementing it using the existing LISTEN/NOTIFY framework: have a
bunch of channels that are predefined by PostgreSQL itself, and set
things up so that the server automatically begins publishing to those
channels as soon as anybody starts listening to them.  I have to
imagine that if we had a good mechanism for this, we'd get all sorts
of proposals for things to publish.  As long as they don't impose
overhead when nobody's listening, we should be able to be fairly
accommodating of such requests.

Or maybe that model is too limiting, either because we don't want to
broadcast to everyone but rather send specific messages to specific
connections, or else because we need a request-and-response mechanism
rather than what is in some sense a one-way communication channel.
Regardless, we should start by coming up with the right model for the
protocol first, bearing in mind how it's going to be used and other
things for which somebody might want to use it (deadlock detection,
failover, leader election), and then implement whatever we need for
global snapshots on top of it.  I don't think that writing the code
here is going to be hugely difficult, but coming up with a good design
is going to require some thought and discussion.

And, for that matter, I think the same thing is true for global
snapshots.  The coding is a lot harder for that than it is for some
new subprotocol, I'd imagine, but it's still easier than coming up
with a good design.  As far as I can see, and everybody can decide for
themselves how far they think that is, the proposal you're making now
sounds like a significant improvement over the XTM proposal.  In
particular, the provisioning and deprovisioning issues sound like they
have been thought through a lot more.  I'm happy to call that
progress.  At the same time, progress on a journey is not synonymous
with arrival at the destination, and I guess it seems to me that you
have some further research to do along the lines you've described:

1. Can we hold back xmin only when necessary and to the extent
necessary instead of all the time?
2. Can we use something like an STO analog, maybe as an optional
feature, rather than actually holding back xmin?

And I'd add:

3. Is there another approach altogether that doesn't rely on holding
back xmin at all?

For example, if you constructed the happens-after graph between
transactions in shared memory, including actions on all nodes, and
looked for cycles, you could abort transactions that would complete a
cycle.  (We say A happens-after B if A reads or writes data previously
written by B.)  If no cycle exists then all is well.  I'm pretty sure
it's been well-established that a naive implementation of this
algorithm is terribly unperformant, but for example SSI works on this
principle.  It reduces the bookkeeping involved by being willing to
abort transactions that aren't really creating a cycle if they look
like they *might* create a cycle.  Now that's an implementation *on
top of* snapshots for the purpose of getting true serializability
rather than a way of getting global snapshots per se, so it's not
suitable for what you're trying do here, but I think it shows that
algorithms b

Re: libpq compression

[Andrew Dunstan]

On 05/15/2018 08:53 AM, Konstantin Knizhnik wrote:



On 15.05.2018 13:23, Dmitry Dolgov wrote:
> On 30 March 2018 at 14:53, Konstantin Knizhnik 
mailto:k.knizh...@postgrespro.ru>> wrote:

> Hi hackers,
> One of our customers was managed to improve speed about 10 times by 
using SSL compression for the system where client and servers are 
located in different geographical regions
> and query results are very large because of JSON columns. Them 
actually do not need encryption, just compression.
> I expect that it is not the only case where compression of libpq 
protocol can be useful. Please notice that Postgres replication is 
also using libpq protocol.

>
> Taken in account that vulnerability was found in SSL compression 
and so SSLComppression is considered to be deprecated and insecure 
(http://www.postgresql-archive.org/disable-SSL-compression-td6010072.html), 
it will be nice to have some alternative mechanism of reducing  libpq 
traffic.

>
> I have implemented some prototype implementation of it (patch is 
attached).
> To use zstd compression, Postgres should be configured with 
--with-zstd. Otherwise compression will use zlib unless it is 
disabled by --without-zlib option.
> I have added compression=on/off parameter to connection string and 
-Z option to psql and pgbench utilities.


I'm a bit confused why there was no reply to this. I mean, it wasn't 
sent on
1st April, the patch still can be applied on top of the master branch 
and looks

like it even works.

I assume the main concern her is that it's implemented in a rather not
extensible way. Also, if I understand correctly, it compresses the 
data stream

in both direction server <-> client, not sure if there is any value in
compressing what a client sends to a server. But still I'm wondering 
why it
didn't start at least a discussion about how it can be implemented. 
Do I miss

something?


Implementation of libpq compression will be included in next release 
of PgProEE.
Looks like community is not so interested in this patch. Frankly 
speaking I do not understand why.

Compression of libpq traffic can significantly increase speed of:
1. COPY
2. Replication (both streaming and logical)
3. Queries returning large results sets (for example JSON) through 
slow connections.


It is possible to compress libpq traffic using SSL compression. But 
SSL compression is unsafe and deteriorated feature.


Yes, this patch is not extensible: it can use either zlib either zstd. 
Unfortunately internal Postgres compression pglz doesn't provide 
streaming API.
May be it is good idea to combine it with Ildus patch (custom 
compression methods): https://commitfest.postgresql.org/18/1294/
In this case it will be possible to use any custom compression 
algorithm. But we need to design and implement streaming API for pglz 
and other compressors.






I'm sure there is plenty of interest in this. However, you guys need to 
understand where we are in the development cycle. We're trying to wrap 
up Postgres 11, which was feature frozen before this patch ever landed. 
So it's material for Postgres 12. That means it will probably need to 
wait a little while before it gets attention. It doesn't mean nobody is 
interested.


I disagree with Dmitry about compressing in both directions - I can 
think of plenty of good cases where we would want to compress traffic 
from the client.


cheers

andrew


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

Re: libpq compression

[Craig Ringer]

On 15 May 2018 at 21:36, Andrew Dunstan 
wrote:

>
>
>>> > To use zstd compression, Postgres should be configured with
>>> --with-zstd. Otherwise compression will use zlib unless it is disabled by
>>> --without-zlib option.
>>> > I have added compression=on/off parameter to connection string and -Z
>>> option to psql and pgbench utilities.
>>>
>>> I'm a bit confused why there was no reply to this. I mean, it wasn't
>>> sent on
>>> 1st April, the patch still can be applied on top of the master branch
>>> and looks
>>> like it even works.
>>>
>>> I assume the main concern her is that it's implemented in a rather not
>>> extensible way. Also, if I understand correctly, it compresses the data
>>> stream
>>> in both direction server <-> client, not sure if there is any value in
>>> compressing what a client sends to a server. But still I'm wondering why
>>> it
>>> didn't start at least a discussion about how it can be implemented. Do I
>>> miss
>>> something?
>>>
>>
>> Implementation of libpq compression will be included in next release of
>> PgProEE.
>> Looks like community is not so interested in this patch. Frankly speaking
>> I do not understand why.
>>
>
I'm definitely very interested, and simply missed the post.

I'll talk with some team mates as we're doing some PG12 planning now.


> Yes, this patch is not extensible: it can use either zlib either zstd.
>> Unfortunately internal Postgres compression pglz doesn't provide streaming
>> API.
>> May be it is good idea to combine it with Ildus patch (custom compression
>> methods): https://commitfest.postgresql.org/18/1294/
>>
>
Given the history of issues with attempting custom/pluggable compression
for toast etc, I really wouldn't want to couple those up.

pglz wouldn't make much sense for protocol compression anyway, except maybe
for fast local links where it was worth a slight compression overhead but
not the cpu needed for gzip. I don't think it's too exciting. zlib/gzip is
likely the sweet spot for the reasonable future for protocol compression,
or a heck of a lot better than what we have, anyway.

We should make sure the protocol part is extensible, but the implementation
doesn't need to be pluggable.

In this case it will be possible to use any custom compression algorithm.
>> But we need to design and implement streaming API for pglz and other
>> compressors.
>>
>
> I'm sure there is plenty of interest in this. However, you guys need to
> understand where we are in the development cycle. We're trying to wrap up
> Postgres 11, which was feature frozen before this patch ever landed. So
> it's material for Postgres 12. That means it will probably need to wait a
> little while before it gets attention. It doesn't mean nobody is interested.
>
> I disagree with Dmitry about compressing in both directions - I can think
> of plenty of good cases where we would want to compress traffic from the
> client.
>

Agreed. The most obvious case being COPY, but there's also big bytea
values, etc.


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

Re: Postgres 11 release notes

[Tom Lane]

David Rowley  writes:
> On 15 May 2018 at 08:28, Bruce Momjian  wrote:
>> Consistently return NaN for
>> NaN inputs to power()
>> on older platforms (Dang Minh Huong)

> While I'm not in favour of removing Dang's credit here, technically
> this patch was Tom's. The code added in float.c by Dang's patch
> (61b200e2f) was effectively reverted by 6bdf1303.  Dang's regression
> tests remain, so should also be credited along with Tom.

I'm not particularly fussed about getting credit for that.  However,
looking again at how that patch series turned out --- ie, that
we ensured POSIX behavior for NaNs only in HEAD --- I wonder
whether we shouldn't do what was mentioned in the commit log for
6bdf1303, and teach numeric_pow() about these same special cases.
It seems like it would be more consistent to change both functions
for v11, rather than letting that other shoe drop in some future
major release.

regards, tom lane

Re: Allow COPY's 'text' format to output a header

[Daniel Verite]

Andrew Dunstan wrote:

> I'm not necessarily opposed to this, but I'm not certain about the use
> case either.

+1.
The downside is that it would create the need, when using COPY TO,
to know whether an input file was generated with or without header,
and a hazard on mistakes.
If you say it was and it wasn't, you quietly loose the first row of data.
If you say it wasn't and in fact it was, either there's a
datatype mismatch or you quietly get a spurious row of data.

This complication should be balanced by some advantage.
What can we do with the header?
If you already have the table ready to COPY in, you don't
need that information. The only reason why COPY TO
needs to know about the header is to throw it away.
And if you don't have the table created yet, a header
with just the column names is hardly sufficient to create it,
isn't it?


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

Re: libpq compression

[Euler Taveira]

2018-05-15 9:53 GMT-03:00 Konstantin Knizhnik :
> Looks like community is not so interested in this patch. Frankly speaking I
> do not understand why.
>
AFAICS the lack of replies is due to feature freeze. I'm pretty sure
people are interested in this topic (at least I am). Did you review a
previous discussion [1] about this?

I did a prototype a few years ago. I didn't look at your patch yet.
I'll do in a few weeks. Please add your patch to the next CF [2].


[1] https://www.postgresql.org/message-id/4FD9698F.2090407%40timbira.com
[2] https://commitfest.postgresql.org/18/


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

[BUGFIX] amcanbackward is not checked before building backward index paths

[Nikita Glukhov]

Hi hackers!

Experimenting with the new pluggable storage API, I found that amcanbackward
flag is not checked in build_index_paths() before
build_index_pathkeys(... BackwardScanDirection) call when we are building
paths for ORDER BY.  And this flag is even not copied into IndexOptInfo struct.
Obviously, this can lead to misuse of Backward Index [Only] Scan plans.

Attached patch with the corresponding fix.

There are no test cases because now only btree supports ordered scans but it
supports backward scans too.

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

>From 2bb9a17a8efb166339bcd5618157728a4b596430 Mon Sep 17 00:00:00 2001
From: Nikita Glukhov 
Date: Sun, 13 May 2018 16:37:00 +0300
Subject: [PATCH] Disable backward scans on indices that do not support it

---
 src/backend/optimizer/path/indxpath.c | 2 +-
 src/backend/optimizer/util/plancat.c  | 1 +
 src/include/nodes/relation.h  | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index f295558..caa688c 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -1084,7 +1084,7 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
 	/*
 	 * 5. If the index is ordered, a backwards scan might be interesting.
 	 */
-	if (index_is_ordered && pathkeys_possibly_useful)
+	if (index_is_ordered && pathkeys_possibly_useful && index->amcanbackward)
 	{
 		index_pathkeys = build_index_pathkeys(root, index,
 			  BackwardScanDirection);
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 8369e3a..470d38e 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -271,6 +271,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 			info->amsearcharray = amroutine->amsearcharray;
 			info->amsearchnulls = amroutine->amsearchnulls;
 			info->amcanparallel = amroutine->amcanparallel;
+			info->amcanbackward = amroutine->amcanbackward;
 			info->amhasgettuple = (amroutine->amgettuple != NULL);
 			info->amhasgetbitmap = (amroutine->amgetbitmap != NULL);
 			info->amcostestimate = amroutine->amcostestimate;
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 3b28d19..593ee48 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -798,6 +798,7 @@ typedef struct IndexOptInfo
 	bool		amhasgettuple;	/* does AM have amgettuple interface? */
 	bool		amhasgetbitmap; /* does AM have amgetbitmap interface? */
 	bool		amcanparallel;	/* does AM support parallel scan? */
+	bool		amcanbackward;	/* does AM support backward scan? */
 	/* Rather than include amapi.h here, we declare amcostestimate like this */
 	void		(*amcostestimate) ();	/* AM's cost estimator */
 } IndexOptInfo;
-- 
2.7.4

Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Mark Dilger]

Hackers,

There was a bug report sent by Hao Lee about Windows build breakage,
"BUG #15167: error C2365: 'errcode' : redefinition; previous definition"
https://www.postgresql.org/message-id/152446498404.19807.4659286570762153837%40wrigleys.postgresql.org

Heikki was the only person who responded to Hao, AFAIK, though that
conversation did not go far.  Perhaps Heikki was unable to reproduce?  

Hao did not specify which commit caused the problem, so I started
bisecting to find out.

With my Windows build setup, building from master, the builds break
staring with da9b580d89903fee871cf54845ffa2b26bda2e11 committed by
Stephen on Apr 7 2018.



Build FAILED.

"D:\jenkins\workspace\trunk\dist\msi\BUILD\postgresql\pgsql.sln" (default 
target) (1) ->
(postgres target) -> 
  .\src\backend\replication\basebackup.c(1470): warning C4146: unary minus 
operator applied to unsigned type, result still unsigned


"D:\jenkins\workspace\trunk\dist\msi\BUILD\postgresql\pgsql.sln" (default 
target) (1) ->
(postgres target) -> 
  .\src\common\file_perm.c(18): error C2065: 'S_IRWXU' : undeclared identifier
  .\src\common\file_perm.c(18): error C2099: initializer is not a constant
  .\src\common\file_perm.c(19): error C2065: 'S_IRUSR' : undeclared identifier
  .\src\common\file_perm.c(19): error C2065: 'S_IWUSR' : undeclared identifier
  .\src\common\file_perm.c(19): error C2099: initializer is not a constant



The build breaks in a different way staring with
c37b3d08ca6873f9d4eaf24c72a90a550970cbb8 committed by Stephen later
that same day.



Microsoft (R) Build Engine Version 3.5.30729.5420
[Microsoft .NET Framework, Version 2.0.50727.8784]
Copyright (C) Microsoft Corporation 2007. All rights reserved.

Build started 5/15/2018 5:24:08 AM.
Project "D:\jenkins\workspace\trunk\dist\msi\BUILD\postgresql\pgsql.sln" on 
node 0 (default targets).
 Building solution configuration "Release|x64".
:
 error C2365: 'errcode' : redefinition; previous definition was 'typedef'
Done Building Project 
"D:\jenkins\workspace\trunk\dist\msi\BUILD\postgresql\pgsql.sln" (default 
targets) -- FAILED.

Build FAILED.



Since I am able to reproduce the problem, I'd like to help debug the
problem.  I find it frustrating that the compiler is not specifying
*where* the prior typedef comes from.

My best guess at the moment is:

diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index c1f0441b08..0a3163398f 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -16,8 +16,11 @@
  *
  *-
  */
+#include 
+
 #include "postgres.h"
 
+#include "common/file_perm.h"
 #include "libpq/libpq-be.h"
 #include "libpq/pqcomm.h"
 #include "miscadmin.h"

Which makes me wonder if  on Windows declares a typedef for errcode?
The work-around in src/include/port/win32.h which attempts to deal with system
headers defining errcode looks like it won't work unless it gets included 
*before*
the offending system header, which appears not to be true for globals.c.

Indeed, the following change (shown here for illustrative purposes only; please
don't commit it this way) fixes the problem, at least in my build environment:

diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c
index 9f1209323a..1622b0be62 100644
--- a/src/backend/utils/init/globals.c
+++ b/src/backend/utils/init/globals.c
@@ -16,7 +16,14 @@
  *
  *-
  */
+
+#if defined(_WIN32) || defined(WIN32) || defined(_MSC_VER) || 
defined(HAVE_CRTDEFS_H)
+#define errcode __msvc_errcode
+#include 
+#undef errcode
+#else
 #include 
+#endif
 
 #include "postgres.h"
 


Let me know if there are any tests you'd like me to perform to further
investigate.


mark

Re: Allow COPY's 'text' format to output a header

[Isaac Morland]

On 15 May 2018 at 10:26, Daniel Verite  wrote:

> Andrew Dunstan wrote:
>
> > I'm not necessarily opposed to this, but I'm not certain about the use
> > case either.
>
> +1.
> The downside is that it would create the need, when using COPY TO,
> to know whether an input file was generated with or without header,
> and a hazard on mistakes.
> If you say it was and it wasn't, you quietly loose the first row of data.
> If you say it wasn't and in fact it was, either there's a
> datatype mismatch or you quietly get a spurious row of data.
>

Just to be clear, we're talking about my "header match" feature, not the
basic idea of allowing a header in text format?

You already need to know whether or not there is a header, no matter what:
there is no way to avoid needing to know the format of the data to be
imported. And certainly if "header" is an option, one has to know whether
or not to set it in any given situation.

The "header match" helps ensure the file is the right one by requiring the
header contents to match the field names, rather than just being thrown
away.

I don't view it as a way to avoid pre-defining the table. It just increases
the chance that the wrong file won't load but will instead trigger an error
condition immediately.

Note that this advantage includes what happens if you specify header but
the file has no header: as long as you actually specified header match, the
error will be caught unless the first row of actual data happens to match
the field names, which is almost always highly unlikely and frequently
impossible (e.g., a person with firstname "firstname", surname "surname",
birthday "birthday" and so on).

One can imagine extensions of the idea: for example, the header could
actually be used to identify the columns, so the column order in the file
doesn't matter. There could also be an "AS" syntax to allow the target
field names to be different from the field names in the header. I have
occasionally found myself wanting to ignore certain columns of the file.
But these are all significantly more complicated than just looking at the
header and requiring it to match the target field names.

If one specifies no header but there actually is a header in the file, then
loading will fail in many cases but it depends on what the header in the
file looks like. This part is unaffected by my idea.


> This complication should be balanced by some advantage.
> What can we do with the header?
> If you already have the table ready to COPY in, you don't
> need that information. The only reason why COPY TO
> needs to know about the header is to throw it away.
> And if you don't have the table created yet, a header
> with just the column names is hardly sufficient to create it,
> isn't it?
>

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Tom Lane]

Mark Dilger  writes:
> My best guess at the moment is:

> diff --git a/src/backend/utils/init/globals.c 
> b/src/backend/utils/init/globals.c
> index c1f0441b08..0a3163398f 100644
> --- a/src/backend/utils/init/globals.c
> +++ b/src/backend/utils/init/globals.c
> @@ -16,8 +16,11 @@
>   *
>   *-
>   */
> +#include 
> +
>  #include "postgres.h"
 
> +#include "common/file_perm.h"

Yipes.  Frost, you didn't really do that did you?  That's a blatant
break of the "c.h must come first" rule.  Whether or not it broke the
Windows build, there are other platforms it'll break.

> Indeed, the following change (shown here for illustrative purposes only; 
> please
> don't commit it this way) fixes the problem, at least in my build environment:

That's pretty ugly, but what happens if you just move the 
inclusion to immediately after postgres.h, as is our normal custom?

regards, tom lane

Re: Allow COPY's 'text' format to output a header

[Tom Lane]

Isaac Morland  writes:
> On 15 May 2018 at 10:26, Daniel Verite  wrote:
>> Andrew Dunstan wrote:
>>> I'm not necessarily opposed to this, but I'm not certain about the use
>>> case either.

>> The downside is that it would create the need, when using COPY TO,
>> to know whether an input file was generated with or without header,
>> and a hazard on mistakes.
>> If you say it was and it wasn't, you quietly loose the first row of data.
>> If you say it wasn't and in fact it was, either there's a
>> datatype mismatch or you quietly get a spurious row of data.

> Just to be clear, we're talking about my "header match" feature, not the
> basic idea of allowing a header in text format?

AFAICS, Daniel's just reacting to the basic idea of a header line.
I agree that by itself that's not worth much.  However, if we added
your proposed option to insist that the column names match during COPY
IN, I think that that could have some value.  It would allow
forestalling one common type of pilot error, ie copying the wrong file
entirely.  (It'd also prevent copying in data that has the wrong column
order, but I think that's a less common scenario.  I might be wrong
about that.)

> One can imagine extensions of the idea: for example, the header could
> actually be used to identify the columns, so the column order in the file
> doesn't matter. There could also be an "AS" syntax to allow the target
> field names to be different from the field names in the header. I have
> occasionally found myself wanting to ignore certain columns of the file.
> But these are all significantly more complicated than just looking at the
> header and requiring it to match the target field names.

Yeah, and every bit of flexibility you add raises the chance of an
undetected error.  COPY isn't intended as a general ETL facility,
so I'd mostly be -1 on adding such things.  But I can see the value
of confirming that you're copying the right file, and a header match
check would go a long way towards doing that.

regards, tom lane

Re: Allow COPY's 'text' format to output a header

[David G. Johnston]

On Tuesday, May 15, 2018, Tom Lane  wrote:
>
>
> AFAICS, Daniel's just reacting to the basic idea of a header line.
> I agree that by itself that's not worth much.  However, if we added
> your proposed option to insist that the column names match during COPY
> IN, I think that that could have some value.
>

I'm fine for adding it without the added matching behavior, though turning
the boolean into an enum is appealing.

HEADER { true | false | match }

Though we'd need to accept all variants of Boolean for compatability...

I'm of the opinion that text and csv should be the same excepting their
defaults for some of the options.

David J.

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Mark Dilger]

> On May 15, 2018, at 8:58 AM, Tom Lane  wrote:
> 
> Mark Dilger  writes:
>> My best guess at the moment is:
> 
>> diff --git a/src/backend/utils/init/globals.c 
>> b/src/backend/utils/init/globals.c
>> index c1f0441b08..0a3163398f 100644
>> --- a/src/backend/utils/init/globals.c
>> +++ b/src/backend/utils/init/globals.c
>> @@ -16,8 +16,11 @@
>>  *
>>  *-
>>  */
>> +#include 
>> +
>> #include "postgres.h"
> 
>> +#include "common/file_perm.h"
> 
> Yipes.  Frost, you didn't really do that did you?  That's a blatant
> break of the "c.h must come first" rule.  Whether or not it broke the
> Windows build, there are other platforms it'll break.
> 
>> Indeed, the following change (shown here for illustrative purposes only; 
>> please
>> don't commit it this way) fixes the problem, at least in my build 
>> environment:
> 
> That's pretty ugly, but what happens if you just move the 
> inclusion to immediately after postgres.h, as is our normal custom?

That also works.

mark

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Stephen Frost]

Greetings,

* Mark Dilger (hornschnor...@gmail.com) wrote:
> > On May 15, 2018, at 8:58 AM, Tom Lane  wrote:
> > Mark Dilger  writes:
> >> My best guess at the moment is:
> > 
> >> diff --git a/src/backend/utils/init/globals.c 
> >> b/src/backend/utils/init/globals.c
> >> index c1f0441b08..0a3163398f 100644
> >> --- a/src/backend/utils/init/globals.c
> >> +++ b/src/backend/utils/init/globals.c
> >> @@ -16,8 +16,11 @@
> >>  *
> >>  *-
> >>  */
> >> +#include 
> >> +
> >> #include "postgres.h"
> > 
> >> +#include "common/file_perm.h"
> > 
> > Yipes.  Frost, you didn't really do that did you?  That's a blatant
> > break of the "c.h must come first" rule.  Whether or not it broke the
> > Windows build, there are other platforms it'll break.

Evidently I managed to.

> >> Indeed, the following change (shown here for illustrative purposes only; 
> >> please
> >> don't commit it this way) fixes the problem, at least in my build 
> >> environment:
> > 
> > That's pretty ugly, but what happens if you just move the 
> > inclusion to immediately after postgres.h, as is our normal custom?
> 
> That also works.

Good, will fix.

Thanks!

Stephen


signature.asc
Description: PGP signature

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Mark Dilger]

> On May 15, 2018, at 9:29 AM, Stephen Frost  wrote:
> 
> Greetings,
> 
> * Mark Dilger (hornschnor...@gmail.com) wrote:
>>> On May 15, 2018, at 8:58 AM, Tom Lane  wrote:
>>> Mark Dilger  writes:
 My best guess at the moment is:
>>> 
 diff --git a/src/backend/utils/init/globals.c 
 b/src/backend/utils/init/globals.c
 index c1f0441b08..0a3163398f 100644
 --- a/src/backend/utils/init/globals.c
 +++ b/src/backend/utils/init/globals.c
 @@ -16,8 +16,11 @@
 *
 *-
 */
 +#include 
 +
 #include "postgres.h"
>>> 
 +#include "common/file_perm.h"
>>> 
>>> Yipes.  Frost, you didn't really do that did you?  That's a blatant
>>> break of the "c.h must come first" rule.  Whether or not it broke the
>>> Windows build, there are other platforms it'll break.
> 
> Evidently I managed to.
> 
 Indeed, the following change (shown here for illustrative purposes only; 
 please
 don't commit it this way) fixes the problem, at least in my build 
 environment:
>>> 
>>> That's pretty ugly, but what happens if you just move the 
>>> inclusion to immediately after postgres.h, as is our normal custom?
>> 
>> That also works.
> 
> Good, will fix.

I'm curious why the Windows build farm members did not pick this up.  Or
perhaps they did?  (I don't get emails about that.)  If none of the animals
are configured to detect this bug, perhaps the community needs another
Windows animal configured along the lines of the build machine I am using?

Please advise...

mark

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Tom Lane]

Mark Dilger  writes:
> I'm curious why the Windows build farm members did not pick this up.  Or
> perhaps they did?  (I don't get emails about that.)

They did not, and I too was wondering why not.

> If none of the animals
> are configured to detect this bug, perhaps the community needs another
> Windows animal configured along the lines of the build machine I am using?

+1.  How do you have yours configured, anyway?

regards, tom lane

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Mark Dilger]

> On May 15, 2018, at 9:54 AM, Tom Lane  wrote:
> 
> Mark Dilger  writes:
>> I'm curious why the Windows build farm members did not pick this up.  Or
>> perhaps they did?  (I don't get emails about that.)
> 
> They did not, and I too was wondering why not.
> 
>> If none of the animals
>> are configured to detect this bug, perhaps the community needs another
>> Windows animal configured along the lines of the build machine I am using?
> 
> +1.  How do you have yours configured, anyway?

I mostly develop on mac and linux and don't look at the windows system
too much:


Windows Server 2008 R2 Standard
Service Pack 1
Processor: Intel Xeon CPU E5-2676 v3 @ 2.40GHz
Installed memory:  8.00 GB
System type: 64 bit Operating System

Microsoft Visual Studio 2008 Beta2 x64 cross tools
Microsoft (R) C/C++ Optimizing Compiler Version 15.00.21022.08 for x64

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Tom Lane]

Mark Dilger  writes:
>>> If none of the animals
>>> are configured to detect this bug, perhaps the community needs another
>>> Windows animal configured along the lines of the build machine I am using?

>> +1.  How do you have yours configured, anyway?

> I mostly develop on mac and linux and don't look at the windows system
> too much:

> Windows Server 2008 R2 Standard
> Service Pack 1
> Microsoft Visual Studio 2008 Beta2 x64 cross tools

Hm.  I'm not sure what our nominal support range for Visual Studio is.
I see that mastodon (running VS2005) and currawong (running VS2008)
haven't reported on HEAD lately, and I think they may have been shut
down intentionally due to desupport?  But if your build still works
then it seems like we could continue to support VS2008.

regards, tom lane

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Mark Dilger]

> On May 15, 2018, at 10:20 AM, Tom Lane  wrote:
> 
> Mark Dilger  writes:
 If none of the animals
 are configured to detect this bug, perhaps the community needs another
 Windows animal configured along the lines of the build machine I am using?
> 
>>> +1.  How do you have yours configured, anyway?
> 
>> I mostly develop on mac and linux and don't look at the windows system
>> too much:
> 
>> Windows Server 2008 R2 Standard
>> Service Pack 1
>> Microsoft Visual Studio 2008 Beta2 x64 cross tools
> 
> Hm.  I'm not sure what our nominal support range for Visual Studio is.
> I see that mastodon (running VS2005) and currawong (running VS2008)
> haven't reported on HEAD lately, and I think they may have been shut
> down intentionally due to desupport?  But if your build still works
> then it seems like we could continue to support VS2008.

I don't have a strong opinion on that.  I could also look to upgrade
to a newer version.  Generally, I try to build using the oldest
supported version rather than the newest.  What is the next oldest
working test machine you have?

mark

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Tom Lane]

Mark Dilger  writes:
>> On May 15, 2018, at 10:20 AM, Tom Lane  wrote:
>> Hm.  I'm not sure what our nominal support range for Visual Studio is.
>> I see that mastodon (running VS2005) and currawong (running VS2008)
>> haven't reported on HEAD lately, and I think they may have been shut
>> down intentionally due to desupport?  But if your build still works
>> then it seems like we could continue to support VS2008.

> I don't have a strong opinion on that.  I could also look to upgrade
> to a newer version.  Generally, I try to build using the oldest
> supported version rather than the newest.  What is the next oldest
> working test machine you have?

thrips is described as running VS2010, though I'm not sure how to verify
that it's not been updated.  The make log shows

  c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\AMD64\CL.exe

but is "10.0" the same thing as "2010"?

regards, tom lane

Re: Make description of heap records more talkative for flags

[Alvaro Herrera]

On 2018-Apr-23, Alvaro Herrera wrote:

> Now, frankly, this being mostly a debugging tool, I think it would be
> better to have the output as complete as we can.  Otherwise, when
> debugging some hideous problem we find ourselves patching the tool
> during an emergency in order to figure out what is going on.  For this,
> I would rather patch the earliest version we conveniently can.  We
> cannot patch pg10 because it's already out there, but not so for pg11.

RMT, I would like your opinion on whether to apply a fix for this
problem to pg11 or not.

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

Re: Allow COPY's 'text' format to output a header

[Daniel Verite]

Isaac Morland wrote:

> Just to be clear, we're talking about my "header match" feature, not the
> basic idea of allowing a header in text format?

For my reply it was on merely allowing it, as does the current
patch at https://commitfest.postgresql.org/18/1629

Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Andrew Dunstan]

On 05/15/2018 01:20 PM, Tom Lane wrote:

Mark Dilger  writes:

If none of the animals
are configured to detect this bug, perhaps the community needs another
Windows animal configured along the lines of the build machine I am using?

+1.  How do you have yours configured, anyway?

I mostly develop on mac and linux and don't look at the windows system
too much:
Windows Server 2008 R2 Standard
Service Pack 1
Microsoft Visual Studio 2008 Beta2 x64 cross tools

Hm.  I'm not sure what our nominal support range for Visual Studio is.
I see that mastodon (running VS2005) and currawong (running VS2008)
haven't reported on HEAD lately, and I think they may have been shut
down intentionally due to desupport?  But if your build still works
then it seems like we could continue to support VS2008.





currawong and friends were shut down in January on >= 11 because of the 
huge pages issue on 32-bit XP. So they aren't going to be revived. Maybe 
we can make VS2008 work on a 64 bit machine. Personally I'm more 
interested in better getting coverage for the modern compilers.



cheers

andrew

Re: Make description of heap records more talkative for flags

[Andres Freund]

On 2018-05-15 13:44:58 -0400, Alvaro Herrera wrote:
> On 2018-Apr-23, Alvaro Herrera wrote:
> 
> > Now, frankly, this being mostly a debugging tool, I think it would be
> > better to have the output as complete as we can.  Otherwise, when
> > debugging some hideous problem we find ourselves patching the tool
> > during an emergency in order to figure out what is going on.  For this,
> > I would rather patch the earliest version we conveniently can.  We
> > cannot patch pg10 because it's already out there, but not so for pg11.
> 
> RMT, I would like your opinion on whether to apply a fix for this
> problem to pg11 or not.

-0.1. We've lived without this for years, I fail to see why this should
get an exception / has any sort of urgency. We could establish a policy
that we just exclude parts of the source tree from the code freeze, but
we haven't so far.

Greetings,

Andres Freund

Re: Postgres 11 release notes

[Bruce Momjian]

On Tue, May 15, 2018 at 08:45:07AM +0530, Amit Kapila wrote:
> No, it is not like that.  We divide the scan among workers and each
> worker should perform projection of the rows it scanned (after
> applying filter).  Now, if the expensive functions are part of target
> lists, then we can push the computation of expensive functions (as
> part of target list) in workers which will divide the work.
> 
> >  Really?  Do
> > we run each column in its own worker or do we split the result set into
> > parts and run those in parallel?  How do we know, just the function call
> > costs?
> >
> 
> The function's cost can be determined via pg_proc->procost.  For this
> particular case, you can refer the call graph -
> create_pathtarget->set_pathtarget_cost_width->cost_qual_eval_node->cost_qual_eval_walker->get_func_cost
> 
> >  I can admit I never saw that coming.
> >
> 
> I think the use case becomes interesting with parallel query because
> now you can divide such cost among workers.
> 
> Feel free to ask more questions if above doesn't clarify the usage of
> these features.

OK, I have added the following release note item for both of these:

2017-11-16 [e89a71fb4] Pass InitPlan values to workers via Gather (Merge).
2018-03-29 [3f90ec859] Postpone generate_gather_paths for topmost scan/join rel
2018-03-29 [11cf92f6e] Rewrite the code that applies scan/join targets to paths

Allow single-evaluation queries, e.g. FROM
clause queries, and functions in the target list to be
parallelized (Amit Kapila, Robert Haas)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Windows build broken starting at da9b580d89903fee871cf54845ffa2b26bda2e11

[Pantelis Theodosiou]

On Tue, May 15, 2018 at 6:29 PM, Tom Lane  wrote:

> Mark Dilger  writes:
>
> > I don't have a strong opinion on that.  I could also look to upgrade
> > to a newer version.  Generally, I try to build using the oldest
> > supported version rather than the newest.  What is the next oldest
> > working test machine you have?
>
> thrips is described as running VS2010, though I'm not sure how to verify
> that it's not been updated.  The make log shows
>
>   c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin\AMD64\CL.exe
>
> but is "10.0" the same thing as "2010"?
>
> regards, tom lane
>
>
According to Wikipedia, yes:
https://en.wikipedia.org/wiki/Microsoft_Visual_Studio

(although the 10s are only by coincidence, 11.0 is VS 2012, ...)

Re: Postgres 11 release notes

[David Rowley]

On 16 May 2018 at 02:01, Tom Lane  wrote:
> David Rowley  writes:
>> While I'm not in favour of removing Dang's credit here, technically
>> this patch was Tom's. The code added in float.c by Dang's patch
>> (61b200e2f) was effectively reverted by 6bdf1303.  Dang's regression
>> tests remain, so should also be credited along with Tom.
>
> I'm not particularly fussed about getting credit for that.  However,
> looking again at how that patch series turned out --- ie, that
> we ensured POSIX behavior for NaNs only in HEAD --- I wonder
> whether we shouldn't do what was mentioned in the commit log for
> 6bdf1303, and teach numeric_pow() about these same special cases.
> It seems like it would be more consistent to change both functions
> for v11, rather than letting that other shoe drop in some future
> major release.

I'm inclined to agree. It's hard to imagine these two functions
behaving differently in regards to NaN input is useful to anyone.

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

Re: Postgres 11 release notes

[Bruce Momjian]

On Tue, May 15, 2018 at 09:51:47AM +0900, Tatsuo Ishii wrote:
> >> I could not find this:
> >> 
> >> $ git log -1 f8e5f15
> >> commit f8e5f156b30efee5d0038b03e38735773abcb7ed
> >> Author: Andres Freund 
> >> Date:   Mon Sep 18 19:36:44 2017 -0700
> >> 
> >> Rearm statement_timeout after each executed query.
> >> 
> >> Previously statement_timeout, in the extended protocol, affected all
> >> messages till a Sync message.  For clients that pipeline/batch query
> >> execution that's problematic.
> >> 
> >> Instead disable timeout after each Execute message, and enable, if
> >> necessary, the timer in start_xact_command(). As that's done only for
> >> Execute and not Parse / Bind, pipelining the latter two could still
> >> cause undesirable timeouts. But a survey of protocol implementations
> >> shows that all drivers issue Sync messages when preparing, and adding
> >> timeout rearming to both is fairly expensive for the common parse /
> >> bind / execute sequence.
> >> 
> >> Author: Tatsuo Ishii, editorialized by Andres Freund
> >> Reviewed-By: Takayuki Tsunakawa, Andres Freund
> >> Discussion: 
> >> https://postgr.es/m/20170222.115044.1665674502985097185.t-is...@sraoss.co.jp
> > 
> > That seemed too detailed for the release notes.  Is that wrong?
> 
> This commit gives user-visible changes to the statment timeout
> behavior. So I think this should be added to the release notes.

OK, makes sense. Here is what I added:

2017-09-18 [f8e5f156b] Rearm statement_timeout after each executed 
query.

In the Extended Query
Protocol, have statement_timeout apply
to each Execute message, not to all commands
before Sync (Tatsuo Ishii)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

index scan over composite type

[Teodor Sigaev]

Hi!

I'm not understand why postgres prefers to sort table instead of using 
index only scan when query is a simple inner join on composite type. 
Query with equality clause with constant works fine with index scan but 
join not. Could somebody point me why? Thank you.


And I'm not able to force merge_join with index scans with any 
combination of enable_* variables.


 Attached script is a self-contained test script. Pg config file is 
default.


explain
select
a.idv, b.idv
from
a, b
where
a.idv = b.idv;


 Merge Join  (cost=25751.64..27751.64 rows=10 width=74)
   Merge Cond: (a.idv = b.idv)
   ->  Sort  (cost=12875.82..13125.82 rows=10 width=37)
 Sort Key: a.idv
 ->  Seq Scan on a  (cost=0.00..1834.00 rows=10 width=37)
   ->  Sort  (cost=12875.82..13125.82 rows=10 width=37)
 Sort Key: b.idv
 ->  Seq Scan on b  (cost=0.00..1834.00 rows=10 width=37)

--
Teodor Sigaev  E-mail: teo...@sigaev.ru
  WWW: http://www.sigaev.ru/


test.sql
Description: application/sql

Re: Postgres 11 release notes

[Bruce Momjian]

On Tue, May 15, 2018 at 06:40:18AM +, Huong Dangminh wrote:
> Hi,
> 
> > From: David Rowley [mailto:david.row...@2ndquadrant.com]
> > Sent: Tuesday, May 15, 2018 3:01 PM
> > To: Bruce Momjian 
> > Cc: Đặng Minh Hướng ; PostgreSQL-development
> > 
> > Subject: Re: Postgres 11 release notes
> > 
> > On 15 May 2018 at 08:28, Bruce Momjian  wrote:
> > > Consistently return NaN for
> > > NaN inputs to power()
> > > on older platforms (Dang Minh Huong)
> > 
> > While I'm not in favour of removing Dang's credit here, technically this
> > patch was Tom's. The code added in float.c by Dang's patch
> > (61b200e2f) was effectively reverted by 6bdf1303.  Dang's regression tests
> > remain, so should also be credited along with Tom.
> 
> Thanks David, I agreed.
> Also 6bdf1303 should be included like below,
> 
> 
>  
> 
>  Consistently return NaN for
>  NaN inputs to power()
> -on older platforms (Dang Minh Huong)
> +on older platforms (Tom Lane, Dang Minh Huong)
> 

OK, changed, thanks.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Postgres 11 release notes

[Bruce Momjian]

On Tue, May 15, 2018 at 02:26:33PM +0900, Etsuro Fujita wrote:
> (2018/05/12 0:08), Bruce Momjian wrote:
> >I have committed the first draft of the Postgres 11 release notes.  I
> >will add more markup soon.  You can view the most current version here:
> >
> > http://momjian.us/pgsql_docs/release-11.html
> >
> >I expect a torrent of feedback.  ;-)
> >
> >On a personal note, I want to apologize for not adequately championing
> >two features that I think have strategic significance but didn't make it
> >into Postgres 11:  parallel FDW access and improved multi-variate
> >statistics.  I hope to do better job during Postgres 12.
> 
> Thanks for compiling this, Bruce!
> 
> I found a copy-pasto in this:
> 
> Below you will find a detailed account of the changes between
> PostgreSQL 10 and the previous major
> release.
> 
> I think the PG version should be 11, not 10.  Patch attached.

Ah, seems I missed that one, thanks.

---


> 
> Best regards,
> Etsuro Fujita

> diff --git a/doc/src/sgml/release-11.sgml b/doc/src/sgml/release-11.sgml
> index 4b31b46..407c67b 100644
> --- a/doc/src/sgml/release-11.sgml
> +++ b/doc/src/sgml/release-11.sgml
> @@ -345,7 +345,7 @@ Branch: master [6bdf1303b] Avoid wrong results for 
> power() with NaN
>  
> 
>  Below you will find a detailed account of the changes between
> -PostgreSQL 10 and the previous major
> +PostgreSQL 11 and the previous major
>  release.
> 
>  


-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: explain (verbose off, normalized) vs query planid

[legrand legrand]

This is for tracking planid into pg_stat_statements too.

A first try is available here
http://www.postgresql-archive.org/Poc-pg-stat-statements-with-planid-td6014027.html

reusing pg_stat_plans's "Plan tree Jumbling algorithm" from Peter Geoghegan.

Hashing the normalized query plan text in one pass (that is also based on
plan tree), 
compared to that Jumbling method seems simple to maintain (if explain works,
planid is available and accurate). 
Today actual planid jumbling comes from pg9.3, doesn't compile anymore, and
I'm not able to verify 
if declarativ partitionning is properly handled ...  

Would there be some functional or performances reasons to prefer jumbling to
hashing normalized plan text?

Regards
PAscal   



--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

Should total_pages be calculated after partition pruning and constraint exclusion?

[David Rowley]

It seems quite strange to me that we calculate total_pages before any
partition pruning and constraint exclusion is performed during
set_base_rel_sizes(). Wouldn't it be better to wait until after that's
done so we don't mistakenly count relations we're not going to scan?

Delaying the calculation until after set_base_rel_sizes() means it's
still done in time before it's needed in set_base_rel_pathlists().

The attached patch implements the change.

There are no visible plan changes in the regression tests, but the
change can affect the plans for larger partitioned tables.

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


total_pages_after_pruning.patch
Description: Binary data

Re: Postgres 11 release notes

[Bruce Momjian]

On Tue, May 15, 2018 at 01:04:55PM +0530, Amit Kapila wrote:
> On Tue, May 15, 2018 at 11:43 AM, David Rowley
>  wrote:
> > On 12 May 2018 at 03:08, Bruce Momjian  wrote:
> >> I have committed the first draft of the Postgres 11 release notes.  I
> >> will add more markup soon.  You can view the most current version here:
> >>
> >> http://momjian.us/pgsql_docs/release-11.html
> >>
> >> I expect a torrent of feedback.  ;-)
> >
> > I wonder if 3cda10f41bfed -- "Use atomic ops to hand out pages to scan
> > in parallel scan." should be listed in the notes.
> >
> > If so, I'd suggest something like:
> >
> > Improve performance of Parallel Seq Scans with many parallel workers
> > (David Rowley).
> >
> 
> +1 to add this in Release Notes.

Done, and URL updated with all confirmed changed.  :-)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Should total_pages be calculated after partition pruning and constraint exclusion?

[Tom Lane]

David Rowley  writes:
> It seems quite strange to me that we calculate total_pages before any
> partition pruning and constraint exclusion is performed during
> set_base_rel_sizes(). Wouldn't it be better to wait until after that's
> done so we don't mistakenly count relations we're not going to scan?

> Delaying the calculation until after set_base_rel_sizes() means it's
> still done in time before it's needed in set_base_rel_pathlists().

I think that when this was first done, there was not a separation between
those passes over the rangetable, so that such a simple fix would have
been impossible.

> The attached patch implements the change.

Please add to next CF.  It's a bit late to be doing such things in v11,
IMO.

regards, tom lane

Re: explain (verbose off, normalized) vs query planid

[Robert Haas]

On Tue, May 15, 2018 at 3:45 PM, legrand legrand
 wrote:
> Would there be some functional or performances reasons to prefer jumbling to
> hashing normalized plan text?

Using the text could produce different query IDs for the same plan if
any information is displayed in the text format which can vary for
reasons other than a plan change.  I don't know if there are any, but
what about, for example, the effect of GUCs on how timestamps are
displayed?  Or, much worse, what if a timer value creeps into the
output somehow?  Certainly, renaming a table is going to change the
output.  Even using a different table alias will change the output.

Using the text could produce the same query ID for different plans if
there's any relevant detail of the plan that's not shown in the text.

Basically, I would be nervous about the idea of an EXPLAIN output
that's required to reflect all and only the plan details that should
be jumbled.  The "normalized" option to EXPLAIN which you mentioned
upthread doesn't exist today...

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

Re: index scan over composite type

[Tom Lane]

Teodor Sigaev  writes:
> I'm not understand why postgres prefers to sort table instead of using 
> index only scan when query is a simple inner join on composite type. 
> Query with equality clause with constant works fine with index scan but 
> join not. Could somebody point me why? Thank you.

Hmm ... the reason why not seems to be that canonicalize_ec_expression()
improperly adds a RelabelType node, causing the composite-type Vars to not
be recognized as matching the eclass they should match.  The attached
patch fixes it and doesn't seem to break anything in the regression tests.

This raises the question of why we don't treat type RECORD more like a
true polymorphic type, but that's a can of worms I don't particularly want
to open right now.  For the moment, this is the only IsPolymorphicType
call in the planner AFAICS, so there's some reason to hope that we don't
have more bugs of the same ilk.

regards, tom lane

diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c
index 70a925c..e8cdea5 100644
--- a/src/backend/optimizer/path/equivclass.c
+++ b/src/backend/optimizer/path/equivclass.c
@@ -497,8 +497,9 @@ canonicalize_ec_expression(Expr *expr, Oid req_type, Oid req_collation)
 
 	/*
 	 * For a polymorphic-input-type opclass, just keep the same exposed type.
+	 * RECORD opclasses work like polymorphic types for this purpose.
 	 */
-	if (IsPolymorphicType(req_type))
+	if (IsPolymorphicType(req_type) || req_type == RECORDOID)
 		req_type = expr_type;
 
 	/*

Re: explain (verbose off, normalized) vs query planid

[Tom Lane]

Robert Haas  writes:
> On Tue, May 15, 2018 at 3:45 PM, legrand legrand
>  wrote:
>> Would there be some functional or performances reasons to prefer jumbling to
>> hashing normalized plan text?

> Basically, I would be nervous about the idea of an EXPLAIN output
> that's required to reflect all and only the plan details that should
> be jumbled.  The "normalized" option to EXPLAIN which you mentioned
> upthread doesn't exist today...

Indeed, and if we did write it, I think it would largely consist of
throwing away info that a jumbling mechanism could ignore far more easily.
Not to mention that we'd have to expend the cycles to emit a text
representation that we didn't actually have use for.  It sounds like a
complete loser both in terms of coding effort and runtime performance.

regards, tom lane

Re: [PROPOSAL] Shared Ispell dictionaries

[Robert Haas]

On Tue, Mar 27, 2018 at 8:19 AM, Arthur Zakirov
 wrote:
>> I assume the DSM infrastructure already has some solution for getting
>> rid of DSM segments when the last interested process disconnects,
>> so maybe you could piggyback on that somehow.
>
> Yes, there is dsm_pin_mapping() for this. But it is necessary to keep a
> segment even if there are no attached processes. From 0003:
>
> +   /* Remain attached until end of postmaster */
> +   dsm_pin_segment(seg);
> +   /* Remain attached until end of session */
> +   dsm_pin_mapping(seg);

I don't quite understand the problem you're trying to solve here, but:

1. Unless dsm_pin_segment() is called, a DSM segment will
automatically be removed when there are no remaining mappings.

2. Unless dsm_pin_mapping() is called, a DSM segment will be unmapped
when the currently-in-scope resource owner is cleaned up, like at the
end of the query.  If it is called, then the mapping will stick around
until the backend exits.

If you pin the mapping or the segment and later no longer want it
pinned, there are dsm_unpin_mapping() and dsm_unpin_segment()
functions available, too.  So it seems like what you might want to do
is pin the segment when it's created, and then unpin it if it's
stale/obsolete.  The latter won't remove it immediately, but will once
all the mappings are gone.

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

Re: Postgres 11 release notes

[Tatsuo Ishii]

>> This commit gives user-visible changes to the statment timeout
>> behavior. So I think this should be added to the release notes.
> 
> OK, makes sense. Here is what I added:
> 
>   2017-09-18 [f8e5f156b] Rearm statement_timeout after each executed 
> query.
> 
> In the Extended Query
> Protocol, have statement_timeout apply
> to each Execute message, not to all commands
> before Sync (Tatsuo Ishii)

Can you please add Andres Freund to the author? He made extensive
changes to the original patch to improve it.

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

Re: Flexible permissions for REFRESH MATERIALIZED VIEW

[Robert Haas]

On Wed, Mar 28, 2018 at 9:56 PM, David G. Johnston
 wrote:
> On Wed, Mar 28, 2018 at 6:38 PM, Isaac Morland 
> wrote:
>> One question I would have is: what proposals exist or have existed for
>> additional privilege bits? How much pressure is there to use some of the
>> remaining bits? I actually looked into the history of the permission bits
>> and found that we can summarize and approximate the history as 10 years of
>> expansion from 4 to 12, then nothing added in the last 10 years.
>
> I made an argument for an "ANALYZE" grant a little while back, and it kinda
> leads one to want one for VACUUM as well.

Yeah, and FWIW, I think that's a totally reasonable request, as is
this one.  The problem is that our authentication model seems to have
been designed under the assumption that there weren't all that many
different things you might want to separately GRANT, and the requests
we've had over the years show that this isn't the case.  So the
request is reasonable; it's just hard to implement.  I think we should
somehow move to a system where there's a set of "core" permissions
that are identified by bits for efficiency, and a set of "extended"
permissions which are identified by names for extensibility.  Things
like VACUUM and ANALYZE and REFRESH could be extended permissions.

To handle the on-disk issue, I think we could introduce a new varlena
type that's like aclitem but permits extra variable-length data at the
end.  It would be a different data type but pretty easy to convert
back and forth.  Still probably a lot of work to make it happen,
though, unfortunately.

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

parallel foreign scan

[Manuel Kniep]

Dear hackers,

I’m working on a foreign database wrapper for Kafka [1]
Now I am trying to make it parallel aware. Following 
the documentation [2]
However it seems that I can’t make it use more than a
single worker with force_parallel_mode = on.

I wonder if I need to do more than just implementing the
needed callback function to benefit from multiple workers.

Looking at create_foreignscan_path in path_nodes.c
I found that the ForeignPath seems to always set

pathnode->path.parallel_aware = false;
pathnode->path.parallel_safe = rel->consider_parallel;
pathnode->path.parallel_workers = 0;

Do I need so set these in my GetForeignPaths callback manually?

Is there anything else I need to do?

Thanks

Manuel

[1] https://github.com/adjust/kafka_fdw 

[2] 
https://www.postgresql.org/docs/current/static/fdw-callbacks.html#FDW-CALLBACKS-PARALLEL
 

Re: Make description of heap records more talkative for flags

[Jonathan S. Katz]

> On May 15, 2018, at 12:50 PM, Andres Freund  wrote:
> 
> On 2018-05-15 13:44:58 -0400, Alvaro Herrera wrote:
>> On 2018-Apr-23, Alvaro Herrera wrote:
>> 
>>> Now, frankly, this being mostly a debugging tool, I think it would be
>>> better to have the output as complete as we can.  Otherwise, when
>>> debugging some hideous problem we find ourselves patching the tool
>>> during an emergency in order to figure out what is going on.  For this,
>>> I would rather patch the earliest version we conveniently can.  We
>>> cannot patch pg10 because it's already out there, but not so for pg11.
>> 
>> RMT, I would like your opinion on whether to apply a fix for this
>> problem to pg11 or not.
> 
> -0.1. We've lived without this for years, I fail to see why this should
> get an exception / has any sort of urgency. We could establish a policy
> that we just exclude parts of the source tree from the code freeze, but
> we haven't so far.

Per Alvaro’s above comment, if this is something low-risk that could prevent
the “emergency patch” scenario later on when we actually need such
debugging flags in place, I would be ok with getting this in prior to Beta 1.
After Beta 1 I would be more risk adverse and go with Andres’s statement
around urgency and wait until the PG12 commit cycle.

And of course, we should come up with a policy, too, but I think we could
kick that can just a bit farther down the road for roughly 2 more weeks.

So +1 if we can commit this prior to Beta 1.
-1 if it waits until after.

Jonathan

Re: Make description of heap records more talkative for flags

[Andres Freund]

On 2018-05-15 16:28:23 -0500, Jonathan S. Katz wrote:
> 
> > On May 15, 2018, at 12:50 PM, Andres Freund  wrote:
> > 
> > On 2018-05-15 13:44:58 -0400, Alvaro Herrera wrote:
> >> On 2018-Apr-23, Alvaro Herrera wrote:
> >> 
> >>> Now, frankly, this being mostly a debugging tool, I think it would be
> >>> better to have the output as complete as we can.  Otherwise, when
> >>> debugging some hideous problem we find ourselves patching the tool
> >>> during an emergency in order to figure out what is going on.  For this,
> >>> I would rather patch the earliest version we conveniently can.  We
> >>> cannot patch pg10 because it's already out there, but not so for pg11.
> >> 
> >> RMT, I would like your opinion on whether to apply a fix for this
> >> problem to pg11 or not.
> > 
> > -0.1. We've lived without this for years, I fail to see why this should
> > get an exception / has any sort of urgency. We could establish a policy
> > that we just exclude parts of the source tree from the code freeze, but
> > we haven't so far.
> 
> Per Alvaro’s above comment, if this is something low-risk that could prevent
> the “emergency patch” scenario later on when we actually need such
> debugging flags in place, I would be ok with getting this in prior to
> Beta 1.

I have an *extremely* hard time that'd ever be the case. And it'd not
really change much, given several years of back branch releases.
Anyway, I'm only -0.1 here.

Greetings,

Andres Freund

NaNs in numeric_power (was Re: Postgres 11 release notes)

[Tom Lane]

David Rowley  writes:
> On 16 May 2018 at 02:01, Tom Lane  wrote:
>> I'm not particularly fussed about getting credit for that.  However,
>> looking again at how that patch series turned out --- ie, that
>> we ensured POSIX behavior for NaNs only in HEAD --- I wonder
>> whether we shouldn't do what was mentioned in the commit log for
>> 6bdf1303, and teach numeric_pow() about these same special cases.
>> It seems like it would be more consistent to change both functions
>> for v11, rather than letting that other shoe drop in some future
>> major release.

> I'm inclined to agree. It's hard to imagine these two functions
> behaving differently in regards to NaN input is useful to anyone.

Here's a proposed patch for that.

regards, tom lane

diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index dcf31e3..8dfdffc 100644
*** a/src/backend/utils/adt/numeric.c
--- b/src/backend/utils/adt/numeric.c
*** numeric_power(PG_FUNCTION_ARGS)
*** 2972,2981 
  	NumericVar	result;
  
  	/*
! 	 * Handle NaN
  	 */
! 	if (NUMERIC_IS_NAN(num1) || NUMERIC_IS_NAN(num2))
  		PG_RETURN_NUMERIC(make_result(&const_nan));
  
  	/*
  	 * Initialize things
--- 2972,2998 
  	NumericVar	result;
  
  	/*
! 	 * Handle NaN cases.  We follow the POSIX spec for pow(3), which says that
! 	 * NaN ^ 0 = 1, and 1 ^ NaN = 1, while all other cases with NaN inputs
! 	 * yield NaN (with no error).
  	 */
! 	if (NUMERIC_IS_NAN(num1))
! 	{
! 		if (!NUMERIC_IS_NAN(num2))
! 		{
! 			init_var_from_num(num2, &arg2);
! 			if (cmp_var(&arg2, &const_zero) == 0)
! PG_RETURN_NUMERIC(make_result(&const_one));
! 		}
! 		PG_RETURN_NUMERIC(make_result(&const_nan));
! 	}
! 	if (NUMERIC_IS_NAN(num2))
! 	{
! 		init_var_from_num(num1, &arg1);
! 		if (cmp_var(&arg1, &const_one) == 0)
! 			PG_RETURN_NUMERIC(make_result(&const_one));
  		PG_RETURN_NUMERIC(make_result(&const_nan));
+ 	}
  
  	/*
  	 * Initialize things
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 17985e8..1cb3c3b 100644
*** a/src/test/regress/expected/numeric.out
--- b/src/test/regress/expected/numeric.out
*** select 0.0 ^ 12.34;
*** 1664,1669 
--- 1664,1700 
   0.
  (1 row)
  
+ -- NaNs
+ select 'NaN'::numeric ^ 'NaN'::numeric;
+  ?column? 
+ --
+   NaN
+ (1 row)
+ 
+ select 'NaN'::numeric ^ 0;
+  ?column? 
+ --
+ 1
+ (1 row)
+ 
+ select 'NaN'::numeric ^ 1;
+  ?column? 
+ --
+   NaN
+ (1 row)
+ 
+ select 0 ^ 'NaN'::numeric;
+  ?column? 
+ --
+   NaN
+ (1 row)
+ 
+ select 1 ^ 'NaN'::numeric;
+  ?column? 
+ --
+ 1
+ (1 row)
+ 
  -- invalid inputs
  select 0.0 ^ (-12.34);
  ERROR:  zero raised to a negative power is undefined
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index d77504e..a939412 100644
*** a/src/test/regress/sql/numeric.sql
--- b/src/test/regress/sql/numeric.sql
*** select (-12.34) ^ 0.0;
*** 911,916 
--- 911,923 
  select 12.34 ^ 0.0;
  select 0.0 ^ 12.34;
  
+ -- NaNs
+ select 'NaN'::numeric ^ 'NaN'::numeric;
+ select 'NaN'::numeric ^ 0;
+ select 'NaN'::numeric ^ 1;
+ select 0 ^ 'NaN'::numeric;
+ select 1 ^ 'NaN'::numeric;
+ 
  -- invalid inputs
  select 0.0 ^ (-12.34);
  select (-12.34) ^ 1.2;

Re: Flexible permissions for REFRESH MATERIALIZED VIEW

[Tom Lane]

Robert Haas  writes:
> On Wed, Mar 28, 2018 at 9:56 PM, David G. Johnston
>  wrote:
>> I made an argument for an "ANALYZE" grant a little while back, and it kinda
>> leads one to want one for VACUUM as well.

> Yeah, and FWIW, I think that's a totally reasonable request, as is
> this one.  The problem is that our authentication model seems to have
> been designed under the assumption that there weren't all that many
> different things you might want to separately GRANT, and the requests
> we've had over the years show that this isn't the case.  So the
> request is reasonable; it's just hard to implement.  I think we should
> somehow move to a system where there's a set of "core" permissions
> that are identified by bits for efficiency, and a set of "extended"
> permissions which are identified by names for extensibility.  Things
> like VACUUM and ANALYZE and REFRESH could be extended permissions.

That seems like an awful lot of work to handle what's still going to be
a pretty small set of permissions.  Every permission we add is going to
have to be enforced in the C code, and it'll break applications to some
extent to treat the situation differently from before, so I don't see
that we're going to add them willy-nilly.

(For what it's worth, my first instinct would be to lump all three of
these proposals under a single grantable permission MAINTAIN, or some
such name.  I don't think it's worth subdividing more finely.)

> To handle the on-disk issue, I think we could introduce a new varlena
> type that's like aclitem but permits extra variable-length data at the
> end.  It would be a different data type but pretty easy to convert
> back and forth.  Still probably a lot of work to make it happen,
> though, unfortunately.

I think an appropriate amount of effort would be to widen AclMode to 64
bits, which wasn't practical back in the day but now we could do easily
(I think).  That would move us from having four spare permission bits
to having 20.  I don't think it'd cause an on-disk compatibility break
because type aclitem is generally only stored in the catalogs anyway.

regards, tom lane

Re: [BUGFIX] amcanbackward is not checked before building backward index paths

[Tom Lane]

Nikita Glukhov  writes:
> Experimenting with the new pluggable storage API, I found that amcanbackward
> flag is not checked in build_index_paths() before
> build_index_pathkeys(... BackwardScanDirection) call when we are building
> paths for ORDER BY.  And this flag is even not copied into IndexOptInfo 
> struct.
> Obviously, this can lead to misuse of Backward Index [Only] Scan plans.

> Attached patch with the corresponding fix.

I think this patch is based on a misunderstanding of what amcanbackward
means.  We *require* indexes that claim to support amcanorder to support
scanning in either direction.  What amcanbackward is about is whether
the index can support reversing direction mid-scan, as would be required
to support FETCH FORWARD followed by FETCH BACKWARD in a cursor.  That's
actually independent of whether the index can implement a defined
ordering; see for example the hash AM, which sets amcanbackward but not
amcanorder.

This is documented, though perhaps not sufficiently clearly, in
indexam.sgml:

The amgettuple function has a direction argument, which can be either
ForwardScanDirection (the normal case) or BackwardScanDirection. If
the first call after amrescan specifies BackwardScanDirection, then
the set of matching index entries is to be scanned back-to-front
rather than in the normal front-to-back direction, so amgettuple must
return the last matching tuple in the index, rather than the first one
as it normally would. (This will only occur for access methods that
set amcanorder to true.) After the first call, amgettuple must be
prepared to advance the scan in either direction from the most
recently returned entry. (But if amcanbackward is false, all
subsequent calls will have the same direction as the first one.)

Perhaps there is a case for adding an additional flag to allow specifying
"I can't support ORDER BY DESC", but I'm not in a big hurry to do so.
I think there would be more changes than this needed to handle such a
restriction, anyway.

regards, tom lane

Re: Should total_pages be calculated after partition pruning and constraint exclusion?

[David Rowley]

On 16 May 2018 at 08:10, Tom Lane  wrote:
> David Rowley  writes:
>> The attached patch implements the change.
>
> Please add to next CF.  It's a bit late to be doing such things in v11,
> IMO.

If I do that, it'll go under bug fix. It seems strange to delay fixing
this until v12. We've still got 1 week before beta.

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

Re: Should total_pages be calculated after partition pruning and constraint exclusion?

[Tom Lane]

David Rowley  writes:
> On 16 May 2018 at 08:10, Tom Lane  wrote:
>> David Rowley  writes:
>> Please add to next CF.  It's a bit late to be doing such things in v11,
>> IMO.

> If I do that, it'll go under bug fix.

No, it should go under "planner improvement".  If this were a bug fix,
it'd be a candidate for back-patch, which IMO it's not --- if only
because of the risk of plan destabilization.  But for a more direct
analogy, if you'd submitted some other improvement in selectivity
or cost estimation, that was not a bug fix in the sense of "corrects
outright wrong answers", would you expect it to escape feature freeze
at this point?

Beta or no beta, there has to be a pretty good argument why changes
should go into v11 and not v12 once we're past feature freeze.

regards, tom lane

Re: Postgres 11 release notes

[Bruce Momjian]

fOn Wed, May 16, 2018 at 06:11:52AM +0900, Tatsuo Ishii wrote:
> >> This commit gives user-visible changes to the statment timeout
> >> behavior. So I think this should be added to the release notes.
> > 
> > OK, makes sense. Here is what I added:
> > 
> > 2017-09-18 [f8e5f156b] Rearm statement_timeout after each executed 
> > query.
> > 
> > In the Extended Query
> > Protocol, have statement_timeout apply
> > to each Execute message, not to all commands
> > before Sync (Tatsuo Ishii)
> 
> Can you please add Andres Freund to the author? He made extensive
> changes to the original patch to improve it.

Done.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Should total_pages be calculated after partition pruning and constraint exclusion?

[David Rowley]

On 16 May 2018 at 11:04, Tom Lane  wrote:
> David Rowley  writes:
>> On 16 May 2018 at 08:10, Tom Lane  wrote:
>>> David Rowley  writes:
>>> Please add to next CF.  It's a bit late to be doing such things in v11,
>>> IMO.
>
>> If I do that, it'll go under bug fix.
>
> No, it should go under "planner improvement".  If this were a bug fix,
> it'd be a candidate for back-patch, which IMO it's not --- if only
> because of the risk of plan destabilization.  But for a more direct
> analogy, if you'd submitted some other improvement in selectivity
> or cost estimation, that was not a bug fix in the sense of "corrects
> outright wrong answers", would you expect it to escape feature freeze
> at this point?

I'm not going to make a fuss over it, but I guess we must differ in
opinion as I would count tracking relation sizes of relations we're
actually not going to scan as a bug. I wouldn't have pushed a
backpatch due to possible plan destabilisation. People may have pushed
effective_cache_size beyond their machines RAM size to work around it.

I'll add to the commitfest before it opens., if it's going in as
"planner improvement" I'll probably hold off until I'm ready with the
other improvements that I'm working on.

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

Re: Postgres 11 release notes

[Tatsuo Ishii]

>> Can you please add Andres Freund to the author? He made extensive
>> changes to the original patch to improve it.
> 
> Done.

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

Re: Postgres 11 release notes

[Tatsuo Ishii]

> Change the ps process display labels to match the
> pg_stat_activity.backend_type labels (Peter Eisentraut)

pg_stat_activity.backend_type label for postgres process dedicated to
user sessions is "client backend" while ps still shows something like
"postgres: t-ishii postgres [local] idle". Do we want to mention this?

For example "Change the ps process display labels to match the
pg_stat_activity.backend_type labels except client backend (Peter
Eisentraut)"

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

Re: Postgres 11 release notes

[Bruce Momjian]

On Wed, May 16, 2018 at 08:33:53AM +0900, Tatsuo Ishii wrote:
> > Change the ps process display labels to match the
> > pg_stat_activity.backend_type labels (Peter Eisentraut)
> 
> pg_stat_activity.backend_type label for postgres process dedicated to
> user sessions is "client backend" while ps still shows something like
> "postgres: t-ishii postgres [local] idle". Do we want to mention this?
> 
> For example "Change the ps process display labels to match the
> pg_stat_activity.backend_type labels except client backend (Peter
> Eisentraut)"

Good point.  Should we mention background workers instead?

Change the ps process display labels for background workers
to match the pg_stat_activity.backend_type labels Eisentraut)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Postgres 11 release notes

[Tatsuo Ishii]

>> pg_stat_activity.backend_type label for postgres process dedicated to
>> user sessions is "client backend" while ps still shows something like
>> "postgres: t-ishii postgres [local] idle". Do we want to mention this?
>> 
>> For example "Change the ps process display labels to match the
>> pg_stat_activity.backend_type labels except client backend (Peter
>> Eisentraut)"
> 
> Good point.  Should we mention background workers instead?
> 
>   Change the ps process display labels for background workers
>   to match the pg_stat_activity.backend_type labels Eisentraut)

Seems good to me.

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

Re: Postgres 11 release notes

[Bruce Momjian]

On Wed, May 16, 2018 at 08:52:05AM +0900, Tatsuo Ishii wrote:
> >> pg_stat_activity.backend_type label for postgres process dedicated to
> >> user sessions is "client backend" while ps still shows something like
> >> "postgres: t-ishii postgres [local] idle". Do we want to mention this?
> >> 
> >> For example "Change the ps process display labels to match the
> >> pg_stat_activity.backend_type labels except client backend (Peter
> >> Eisentraut)"
> > 
> > Good point.  Should we mention background workers instead?
> > 
> > Change the ps process display labels for background workers
> > to match the pg_stat_activity.backend_type labels Eisentraut)
> 
> Seems good to me.

Done.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: Postgres 11 release notes

[Tatsuo Ishii]

There's a small typo.

> Add support for with huge(large) pages on Windows (Takayuki Tsunakawa, Thomas 
> Munro) 

I think a space between "huge" and "(large)" is needed.

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

Re: Postgres 11 release notes

[Bruce Momjian]

On Wed, May 16, 2018 at 09:01:35AM +0900, Tatsuo Ishii wrote:
> There's a small typo.
> 
> > Add support for with huge(large) pages on Windows (Takayuki Tsunakawa, 
> > Thomas Munro) 
> 
> I think a space between "huge" and "(large)" is needed.

Done, URL updated.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+  Ancient Roman grave inscription +

Re: [HACKERS] Planning counters in pg_stat_statements

[Thomas Munro]

Hi Dmitry,

On Tue, May 15, 2018 at 11:10 PM, Dmitry Ivanov  wrote:
> Is anybody still working on this? Are there any plans to add this to
> commitfest?

I am not actively working on this now, but I'll come back to it for
PG12 if you or Lukas don't beat me to it, and I'll help/test/review if
I you do.  It seems there is plenty of demand for the feature and I'll
be very happy to see it.

> I'd like to add planning time to auto_explain, and it turns out that this
> patch is somewhat relevant to that feature.

Sounds very useful.

> The current approach here is to set planning_time in PlannedStmt via
> planner_hook, which (in my opinion) has several flaws:
>
> 1. Depending on the order of extensions in shared_preload_libraries, it
> might not measure time spent on preceding planner hooks.
>
> 2. Provided that there are multiple users of this metric, it might become a
> little too costy to register several hooks with identical purpose.
>
> 3. [Bikeshedding] Although planning time is stored in PlannedStmt, it's
> definitely not an inherent property of a plan. You could have two machines
> with identical settings but quite different planning times due to various
> circumstances (raw CPU power, I/O etc).

Yeah.  Putting that inside the PlannedStmt wasn't very nice.  Another
thing I tried was to have some opaque extension workspace, but that
runs into a number of technical problem.  That idea is definitely
dead.

> I'd argue that it might be better to add a new argument to pg_plan_query()
> and pg_plan_queries() and a new field to QueryDesc, i.e.:
>
> PlannedStmt *
> pg_plan_query(Query *querytree,
>   int cursorOptions,
>   ParamListInfo boundParams,
>   double *planningTime)
>
> List *
> pg_plan_queries(List *querytrees,
> int cursorOptions,
> ParamListInfo boundParams,
> double *planningTime) /* total time as in
> BuildCachedPlan() */
>
> The measured time can later be passed to QueryDesc via PortalDefineQuery().
> Of course, this requires more changes, but the result might be worth it.
>
> What do you think?

So who does the actual timing work in this model?  Does the core code
do the timing, but it'd be disabled by default because NULL is usually
passed in, and you need to register an extension that provides a place
to stick the result in order to turn on the time-measuring code?  If
you mean that it's still the extension that does the timing, it seems
strange to have struct members and parameter arguments for something
specific that the core code doesn't understand.

As a more general version of that, I wondered about having some kind
of associative data structure (hash table, assoc list, etc) that would
somehow travel everywhere with the PlannedStmt, but not inside it,
just for the use of extensions.  Then planning time could be stored in
there by a planner hook, and the fished out by any other hook that
knows the same key (not sure how you manage that key space but I'm
sure you could come up with something).  That could have uses for
other extensions too, and could also be used for the "query ID", which
is, after all, the model that the abandoned planning_time member was
following.  Just a thought.

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

Re: parallel foreign scan

[Kyotaro HORIGUCHI]

At Tue, 15 May 2018 23:09:31 +0200, Manuel Kniep  wrote in 

> Dear hackers,
> 
> I’m working on a foreign database wrapper for Kafka [1]
> Now I am trying to make it parallel aware. Following 
> the documentation [2]
> However it seems that I can’t make it use more than a
> single worker with force_parallel_mode = on.
> 
> I wonder if I need to do more than just implementing the
> needed callback function to benefit from multiple workers.
> 
> Looking at create_foreignscan_path in path_nodes.c
> I found that the ForeignPath seems to always set
> 
> pathnode->path.parallel_aware = false;
> pathnode->path.parallel_safe = rel->consider_parallel;
> pathnode->path.parallel_workers = 0;
> 
> Do I need so set these in my GetForeignPaths callback manually?

Right. create_foreignscan_path is used by FDW drivers to create
the path struct. GetForeignPaths() needs to finish the path by
setting the parameters and partial paths.

# I myself haven't do that so I'm not sure the details.

> Is there anything else I need to do?

I think you are trying collecting data from multple kafka
server. This means each server has a dedicate foreign table on a
dedicate foreign server. Parallel execution doesn't fit in that
case since it works on single base relation (or a
table). Parallel append/merge append look a bit different but
actually is the same in the sense that one base relation is
scanned on multiple workers. Even if you are trying to fetch from
one kafka stream on multiple workers, I think the fdw driver
doesn't support parallel scanning anyway.

In any case it is inevitable to modify the fdw driver.

If you are trying to collect data from multple servers, the
following proposed PoC patch is a implement of asynchronous
execution of postgres_fdw and it might be helpful.

https://www.postgresql.org/message-id/20180515.202945.69332784.horiguchi.kyot...@lab.ntt.co.jp

The postgres_fdw.c part in it is complicated since it supports
shared connection but not that complex ignoring that.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: Cache lookup errors with functions manipulation object addresses

[Michael Paquier]

On Mon, May 14, 2018 at 06:32:52PM -0400, Alvaro Herrera wrote:
> So, I don't know -- if the reaction is to add a #ifdef for pg version
> that adds a second argument passed always false, then we haven't won
> anything.

Yeah, that improves nothing..

>> What about naming those GetForeignServerExtended and
>> GetForeignDataWrapperExtended?
> 
> WFM.

Okay, I have done so in the updated set attached.  I have added some
documentation as well in fdwhandler.sgml about those two new things.
That's too late for v11 of course, so let's them sit until the time
comes.
--
Michael
From be6f8c7b986ea283e1f346b18b1c3116d1956772 Mon Sep 17 00:00:00 2001
From: Michael Paquier 
Date: Tue, 13 Feb 2018 11:12:44 +0900
Subject: [PATCH 1/3] Extend lookup routines for FDW and foreign server with
 NULL handling

The cache lookup routines for foreign-data wrappers and foreign servers
are extended with an extra argument able to control if an error or a
NULL object is returned to the caller in the event of an undefined
object. This is added in a set of new routines to not impact
unnecessrily any FPW plugins.
---
 doc/src/sgml/fdwhandler.sgml  | 30 +
 src/backend/foreign/foreign.c | 36 +--
 src/include/foreign/foreign.h |  4 
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 7b758bdf09..bfc538f249 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -1404,6 +1404,21 @@ ReparameterizeForeignPathByChild(PlannerInfo *root, List *fdw_private,
 
 
 ForeignDataWrapper *
+GetForeignDataWrapperExtended(Oid fdwid, bool missing_ok);
+
+
+ This function returns a ForeignDataWrapper
+ object for the foreign-data wrapper with the given OID.  A
+ ForeignDataWrapper object contains properties
+ of the FDW (see foreign/foreign.h for details).
+ If missing_ok is true, a NULL
+ result is returned to the caller instead of an error for an undefined
+ FDW.
+
+
+
+
+ForeignDataWrapper *
 GetForeignDataWrapper(Oid fdwid);
 
 
@@ -1416,6 +1431,21 @@ GetForeignDataWrapper(Oid fdwid);
 
 
 ForeignServer *
+GetForeignServerExtended(Oid serverid, bool missing_ok);
+
+
+ This function returns a ForeignServer object
+ for the foreign server with the given OID.  A
+ ForeignServer object contains properties
+ of the server (see foreign/foreign.h for details).
+ If missing_ok is true, a NULL
+ result is returned to the caller instead of an error for an undefined
+ foreign server.
+
+
+
+
+ForeignServer *
 GetForeignServer(Oid serverid);
 
 
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index eac78a5d31..01b5175e71 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -33,6 +33,18 @@
  */
 ForeignDataWrapper *
 GetForeignDataWrapper(Oid fdwid)
+{
+	return GetForeignDataWrapperExtended(fdwid, false);
+}
+
+
+/*
+ * GetForeignDataWrapperExtended -	look up the foreign-data wrapper
+ * by OID. If missing_ok is true, return NULL if the object cannot be
+ * found instead of raising an error.
+ */
+ForeignDataWrapper *
+GetForeignDataWrapperExtended(Oid fdwid, bool missing_ok)
 {
 	Form_pg_foreign_data_wrapper fdwform;
 	ForeignDataWrapper *fdw;
@@ -43,7 +55,11 @@ GetForeignDataWrapper(Oid fdwid)
 	tp = SearchSysCache1(FOREIGNDATAWRAPPEROID, ObjectIdGetDatum(fdwid));
 
 	if (!HeapTupleIsValid(tp))
-		elog(ERROR, "cache lookup failed for foreign-data wrapper %u", fdwid);
+	{
+		if (!missing_ok)
+			elog(ERROR, "cache lookup failed for foreign-data wrapper %u", fdwid);
+		return NULL;
+	}
 
 	fdwform = (Form_pg_foreign_data_wrapper) GETSTRUCT(tp);
 
@@ -91,6 +107,18 @@ GetForeignDataWrapperByName(const char *fdwname, bool missing_ok)
  */
 ForeignServer *
 GetForeignServer(Oid serverid)
+{
+	return GetForeignServerExtended(serverid, false);
+}
+
+
+/*
+ * GetForeignServerExtended - look up the foreign server definition. If
+ * missing_ok is true, return NULL if the object cannot be found instead
+ * of raising an error.
+ */
+ForeignServer *
+GetForeignServerExtended(Oid serverid, bool missing_ok)
 {
 	Form_pg_foreign_server serverform;
 	ForeignServer *server;
@@ -101,7 +129,11 @@ GetForeignServer(Oid serverid)
 	tp = SearchSysCache1(FOREIGNSERVEROID, ObjectIdGetDatum(serverid));
 
 	if (!HeapTupleIsValid(tp))
-		elog(ERROR, "cache lookup failed for foreign server %u", serverid);
+	{
+		if (!missing_ok)
+			elog(ERROR, "cache lookup failed for foreign server %u", serverid);
+		return NULL;
+	}
 
 	serverform = (Form_pg_foreign_server) GETSTRUCT(tp);
 
diff --git a/src/include/foreign/foreign.h b/src/include/foreign/foreign.h
index 3ca12e64d2..5cc89e967c 100644
--- a/src/include/foreign/foreign.h
+++ b/src/include/foreign/foreign.h
@@ -70,9 +70,13 @@ typedef struct ForeignTable
 
 
 extern ForeignServer *GetForeignServer(Oid serverid);
+extern ForeignServer *GetForeignServ

Re: Make description of heap records more talkative for flags

[Michael Paquier]

On Tue, May 15, 2018 at 02:32:55PM -0700, Andres Freund wrote:
> I have an *extremely* hard time that'd ever be the case. And it'd not
> really change much, given several years of back branch releases.
> Anyway, I'm only -0.1 here.

I personally don't care much if this gets in v11 or later in v12 as
generally bug fixes are not branch-specific and can be found on master.
For now, I am just letting this version of the patch sit in the CF:
https://www.postgresql.org/message-id/20180423234928.GA1570%40paquier.xyz
https://commitfest.postgresql.org/18/1633/
--
Michael


signature.asc
Description: PGP signature

Re: Postgres 11 release notes

[Michael Paquier]

On Mon, May 14, 2018 at 08:45:44PM -0400, Bruce Momjian wrote:
> What TLS does is to mix the offered ciphers into the negotiation hash so
> a man-in-the-middle can't pretend it doesn't support something.  Could
> we do something like that here?

I have to admit that I don't quite follow here, the shape of channel
binding data is decided by RFC 5929, so we need to stick with it.

> I have to question the value of man-in-the-middle protection that is so
> easily bypassed.

Well, the backend does its job, and answers based on what the client
wants to do.  But what you are questioning here is the handling of
authentication downgrade attempts from a server by libpq, which is a
different problem, larger than just channel binding as it relates as
well to MD5/SCRAM interactions.  For example, it is perfectly possible
to implement downgrade protections for any drivers which speak the
protocol, like JDBC, even with a v11 backend.
--
Michael


signature.asc
Description: PGP signature

Re: Postgres 11 release notes

[Amit Kapila]

On Wed, May 16, 2018 at 12:17 AM, Bruce Momjian  wrote:
> On Tue, May 15, 2018 at 08:45:07AM +0530, Amit Kapila wrote:
>> No, it is not like that.  We divide the scan among workers and each
>> worker should perform projection of the rows it scanned (after
>> applying filter).  Now, if the expensive functions are part of target
>> lists, then we can push the computation of expensive functions (as
>> part of target list) in workers which will divide the work.
>>
>> >  Really?  Do
>> > we run each column in its own worker or do we split the result set into
>> > parts and run those in parallel?  How do we know, just the function call
>> > costs?
>> >
>>
>> The function's cost can be determined via pg_proc->procost.  For this
>> particular case, you can refer the call graph -
>> create_pathtarget->set_pathtarget_cost_width->cost_qual_eval_node->cost_qual_eval_walker->get_func_cost
>>
>> >  I can admit I never saw that coming.
>> >
>>
>> I think the use case becomes interesting with parallel query because
>> now you can divide such cost among workers.
>>
>> Feel free to ask more questions if above doesn't clarify the usage of
>> these features.
>
> OK, I have added the following release note item for both of these:
>
> 2017-11-16 [e89a71fb4] Pass InitPlan values to workers via Gather (Merge).
> 2018-03-29 [3f90ec859] Postpone generate_gather_paths for topmost scan/join 
> rel
> 2018-03-29 [11cf92f6e] Rewrite the code that applies scan/join targets to 
> paths
>
> Allow single-evaluation queries, e.g. FROM
> clause queries, and functions in the target list to be
> parallelized (Amit Kapila, Robert Haas)
>

Sorry, but it is not clear to me what you intend to say by "e.g.
FROM clause queries"?   What I could imagine is
something like "e.g. queries in WHERE clause that
return aggregate value"

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