[COMMITTERS] pgsql: Pass the source text for a parallel query to the workers.

2017-02-21 Thread Robert Haas 
Pass the source text for a parallel query to the workers.

With this change, you can see the query that a parallel worker is
executing in pg_stat_activity, and if the worker crashes you can
see what query it was executing when it crashed.

Rafia Sabih, reviewed by Kuntal Ghosh and Amit Kapila and slightly
revised by me.

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/4c728f382970b6346142fe4409212063ee3e92dc

Modified Files
--
src/backend/executor/execMain.c |  2 ++
src/backend/executor/execParallel.c | 26 +-
src/backend/executor/execUtils.c|  1 +
src/include/nodes/execnodes.h   |  1 +
4 files changed, 29 insertions(+), 1 deletion(-)


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

[COMMITTERS] pgsql: Fix incorrect typecast.

2017-02-21 Thread Robert Haas 
Fix incorrect typecast.

Ashutosh Sharma, per a report from Mithun Cy.

Discussion: 
http://postgr.es/m/CAD__OujgqNNnCujeFTmKpjNu+W4smS8Hbi=rcwahf1zus3h...@mail.gmail.com

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/b4316928d57bec22e95875e6487a4d665bd03a52

Modified Files
--
contrib/pageinspect/hashfuncs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

Re: [COMMITTERS] pgsql: Fix connection leak in DROP SUBSCRIPTION command.

2017-02-21 Thread Michael Paquier 
On Wed, Feb 22, 2017 at 8:39 AM, Fujii Masao  wrote:
> On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier
>  wrote:
>> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane  wrote:
>>> Fujii Masao  writes:
 Fix connection leak in DROP SUBSCRIPTION command.
 Previously the command forgot to close the connection to the publisher
 when it failed to drop the replication slot.
>>>
>>> If there's a bug here, this seems like an extremely unreliable way of
>>> fixing it.  What if an error gets thrown before you reach that ereport?
>>>
>>> In other words, this coding is assuming that the walrcv_command()
>>> subroutine cannot throw an error,
>
> Yes, but I agree that walrcv_command() may be changed in the future so that
> an error is thrown and current coding is not reliable in that case.
>
>>> which I would consider dangerous
>>> even if it were a fixed subroutine.  If it's a hook that's doing
>>> unknown stuff, that seems a completely untenable assumption.  You
>>> really need either to hook the cleanup action into normal error
>>> recovery, or to use a PG_TRY block.
>>
>> To be honest, I have thought about using PG_ENSURE_ERROR_CLEANUP()
>> when seeing the thread. If other ERROR messages are generated in the
>> future that the current fix would be unreliable.
>
> What about the attached patch?

Thanks for the patch. That looks good to me.
-- 
Michael


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

[COMMITTERS] pgsql: Shut down Gather's children before shutting down Gather itself.

2017-02-21 Thread Robert Haas 
Shut down Gather's children before shutting down Gather itself.

It turns out that the original shutdown order here does not work well.
Multiple people attempting to develop further parallel query patches
have discovered that they need to do cleanup before the DSM goes away,
and you can't do that if the parent node gets cleaned up first.

Patch by me, reviewed by KaiGai Kohei and Dilip Kumar.

Discussion: 
http://postgr.es/m/ca+tgmoy6boc1ynhcaqnmfcbdbsjzroq3syxsal-syb5tmjc...@mail.gmail.com
Discussion: 
http://postgr.es/m/9a28c8860f777e439aa12e8aea7694f8012ae...@bpxm15gp.gisp.nec.co.jp
Discussion: 
http://postgr.es/m/CA+TgmoYuPOc=+xrg1v0fcsolbkaab9f1ddoeaailmzkoiba...@mail.gmail.com

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/acf555bc53acb589b5a2827e65d655fa8c9adee0

Modified Files
--
src/backend/executor/execProcnode.c | 4 +++-
src/backend/executor/nodeGather.c   | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)


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

[COMMITTERS] pgsql: doc: Add missing comma.

2017-02-21 Thread Robert Haas 
doc: Add missing comma.

Yugo Nagata

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/d912dd062b64287adcabab4180abafefd07cea14

Modified Files
--
doc/src/sgml/ddl.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

Re: [COMMITTERS] pgsql: Fix connection leak in DROP SUBSCRIPTION command.

2017-02-21 Thread Petr Jelinek 
On 22/02/17 00:39, Fujii Masao wrote:
> On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier
>  wrote:
>> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane  wrote:
>>> Fujii Masao  writes:
 Fix connection leak in DROP SUBSCRIPTION command.
 Previously the command forgot to close the connection to the publisher
 when it failed to drop the replication slot.
>>>
>>> If there's a bug here, this seems like an extremely unreliable way of
>>> fixing it.  What if an error gets thrown before you reach that ereport?
>>>
>>> In other words, this coding is assuming that the walrcv_command()
>>> subroutine cannot throw an error,
> 
> Yes, but I agree that walrcv_command() may be changed in the future so that
> an error is thrown and current coding is not reliable in that case.
> 
>>> which I would consider dangerous
>>> even if it were a fixed subroutine.  If it's a hook that's doing
>>> unknown stuff, that seems a completely untenable assumption.  You
>>> really need either to hook the cleanup action into normal error
>>> recovery, or to use a PG_TRY block.
>>
>> To be honest, I have thought about using PG_ENSURE_ERROR_CLEANUP()
>> when seeing the thread. If other ERROR messages are generated in the
>> future that the current fix would be unreliable.
> 
> What about the attached patch?
> 

Looks more or less like what we do in CREATE SUBSCRIPTION for this, so I
guess it's okay.

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


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

Re: [COMMITTERS] pgsql: Fix connection leak in DROP SUBSCRIPTION command.

2017-02-21 Thread Fujii Masao 
On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier
 wrote:
> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane  wrote:
>> Fujii Masao  writes:
>>> Fix connection leak in DROP SUBSCRIPTION command.
>>> Previously the command forgot to close the connection to the publisher
>>> when it failed to drop the replication slot.
>>
>> If there's a bug here, this seems like an extremely unreliable way of
>> fixing it.  What if an error gets thrown before you reach that ereport?
>>
>> In other words, this coding is assuming that the walrcv_command()
>> subroutine cannot throw an error,

Yes, but I agree that walrcv_command() may be changed in the future so that
an error is thrown and current coding is not reliable in that case.

>> which I would consider dangerous
>> even if it were a fixed subroutine.  If it's a hook that's doing
>> unknown stuff, that seems a completely untenable assumption.  You
>> really need either to hook the cleanup action into normal error
>> recovery, or to use a PG_TRY block.
>
> To be honest, I have thought about using PG_ENSURE_ERROR_CLEANUP()
> when seeing the thread. If other ERROR messages are generated in the
> future that the current fix would be unreliable.

What about the attached patch?

Regards,

-- 
Fujii Masao


bugfix.patch
Description: Binary data

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

Re: [COMMITTERS] pgsql: Make walsender always initialize the buffers.

2017-02-21 Thread Fujii Masao 
On Wed, Feb 22, 2017 at 4:04 AM, Tom Lane  wrote:
> Fujii Masao  writes:
>> Make walsender always initialize the buffers.
>> ...
>> Back-patch to 9.4 where replication slot was introduced.
>
> Doesn't look like you actually pushed the back-patch?  I only see
> this in HEAD.

Oh, sorry... I pushed the back-patch.

Regards,

-- 
Fujii Masao


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

[COMMITTERS] pgsql: Make walsender always initialize the buffers.

2017-02-21 Thread Fujii Masao 
Make walsender always initialize the buffers.

Walsender uses the local buffers for each outgoing and incoming message.
Previously when creating replication slot, walsender forgot to initialize
one of them and which can cause the segmentation fault error. To fix this
issue, this commit changes walsender so that it always initialize them
before it executes the requested replication command.

Back-patch to 9.4 where replication slot was introduced.

Problem report and initial patch by Stas Kelvich, modified by me.
Report: 
https://www.postgresql.org/message-id/a1e9cb90-1fac-4cad-8dba-9aa62a6e9...@postgrespro.ru

Branch
--
REL9_6_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/9fab155c68a4e2164c39f61bc849ad669ad54f2c

Modified Files
--
src/backend/replication/walsender.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)


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

[COMMITTERS] pgsql: Make walsender always initialize the buffers.

2017-02-21 Thread Fujii Masao 
Make walsender always initialize the buffers.

Walsender uses the local buffers for each outgoing and incoming message.
Previously when creating replication slot, walsender forgot to initialize
one of them and which can cause the segmentation fault error. To fix this
issue, this commit changes walsender so that it always initialize them
before it executes the requested replication command.

Back-patch to 9.4 where replication slot was introduced.

Problem report and initial patch by Stas Kelvich, modified by me.
Report: 
https://www.postgresql.org/message-id/a1e9cb90-1fac-4cad-8dba-9aa62a6e9...@postgrespro.ru

Branch
--
REL9_5_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/feb659cced9755966190bbabfcead54fcfcddf0e

Modified Files
--
src/backend/replication/walsender.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)


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

[COMMITTERS] pgsql: Make walsender always initialize the buffers.

2017-02-21 Thread Fujii Masao 
Make walsender always initialize the buffers.

Walsender uses the local buffers for each outgoing and incoming message.
Previously when creating replication slot, walsender forgot to initialize
one of them and which can cause the segmentation fault error. To fix this
issue, this commit changes walsender so that it always initialize them
before it executes the requested replication command.

Back-patch to 9.4 where replication slot was introduced.

Problem report and initial patch by Stas Kelvich, modified by me.
Report: 
https://www.postgresql.org/message-id/a1e9cb90-1fac-4cad-8dba-9aa62a6e9...@postgrespro.ru

Branch
--
REL9_4_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/a3eb715a36bdd28d809d3dd859f169489352882b

Modified Files
--
src/backend/replication/walsender.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)


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

[COMMITTERS] pgsql: Suppress unused-variable warning.

2017-02-21 Thread Tom Lane 
Suppress unused-variable warning.

Rearrange so we don't have an unused variable in disable-cassert case.

Discussion: 
https://postgr.es/m/CAMkU=1x63f2qyfteas83xjqd+hm1pbuok1lrzyzs-ongdzy...@mail.gmail.com

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/c56ac2913a1f3adce674a2eb27257d0bca81317f

Modified Files
--
src/backend/optimizer/path/indxpath.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: Fix sloppy handling of corner-case errors in fd.c.

2017-02-21 Thread Tom Lane 
Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

Discussion: https://postgr.es/m/2982.1487617...@sss.pgh.pa.us

Branch
--
REL9_5_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/ff1b032a9c9f407bce711400a8edaa57218a2e81

Modified Files
--
src/backend/storage/file/fd.c | 195 +-
1 file changed, 136 insertions(+), 59 deletions(-)


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

[COMMITTERS] pgsql: Fix sloppy handling of corner-case errors in fd.c.

2017-02-21 Thread Tom Lane 
Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

Discussion: https://postgr.es/m/2982.1487617...@sss.pgh.pa.us

Branch
--
REL9_6_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/62ed08422cba0c1bf4ad1e721c13cd6ac9ed29a0

Modified Files
--
src/backend/storage/file/fd.c | 195 +-
1 file changed, 136 insertions(+), 59 deletions(-)


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

[COMMITTERS] pgsql: Fix sloppy handling of corner-case errors in fd.c.

2017-02-21 Thread Tom Lane 
Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

Discussion: https://postgr.es/m/2982.1487617...@sss.pgh.pa.us

Branch
--
REL9_3_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/3f613c6a4073149bb79af8ec82eab9c0a900fd53

Modified Files
--
src/backend/storage/file/fd.c | 195 +-
1 file changed, 136 insertions(+), 59 deletions(-)


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

[COMMITTERS] pgsql: Fix sloppy handling of corner-case errors in fd.c.

2017-02-21 Thread Tom Lane 
Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

Discussion: https://postgr.es/m/2982.1487617...@sss.pgh.pa.us

Branch
--
REL9_4_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/d9959e6ebb0d42dd385bf06d131468bb221e8b9f

Modified Files
--
src/backend/storage/file/fd.c | 195 +-
1 file changed, 136 insertions(+), 59 deletions(-)


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

[COMMITTERS] pgsql: Fix sloppy handling of corner-case errors in fd.c.

2017-02-21 Thread Tom Lane 
Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

Discussion: https://postgr.es/m/2982.1487617...@sss.pgh.pa.us

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/f97de05a14bbd26cf0252906b44643e8923bdf85

Modified Files
--
src/backend/storage/file/fd.c | 195 +-
1 file changed, 136 insertions(+), 59 deletions(-)


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

[COMMITTERS] pgsql: Fix sloppy handling of corner-case errors in fd.c.

2017-02-21 Thread Tom Lane 
Fix sloppy handling of corner-case errors in fd.c.

Several places in fd.c had badly-thought-through handling of error returns
from lseek() and close().  The fact that those would seldom fail on valid
FDs is probably the reason we've not noticed this up to now; but if they
did fail, we'd get quite confused.

LruDelete and LruInsert actually just Assert'd that lseek never fails,
which is pretty awful on its face.

In LruDelete, we indeed can't throw an error, because that's likely to get
called during error abort and so throwing an error would probably just lead
to an infinite loop.  But by the same token, throwing an error from the
close() right after that was ill-advised, not to mention that it would've
left the LRU state corrupted since we'd already unlinked the VFD from the
list.  I also noticed that really, most of the time, we should know the
current seek position and it shouldn't be necessary to do an lseek here at
all.  As patched, if we don't have a seek position and an lseek attempt
doesn't give us one, we'll close the file but then subsequent re-open
attempts will fail (except in the somewhat-unlikely case that a
FileSeek(SEEK_SET) call comes between and allows us to re-establish a known
target seek position).  This isn't great but it won't result in any state
corruption.

Meanwhile, having an Assert instead of an honest test in LruInsert is
really dangerous: if that lseek failed, a subsequent read or write would
read or write from the start of the file, not where the caller expected,
leading to data corruption.

In both LruDelete and FileClose, if close() fails, just LOG that and mark
the VFD closed anyway.  Possibly leaking an FD is preferable to getting
into an infinite loop or corrupting the VFD list.  Besides, as far as I can
tell from the POSIX spec, it's unspecified whether or not the file has been
closed, so treating it as still open could be the wrong thing anyhow.

I also fixed a number of other places that were being sloppy about
behaving correctly when the seekPos is unknown.

Also, I changed FileSeek to return -1 with EINVAL for the cases where it
detects a bad offset, rather than throwing a hard elog(ERROR).  It seemed
pretty inconsistent that some bad-offset cases would get a failure return
while others got elog(ERROR).  It was missing an offset validity check for
the SEEK_CUR case on a closed file, too.

Back-patch to all supported branches, since all this code is fundamentally
identical in all of them.

Discussion: https://postgr.es/m/2982.1487617...@sss.pgh.pa.us

Branch
--
REL9_2_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/775227590d40205a74042c01b5e6947ab47bccc7

Modified Files
--
src/backend/storage/file/fd.c | 195 +-
1 file changed, 136 insertions(+), 59 deletions(-)


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

[COMMITTERS] pgsql: Add tests for two-phase commit

2017-02-21 Thread Alvaro Herrera 
Add tests for two-phase commit

There's some ongoing performance work on this area, so let's make sure
we don't break things.

Extracted from a larger patch originally by Stas Kelvich.

Authors: Stas Kelvich, Nikhil Sontakke, Michael Paquier
Discussion: 
https://postgr.es/m/CAMGcDxfsuLLOg=h5cTg3g77Jjk-UGnt=rw7zk57zbsofsap...@mail.gmail.com

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/30820982b295404ed00ddd28c8864211dc986dd3

Modified Files
--
src/test/recovery/t/009_twophase.pl | 322 
1 file changed, 322 insertions(+)


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

Re: [COMMITTERS] pgsql: Fix connection leak in DROP SUBSCRIPTION command.

2017-02-21 Thread Michael Paquier 
On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane  wrote:
> Fujii Masao  writes:
>> Fix connection leak in DROP SUBSCRIPTION command.
>> Previously the command forgot to close the connection to the publisher
>> when it failed to drop the replication slot.
>
> If there's a bug here, this seems like an extremely unreliable way of
> fixing it.  What if an error gets thrown before you reach that ereport?
>
> In other words, this coding is assuming that the walrcv_command()
> subroutine cannot throw an error, which I would consider dangerous
> even if it were a fixed subroutine.  If it's a hook that's doing
> unknown stuff, that seems a completely untenable assumption.  You
> really need either to hook the cleanup action into normal error
> recovery, or to use a PG_TRY block.

To be honest, I have thought about using PG_ENSURE_ERROR_CLEANUP()
when seeing the thread. If other ERROR messages are generated in the
future that the current fix would be unreliable.
-- 
Michael


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

[COMMITTERS] pgsql: Fix whitespace

2017-02-21 Thread Peter Eisentraut 
Fix whitespace

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/74321d87fb1a27746c9dd0853b2da3287a0940d9

Modified Files
--
src/backend/commands/subscriptioncmds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

Re: [COMMITTERS] pgsql: Fix connection leak in DROP SUBSCRIPTION command.

2017-02-21 Thread Tom Lane 
Fujii Masao  writes:
> Fix connection leak in DROP SUBSCRIPTION command.
> Previously the command forgot to close the connection to the publisher
> when it failed to drop the replication slot.

If there's a bug here, this seems like an extremely unreliable way of
fixing it.  What if an error gets thrown before you reach that ereport?

In other words, this coding is assuming that the walrcv_command()
subroutine cannot throw an error, which I would consider dangerous
even if it were a fixed subroutine.  If it's a hook that's doing
unknown stuff, that seems a completely untenable assumption.  You
really need either to hook the cleanup action into normal error
recovery, or to use a PG_TRY block.

regards, tom lane


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

Re: [COMMITTERS] pgsql: Make walsender always initialize the buffers.

2017-02-21 Thread Tom Lane 
Fujii Masao  writes:
> Make walsender always initialize the buffers.
> ...
> Back-patch to 9.4 where replication slot was introduced.

Doesn't look like you actually pushed the back-patch?  I only see
this in HEAD.

regards, tom lane


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

[COMMITTERS] pgsql: Fix typo in comment.

2017-02-21 Thread Fujii Masao 
Fix typo in comment.

neha khatri

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/e14ec7d346f8686c9471c16a01579d6b1c3b4975

Modified Files
--
src/backend/utils/adt/varlena.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: Fix connection leak in DROP SUBSCRIPTION command.

2017-02-21 Thread Fujii Masao 
Fix connection leak in DROP SUBSCRIPTION command.

Previously the command forgot to close the connection to the publisher
when it failed to drop the replication slot.

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/898a792eb8283e31efc0b6fcbc03bbcd5f7df667

Modified Files
--
src/backend/commands/subscriptioncmds.c | 4 
1 file changed, 4 insertions(+)


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

[COMMITTERS] pgsql: Make walsender always initialize the buffers.

2017-02-21 Thread Fujii Masao 
Make walsender always initialize the buffers.

Walsender uses the local buffers for each outgoing and incoming message.
Previously when creating replication slot, walsender forgot to initialize
one of them and which can cause the segmentation fault error. To fix this
issue, this commit changes walsender so that it always initialize them
before it executes the requested replication command.

Back-patch to 9.4 where replication slot was introduced.

Problem report and initial patch by Stas Kelvich, modified by me.
Report: 
https://www.postgresql.org/message-id/a1e9cb90-1fac-4cad-8dba-9aa62a6e9...@postgrespro.ru

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/1d04a59be31bf004b880226be0e3fe84acff2815

Modified Files
--
src/backend/replication/walsender.c | 18 --
1 file changed, 8 insertions(+), 10 deletions(-)


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

[COMMITTERS] pgsql: Remove confusing comment about unsupported feature.

2017-02-21 Thread Fujii Masao 
Remove confusing comment about unsupported feature.

The initial table synchronization feature has not been supported yet,
but there was the confusing header comment about it in logical/worker.c.

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/d36537008a8d53853d2fda49913cb54fa6e28f94

Modified Files
--
src/backend/replication/logical/worker.c | 3 ---
1 file changed, 3 deletions(-)


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

[COMMITTERS] pgsql: doc: Update URL for plr

2017-02-21 Thread Peter Eisentraut 
doc: Update URL for plr

Branch
--
REL9_6_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/efc286643f33ce2949e5210338cb5cca0426eb04

Modified Files
--
doc/src/sgml/external-projects.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: doc: Update URL for plr

2017-02-21 Thread Peter Eisentraut 
doc: Update URL for plr

Branch
--
REL9_2_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/b9f05be3855e8573d44e16af39682705cc33b9f5

Modified Files
--
doc/src/sgml/external-projects.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: doc: Update URL for plr

2017-02-21 Thread Peter Eisentraut 
doc: Update URL for plr

Branch
--
REL9_5_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/931182fe3a2c7725238d4351f42c5318b4143bee

Modified Files
--
doc/src/sgml/external-projects.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: doc: Update URL for plr

2017-02-21 Thread Peter Eisentraut 
doc: Update URL for plr

Branch
--
REL9_3_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/6b75aee4b645ae48ba99ed383218d50e46c292ae

Modified Files
--
doc/src/sgml/external-projects.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: doc: Update URL for plr

2017-02-21 Thread Peter Eisentraut 
doc: Update URL for plr

Branch
--
REL9_4_STABLE

Details
---
http://git.postgresql.org/pg/commitdiff/cabfec988191bcaf49c9aefc777ac8b0ba35e6ec

Modified Files
--
doc/src/sgml/external-projects.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: doc: Update URL for plr

2017-02-21 Thread Peter Eisentraut 
doc: Update URL for plr

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/7248099c169b40b8f70cdaf8e12d0deaab9b16e2

Modified Files
--
doc/src/sgml/external-projects.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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

[COMMITTERS] pgsql: Use less-generic table name in new regression test case.

2017-02-21 Thread Tom Lane 
Use less-generic table name in new regression test case.

Creating global objects named "foo" isn't an especially wise thing,
but especially not in a test script that has already used that name
for something else, and most especially not in a script that runs
in parallel with other scripts that use that name :-(

Per buildfarm.

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/1c95f0b478a91b58391720dcda35bc032e582564

Modified Files
--
src/test/regress/expected/alter_table.out | 8 
src/test/regress/sql/alter_table.sql  | 8 
2 files changed, 8 insertions(+), 8 deletions(-)


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

[COMMITTERS] pgsql: Make more use of castNode()

2017-02-21 Thread Peter Eisentraut 
Make more use of castNode()

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/38d103763d14baddf3cbfe4b00b501059fc9447f

Modified Files
--
src/backend/commands/copy.c|  2 +-
src/backend/nodes/nodeFuncs.c  | 24 +--
src/backend/optimizer/path/allpaths.c  |  4 +--
src/backend/optimizer/path/costsize.c  | 20 
src/backend/optimizer/path/indxpath.c  | 10 +++---
src/backend/optimizer/path/joinrels.c  |  3 +-
src/backend/optimizer/plan/analyzejoins.c  |  8 ++---
src/backend/optimizer/plan/createplan.c| 36 +-
src/backend/optimizer/plan/planner.c   |  3 +-
src/backend/optimizer/plan/setrefs.c   |  4 +--
src/backend/optimizer/plan/subselect.c | 17 +-
src/backend/optimizer/prep/prepjointree.c  |  7 ++---
src/backend/optimizer/prep/prepunion.c |  7 ++---
src/backend/optimizer/util/clauses.c   | 10 ++
src/backend/optimizer/util/orclauses.c | 13 
src/backend/optimizer/util/restrictinfo.c  | 12 ++--
src/backend/parser/analyze.c   | 16 --
src/backend/parser/gram.y  |  9 ++
src/backend/parser/parse_clause.c  |  3 +-
src/backend/parser/parse_collate.c | 13 +++-
src/backend/parser/parse_expr.c| 20 
src/backend/parser/parse_func.c|  3 +-
src/backend/parser/parse_node.c|  3 +-
src/backend/parser/parse_relation.c|  3 +-
src/backend/parser/parse_type.c|  3 +-
src/backend/parser/parse_utilcmd.c | 24 +--
src/backend/rewrite/rewriteHandler.c   | 16 +++---
src/backend/utils/adt/datetime.c   |  3 +-
src/backend/utils/adt/numeric.c|  3 +-
src/backend/utils/adt/ruleutils.c  | 21 +
src/backend/utils/adt/selfuncs.c   |  3 +-
src/backend/utils/adt/timestamp.c  |  3 +-
src/backend/utils/adt/varbit.c |  3 +-
src/backend/utils/adt/varchar.c|  3 +-
src/backend/utils/misc/guc.c   |  3 +-
src/pl/plpgsql/src/pl_handler.c|  4 +--
.../modules/test_ddl_deparse/test_ddl_deparse.c|  4 +--
37 files changed, 121 insertions(+), 222 deletions(-)


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

[COMMITTERS] pgsql: Reject too-old Python versions a bit sooner.

2017-02-21 Thread Tom Lane 
Reject too-old Python versions a bit sooner.

Commit 04aad4018 added this check after the search for a Python shared
library, which seems to me to be a pretty unfriendly ordering.  The
search might fail for what are basically version-related reasons, and
in such a case it'd be better to say "your Python is too old" than
"could not find shared library for Python".

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/4e5ce3c1aeadf81b504bc9d683b67950bd3f8766

Modified Files
--
config/python.m4 | 8 +++-
configure| 8 +---
configure.in | 3 ---
3 files changed, 12 insertions(+), 7 deletions(-)


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

[COMMITTERS] pgsql: Drop support for Python 2.3

2017-02-21 Thread Peter Eisentraut 
Drop support for Python 2.3

There is no specific reason for this right now, but keeping support for
old Python versions around indefinitely increases the maintenance
burden.  The oldest supported Python version is now Python 2.4, which is
still shipped in RHEL/CentOS 5 by default.

In configure, add a check for the required Python version and give a
friendly error message for an old version, instead of relying on an
obscure build error later on.

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/04aad401867ad3e1519615d8486e32b50dbcb5f5

Modified Files
--
config/python.m4   |  1 +
configure  |  4 
configure.in   |  3 +++
.../hstore_plpython/expected/hstore_plpython.out   |  8 ++--
contrib/hstore_plpython/sql/hstore_plpython.sql|  8 ++--
doc/src/sgml/installation.sgml |  6 +-
src/pl/plpython/expected/plpython_ereport.out  | 24 +-
src/pl/plpython/plpy_typeio.c  | 10 ++---
src/pl/plpython/sql/plpython_ereport.sql   |  8 ++--
9 files changed, 27 insertions(+), 45 deletions(-)


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

[COMMITTERS] pgsql: Small correction to BRIN docs

2017-02-21 Thread Simon Riggs 
Small correction to BRIN docs

Replace incorrect word "index" with "heap"

Takayuki Tsunakawa

Branch
--
master

Details
---
http://git.postgresql.org/pg/commitdiff/0bf41dd1908a0c05833168b9972e1c52cb7547b7

Modified Files
--
doc/src/sgml/brin.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


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