Re: [COMMITTERS] pgsql: Remove secondary checkpoint

[Michael Paquier]

On Wed, Nov 8, 2017 at 4:43 AM, Tom Lane  wrote:
> Andres Freund  writes:
>> I think you misunderstand my point - I'm saying that pg_resetxlog should
>> be able to force the use of older checkpoints, basically as a fallback
>> to cases where the previous approach might actually have worked, not
>> that it needs to work across format changes.
>
> That seems like a completely separate feature --- and one of dubious
> value, frankly.  The further back you go, the less likely it'd be
> to work.

Because the less guarantees you would have to reach a consistent point
with a non-corrupted instance. I think that Andres' idea here are
worth debating though. Why not just spawning a new thread on the
matter and summarize what you have in mind?
-- 
Michael


-- 
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: Stamp 9.2.24.

[Michael Paquier]

On Tue, Nov 7, 2017 at 7:39 AM, Tom Lane  wrote:
> Peter Geoghegan  writes:
>> Tom Lane  wrote:
>>> Stamp 9.2.24.
>
>> Uh, I thought 9.2 was EOL.
>
> Now it is ...

9.2.24 is the last of the 9.2-series, November being the last minor
release after the 5-year community support window.
-- 
Michael


-- 
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: Replace most usages of ntoh[ls] and hton[sl] with pg_bswap.h.

[Michael Paquier]

On Mon, Oct 2, 2017 at 8:08 AM, Andres Freund  wrote:
> Replace most usages of ntoh[ls] and hton[sl] with pg_bswap.h.
>
> All postgres internal usages are replaced, it's just libpq example
> usages that haven't been converted. External users of libpq can't
> generally rely on including postgres internal headers.
>
> Note that this includes replacing open-coded byte swapping of 64bit
> integers (using two 32 bit swaps) with a single 64bit swap.
>
> Where it looked applicable, I have removed netinet/in.h and
> arpa/inet.h usage, which previously provided the relevant
> functionality. It's perfectly possible that I missed other reasons for
> including those, the buildfarm will tell.

Thanks for taking the time to improve that! I was looking for a 64b
equivalent not long ago for pg_rewind... Those changes look good to me
at quick glance.
-- 
Michael


-- 
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 bogus size calculation introduced by commit cc5f81366.

[Michael Paquier]

On Mon, Sep 18, 2017 at 6:58 AM, Thomas Munro
 wrote:
> While googling around trying to find where I could read Coverity's
> output myself I was intrigued to see that https://scan.coverity.com
> offers integration with Travis CI[1], which suggests the possibility
> of automatically scanning all Commitfest submissions.  The trouble is
> that for projects over 1 million lines of code they limit scans to one
> per day, so it'd take over 200 days to get through the current
> Commitfest, assuming no one ever posted new versions or committed
> anything in the meantime.  Hah.  I guess Coverity analysis is going to
> have to remain post-commit only.

There are a mountain of false positives to take care of when doing the
initial scanning of a new project. So while the initial cost is high,
this would be maintainable in the long term if there is a continuous
effort put into it. The limit due to the project size sucks, but at
least this lets us know that coverity is not a solution for the CF.
Careful review is able to remove most of the problems anyway.
-- 
Michael


-- 
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: pg_receivewal: Improve verbose mode

[Michael Paquier]

On Wed, Aug 16, 2017 at 9:27 AM, Peter Eisentraut  wrote:
> pg_receivewal: Improve verbose mode
>
> Some informational messages showed up even if verbose mode was not
> used.  Move them to verbose mode.

Perhaps this should be back-patched? It seems to me that this is an
oversight from the beginning.
-- 
Michael


-- 
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] Vendor LLVM 4.0.

[Michael Paquier]

On Sat, Apr 1, 2017 at 5:27 PM, Thomas Munro
 wrote:
> On Sat, Apr 1, 2017 at 8:41 PM, Andres Freund  wrote:
>> For the upcoming JIT support LLVM is required.  To avoid issues with
>> having to support multiple LLVM versions, add a vendored version of
>> LLVM.
>>
>> The large size of LLVM makes this not great, but I think it's better
>> than the alternatives.  And I'll forever have the most lines added to
>> postgres.
>>
>> Author: Andres Freund
>> Discussion: 
>> http://postgr.es/m/20161206034955.bh33paeralxbt...@alap3.anarazel.de
>>
>> Branch
>> --
>> master
>>
>> Details
>> ---
>> http://git.postgresql.org/pg/commitdiff/d31084e9d1118b25fd16580d9d8c2924b5740dff
>
> This has broken VAX build farm members "poisson" and "davril".  One
> minor nitpick, I think it needs to rewritten in C so that pgindent can
> handle it.

That may not be the only problem. So this has been committed but it is
not present in the tree?
-- 
Michael


-- 
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: Faster expression evaluation and targetlist projection.

[Michael Paquier]

On Sun, Mar 26, 2017 at 7:58 AM, David Rowley
 wrote:
> On 26 March 2017 at 09:55, Thomas Munro  wrote:
>> On Sun, Mar 26, 2017 at 11:11 AM, Andres Freund  wrote:
>>> Faster expression evaluation and targetlist projection.
>>>
>>> This replaces the old, recursive tree-walk based evaluation, with
>>> non-recursive, opcode dispatch based, expression evaluation.
>>> Projection is now implemented as part of expression evaluation.
>>>
>>> This both leads to significant performance improvements, and makes
>>> future just-in-time compilation of expressions easier.
>>
>> This is a huge achievement.  Congratulations!
>
> Agreed. Congratulations!

Hurray. Congratulations.
-- 
Michael


-- 
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.

[Michael Paquier]

On Wed, Feb 22, 2017 at 8:39 AM, Fujii Masao <masao.fu...@gmail.com> wrote:
> On Wed, Feb 22, 2017 at 6:57 AM, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
>> On Wed, Feb 22, 2017 at 4:12 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> Fujii Masao <fu...@postgresql.org> 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

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

[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

Re: [COMMITTERS] pgsql: Add new function dsa_allocate0.

[Michael Paquier]

On Fri, Feb 17, 2017 at 12:03 PM, Thomas Munro
 wrote:
> On Fri, Feb 17, 2017 at 11:34 AM, Thomas Munro
>  wrote:
>> On Fri, Feb 17, 2017 at 7:02 AM, Robert Haas  wrote:
>>> http://git.postgresql.org/pg/commitdiff/9acb85597f1223ac26a5b19a9345849c43d0ff54
>> Hmm.  This will segfault if you're out of memory.
>
> Or to provide a more useful response... maybe this should be like the
> attached?  Or maybe people think that dsa_allocate should throw on
> failure to allocate, like palloc?

 dp = dsa_allocate(area, size);
-object = dsa_get_address(area, dp);
-memset(object, 0, size);
+if (DsaPointerIsValid(dp))
+memset(dsa_get_address(area, dp), 0, size);
What you are proposing here looks like the right answer to me. Like
dsa_allocate, dsa_allocate0 should allow users to fallback to other
methods if what is returned is InvalidDsaPointer for consistency.
-- 
Michael


-- 
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: Replace PostmasterRandom() with a stronger source, second attemp

[Michael Paquier]

On Wed, Dec 7, 2016 at 7:46 AM, Andres Freund  wrote:
> On 2016-12-05 11:44:37 +, Heikki Linnakangas wrote:
>> Replace PostmasterRandom() with a stronger source, second attempt.
>
> Since this went in I've seen
> 2016-12-06 14:42:17.005 PST [23658][] LOG:  wrong key in cancel request for 
> process 23657
> 2016-12-06 14:42:19.010 PST [23662][] LOG:  wrong key in cancel request for 
> process 23657
> a couple times.
>
> I've not yet started debugging it. But it seems to only happen in the
> first (few?) queries in a new connection.

Hm. I recall testing this patch if cancel keys are working or not, but
connecting with pgbench -C runnung in parallel I am seeing the
following spurious error:
=# select pg_sleep(10);
^CCancel request sent
Time: 10001.496 ms (00:10.001)

With the logs complaining as well:
[1-1] db=[unknown],user=[unknown],app=[unknown],client=[local] LOG:
wrong key in cancel request for process 57361

As far as I can see processCancelRequest uses a long variable, and it
gets compared with an int32. MyCancelKey has been changed to an int32
in globals.c with the above commit. So on 64b machines the current
code is aimed to fail. Attached is a patch to fix the issue.

It is taking me at most 10 times to reproduce the failure without the
patch, after trying 20~ times with the patch I am still able to cancel
the queries.
-- 
Michael


fix-cancel-key.patch
Description: application/download

-- 
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: Use latch instead of select() in walreceiver

[Michael Paquier]

On Fri, Dec 2, 2016 at 10:29 AM, Peter Eisentraut  wrote:
> Use latch instead of select() in walreceiver
>
> Replace use of poll()/select() by WaitLatchOrSocket(), which is more
> portable and flexible.
>
> Also change walreceiver to use its procLatch instead of a custom latch.
>
> From: Petr Jelinek 

+   ResetLatch(>procLatch);
+   rc = WaitLatchOrSocket(>procLatch,
+  WL_POSTMASTER_DEATH | WL_SOCKET_READABLE |
+  WL_LATCH_SET,
+  PQsocket(streamConn),
+  0,
+  WAIT_EVENT_LIBPQWALRECEIVER_READ);
+   if (rc & WL_POSTMASTER_DEATH)
+   exit(1);
Hmm. We have always been very careful about not leaving immediately
from libpqwalreceiver.c which is an independent shared library so as
the WAL receiver can take cleanup actions. See here for more details:
https://www.postgresql.org/message-id/CAEepm=0hg_fx7kduhostpj_kpsuzw6k-7nuqny-dgaoaetn...@mail.gmail.com
-- 
Michael


-- 
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: Use OpenSSL EVP API for symmetric encryption in pgcrypto.

[Michael Paquier]

On Tue, Oct 18, 2016 at 6:28 AM, Tom Lane  wrote:
> Heikki Linnakangas  writes:
>> Use OpenSSL EVP API for symmetric encryption in pgcrypto.
>
> BTW, "narwhal" seems to have a problem with this.
> Not very clear what, maybe an incompatibility with old openssl versions?

Details are here:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=narwhal=2016-10-17%2016%3A00%3A01
-- 
Michael


-- 
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: pg_ctl: Detect current standby state from pg_control

[Michael Paquier]

On Mon, Sep 26, 2016 at 12:54 AM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> pg_ctl: Detect current standby state from pg_control
>
> Coverity thinks that this patch introduced a bunch of
> null-pointer-dereference hazards, and AFAICS it is right.
> The change in get_controlfile()'s API is completely broken
> and needs to be undone.

So you'd rather have a sleep(1) and complicate this code to replace it
with a WaitLatch() when the code is used by the backend? I don't agree
with that as the new API is cleaner as presented, though I agree that
it is a change that caller does not get any result in case of a CRC
mismatch, but who's going to use results that cannot be trusted
anyway? It seems to me that the correct fix here is that
pg_controldata.c should just exit like in the attached patch. Somewhat
I missed that during my lookup of c1dc51d4.

If we would still want callers of get_controlfile() to be allowed to
read untrustworthy results, we could just add a boolean status flag..
-- 
Michael


controldata-untrust.patch
Description: application/download

-- 
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: Add putenv support for msvcrt from Visual Studio 2013

[Michael Paquier]

On Tue, Sep 6, 2016 at 5:36 PM, Christian Ullrich <ch...@chrullrich.net> wrote:
> * Michael Paquier wrote:
>> In order to avoid any problems with the load and unload windows, my
>> bet goes for 0001, 0002 and 0003, with the last two patches merged
>> together, 0001 being only a set of independent fixes. That's ugly, but
>> that looks the safest course of actions. I have rebased/rewritten the
>> patches as attached. Thoughts?
>
>
> In lieu of convincing you to drop the entire thing, yes, that looks about
> right, except for the BOOL thing.

Yes, right. Thanks. Patch 0001 is definitely something that should be
applied and backpatched, the CloseHandle() call is buggy. Now 0002 and
0003 are improving things, but there have been no reports regarding
problems in this area, so this could just be applied to master I
guess. Christian, does that sound right?

By the way, how is it decided that a DLL gets unloaded in a process if
it is not pinned? Is that something the OS decides?
-- 
Michael


-- 
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: Add putenv support for msvcrt from Visual Studio 2013

[Michael Paquier]

On Thu, Sep 1, 2016 at 4:03 PM, Christian Ullrich <ch...@chrullrich.net> wrote:
> * Michael Paquier wrote:
>
>> On Wed, Apr 27, 2016 at 2:39 AM, Christian Ullrich <ch...@chrullrich.net>
>> wrote:
>
>>> * Christian Ullrich wrote:
>
>> And actually, by looking at those patches, isn't it a dangerous
>> practice to be able to load multiple versions of the same DLL routines
>> in the same workspace? I have personally very bad souvenirs with that,
>
> No, it is exactly what the version-specific CRTs are meant to allow. Each
> module uses the CRT version it needs, and they don't share any state, so
> absent bugs, they cannot conflict.

Hm. OK.

> That said, introducing this requirement would be a very significant change.
> I'm not sure how many independently maintained compiled extensions there
> are, but this would mean that their developers would have to have the
> matching VS versions for every PG distribution they want to support. Even if
> that's just EDB, it still is a lot of effort.

Yes. FWIW in my stuff everything gets compiled based on the same VS
version and bundled in the same msi, including a set of extensions
compiled from source, but perhaps my sight is too narrow in this
area... Well let's forget about that.

> My conclusion from April stands:
>
>> The fact that master looks like it does means that there have not been
>> many (or any) complaints about missing cross-module environment
>> variables. If nobody ever needs to see a variable set elsewhere, we
>> have a very simple solution: Why don't we simply throw out the whole
>> #ifdef _MSC_VER block?

Looking at the commit logs and 741e4ad7 that does not sound like a good idea.

+   if (!rtmodules[i].pinned)
+   {
+   HMODULE tmp;
+   BOOL res = GetModuleHandleEx(
+   GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
+   | GET_MODULE_HANDLE_EX_FLAG_PIN,
+   (LPCTSTR)rtmodules[i].hmodule,
+   );
+   rtmodules[i].pinned = !!res;
+   }
It is better to avoid !!. See for example 1a7a436 that avoided
problems with VS2015 as far as I recall.

In order to avoid any problems with the load and unload windows, my
bet goes for 0001, 0002 and 0003, with the last two patches merged
together, 0001 being only a set of independent fixes. That's ugly, but
that looks the safest course of actions. I have rebased/rewritten the
patches as attached. Thoughts?
-- 
Michael
From b809a0b1c323529c2d7460962a3c688ad8aec3f4 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 6 Sep 2016 15:51:55 +0900
Subject: [PATCH 1/3] Cleanups in pgwin32_putenv().

- Added UCRT and all debug CRTs
- Condensed the array to one line per item (it was getting long)
- Test HMODULEs for NULL instead of zero
- Removed unnecessary CloseHandle() call
---
 src/port/win32env.c | 61 +
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/src/port/win32env.c b/src/port/win32env.c
index e64065c..77f8334 100644
--- a/src/port/win32env.c
+++ b/src/port/win32env.c
@@ -45,36 +45,34 @@ pgwin32_putenv(const char *envval)
 		PUTENVPROC	putenvFunc;
 	}			rtmodules[] =
 	{
-		{
-			"msvcrt", 0, NULL
-		},		/* Visual Studio 6.0 / mingw */
-		{
-			"msvcr70", 0, NULL
-		},		/* Visual Studio 2002 */
-		{
-			"msvcr71", 0, NULL
-		},		/* Visual Studio 2003 */
-		{
-			"msvcr80", 0, NULL
-		},		/* Visual Studio 2005 */
-		{
-			"msvcr90", 0, NULL
-		},		/* Visual Studio 2008 */
-		{
-			"msvcr100", 0, NULL
-		},		/* Visual Studio 2010 */
-		{
-			"msvcr110", 0, NULL
-		},		/* Visual Studio 2012 */
-		{
-			"msvcr120", 0, NULL
-		},		/* Visual Studio 2013 */
-		{
-			"ucrtbase", 0, NULL
-		},		/* Visual Studio 2015 and later */
-		{
-			NULL, 0, NULL
-		}
+		/* Visual Studio 6.0 / mingw */
+		{"msvcrt",		NULL,	NULL},
+		{"msvcrtd",		NULL,	NULL},
+		/* Visual Studio 2002 */
+		{"msvcr70",		NULL,	NULL},
+		{"msvcr70d",	NULL,	NULL},
+		/* Visual Studio 2003 */
+		{"msvcr71",		NULL,	NULL},
+		{"msvcr71d",	NULL,	NULL},
+		/* Visual Studio 2005 */
+		{"msvcr80",		NULL,	NULL},
+		{"msvcr80d",	NULL,	NULL},
+		/* Visual Studio 2008 */
+		{"msvcr90",		NULL,	NULL},
+		{"msvcr90d",	NULL,	NULL},
+		/* Visual Studio 2010 */
+		{"msvcr100",	NULL,	NULL},
+		{"msvcr100d",	NULL,	NULL},
+		/* Visual Studio 2012 */
+		{"msvcr110",	NULL,	NULL},
+		{"msvcr110d",	NULL,	NULL},
+		/* Visual Studio 2013 */
+		{"msvcr120",	NULL,	NULL},
+		{"

Re: [COMMITTERS] pgsql: Add putenv support for msvcrt from Visual Studio 2013

[Michael Paquier]

On Wed, Apr 27, 2016 at 2:39 AM, Christian Ullrich  wrote:
> * Christian Ullrich wrote:
>
>> wrong even without considering the debug/release split. If we load a
>> compiled extension built with a CRT we have not seen yet, _after_ the
>> first call to pgwin32_putenv(), that module's CRT's view of its
>> environment will be frozen because we will never attempt to update it.
>
>
> Four patches attached:
>
> master --- 0001 --- 0002 --- 0003
>  \
>   `- 0004
>
> 0001 is just some various fixes to set the stage.
>
> 0002 fixes this "load race" by not remembering a negative result anymore.
> However, ...

>From 0001, which does not apply anymore on HEAD because of the
integration with MS2015:
if (rtmodules[i].putenvFunc == NULL)
{
-   CloseHandle(rtmodules[i].hmodule);
rtmodules[i].hmodule = INVALID_HANDLE_VALUE;
continue;
}
Nice catch. This portion is a bug and should be backpatched. As far as
I can read from MS docs, GetModuleHandle() retrieves an existing
handle so there is no need to free it. Or that would fail.

And actually, by looking at those patches, isn't it a dangerous
practice to be able to load multiple versions of the same DLL routines
in the same workspace? I have personally very bad souvenirs with that,
and particularly loading multiple versions of msvcr into the same
workspace can cause unwanted crashes and corruptions of processes. In
short I mean this thing: https://en.wikipedia.org/wiki/DLL_Hell.

So, shouldn't we first make the DLL list a little bit more severe
depending on the value of _MSC_VER? I would mean something like that:
#ifdef _MSC_VER >= 1900
{"ucrtbase",NULL,   NULL},
#elif _MSC_VER >= 1800
{"msvcr120",NULL,   NULL},
#elif etc, etc.
[...]
#endif

This would require modules to be built using the same msvc version as
the core server, but that's better than just plain crash if loaded
DLLs corrupt the stack. Am I missing something?
-- 
Michael


-- 
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] Re: pgsql: Convert contrib/seg's bool-returning SQL functions to V1 call co

[Michael Paquier]

On Wed, Apr 27, 2016 at 12:04 PM, Andres Freund  wrote:
> On 2016-04-26 22:59:44 -0400, Tom Lane wrote:
>> What's the argument that it makes debugging harder?  Especially if
>> you aren't using it?
>
> If you try to write a V1 function, but forget or mistype/rename the
> function in PG_FUNCTION_INFO_V1, you'll get crashes, at least if you're
> lucky.

At some point we'll surely arrive at dropping it... Now if V0 is
decided to be dropped, making a deprecation notice in the release
notes of major version X and actually dropping it 2-3 years after
would be really welcome to ease the transaction. I am guessing that
you meant that.
-- 
Michael


-- 
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: Use GRANT system to manage access to sensitive functions

[Michael Paquier]

On Thu, Apr 7, 2016 at 10:45 AM, Stephen Frost  wrote:
> Use GRANT system to manage access to sensitive functions
>
> Now that pg_dump will properly dump out any ACL changes made to
> functions which exist in pg_catalog, switch to using the GRANT system
> to manage access to those functions.
>
> This means removing 'if (!superuser()) ereport()' checks from the
> functions themselves and then REVOKEing EXECUTE right from 'public' for
> these functions in system_views.sql.

+1.
-- 
Michael


-- 
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: Generic Messages for Logical Decoding

[Michael Paquier]

On Wed, Apr 6, 2016 at 6:08 PM, Simon Riggs  wrote:
> Generic Messages for Logical Decoding
>
> API and mechanism to allow generic messages to be inserted into WAL that are
> intended to be read by logical decoding plugins. This commit adds an optional
> new callback to the logical decoding API.
>
> Messages are either text or bytea. Messages can be transactional, or not, and
> are identified by a prefix to allow multiple concurrent decoding plugins.
>
> (Not to be confused with Generic WAL records, which are intended to allow 
> crash
> recovery of extensible objects.)

jacana says boom:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana=2016-04-06%2012%3A02%3A05

*** 
c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/../pgsql/contrib/test_decoding/expected/messages.out
Wed Apr  6 08:02:30 2016
--- 
c:/mingw/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.build/contrib/test_decoding/./regression_output/results/messages.out
Wed Apr  6 08:40:24 2016
***
*** 54,75 

  COMMIT;
  SELECT 'žluťoučký kůň' FROM pg_logical_emit_message(true, 'test',
'žluťoučký kůň');
!?column?
! ---
!  žluťoučký kůň
! (1 row)
!
  SELECT data FROM pg_logical_slot_get_changes('regression_slot',
NULL, NULL, 'force-binary', '0', 'skip-empty-xacts', '1');
   data
! --
   message: transactional: 1 prefix: test, sz: 4 content:msg1
   message: transactional: 0 prefix: test, sz: 4 content:msg2
   message: transactional: 0 prefix: test, sz: 4 content:msg4
   message: transactional: 0 prefix: test, sz: 4 content:msg6
   message: transactional: 1 prefix: test, sz: 4 content:msg5
   message: transactional: 1 prefix: test, sz: 4 content:msg7
!  message: transactional: 1 prefix: test, sz: 19 content:žluťoučký kůň
! (7 rows)

  SELECT 'init' FROM pg_drop_replication_slot('regression_slot');
   ?column?
--- 54,70 

  COMMIT;
  SELECT 'žluťoučký kůň' FROM pg_logical_emit_message(true, 'test',
'žluťoučký kůň');
! ERROR:  character with byte sequence 0xc5 0xa5 in encoding "UTF8"
has no equivalent in encoding "WIN1252"
  SELECT data FROM pg_logical_slot_get_changes('regression_slot',
NULL, NULL, 'force-binary', '0', 'skip-empty-xacts', '1');
  data
! 
   message: transactional: 1 prefix: test, sz: 4 content:msg1
   message: transactional: 0 prefix: test, sz: 4 content:msg2
   message: transactional: 0 prefix: test, sz: 4 content:msg4
   message: transactional: 0 prefix: test, sz: 4 content:msg6
   message: transactional: 1 prefix: test, sz: 4 content:msg5
   message: transactional: 1 prefix: test, sz: 4 content:msg7
! (6 rows)

  SELECT 'init' FROM pg_drop_replication_slot('regression_slot');
   ?column?

I thought we always avoided non-ASCII characters in tests, no?
-- 
Michael


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

Re: [HACKERS] [COMMITTERS] pgsql: Avoid archiving XLOG_RUNNING_XACTS on idle server

[Michael Paquier]

On Wed, Apr 6, 2016 at 9:27 PM, Andres Freund  wrote:
> On 2016-04-06 13:11:40 +0100, Simon Riggs wrote:
>> On 6 April 2016 at 10:09, Andres Freund  wrote:
>> > On 2016-04-06 10:04:42 +0100, Simon Riggs wrote:
>> > The issue there is that we continue to issue checkpoints if the only
>> > activity since the last checkpoint was emitting a standby
>> > snapshot. That's because:
>> >
>>
>> I agree this is the current situation in 9.4 and 9.5, hence the bug report.
>>
>> This no longer occurs with the patches I have proposed. The snapshot is
>> skipped, so a further checkpoint never triggers.
>
> Not if there's a longrunning/idle transaction.
>
> Note that skipping the snapshot is actually a *problem* in some
> cases. As I've brought up upthread, to which you never replied. A
> xl_running_xacts->xcnt == 0/!overflowed snapshot can be very important
> for hot standby, because it allows ProcArrayApplyRecoveryInfo() to
> switch to INITIALIZED state:
> if (standbyState == STANDBY_SNAPSHOT_PENDING)
> {
> /*
>  * If the snapshot isn't overflowed or if its empty we can 
> reset our
>  * pending state and use this snapshot instead.
>  */
> if (!running->subxid_overflow || running->xcnt == 0)
> {
> /*
>  * If we have already collected known assigned xids, 
> we need to
>  * throw them away before we apply the recovery 
> snapshot.
>  */
> KnownAssignedXidsReset();
> standbyState = STANDBY_INITIALIZED;
> }

Yes. Such snapshot allows to initialize more quickly a hot standby...
And that's useful in some cases if the recovery is on a pending
snapshot (bgwriter standby snapshots help a lot with that btw).

>> > > > We now log more WAL with
>> > > > XLogArchiveTimeout > 0 than without.
>> >
>> > > And the problem with that is what?
>> >
>> > That an idle system unnecessarily produces WAL? Waking up disks and
>> > everything?
>> >
>>
>> The OP discussed a problem with archive_timeout > 0. Are you saying we
>> should put in a fix that applies whatever the setting of archive_timeout?
>
> Yes. We should strive to fix the full scope of an issue; not just the
> part complained about.
>
> You seem to ignore valid points made against your approach, apparently
> because you dismiss them as "emotive language". Stop.

Yeah... We have reached a clear consensus about the way things should
be done after quite a lot of discussions that has gone for a couple of
months. And Andres' design on the matter is what is getting approval
from everybody who has worked toward addressing this issue while
really taking care of the problem at its root. The patch, as currently
proposed, is a bandaid, and has been committed at the surprise of
everybody without any discussion.

Please let's revert this patch and really move toward fixing this
problem... Which is a way in short a way to fix the decision-making
for checkpoint skipping.
-- 
Michael


-- 
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: Avoid archiving XLOG_RUNNING_XACTS on idle server

[Michael Paquier]

On Mon, Apr 4, 2016 at 4:50 PM, Andres Freund  wrote:
> On 2016-04-04 08:44:47 +0100, Simon Riggs wrote:
>> That patch does exactly the same thing as the patch you prefer, just
>> does it differently;
>
> No, it doesn't; as explained above.

FWIW, I vote also for reverting this patch. This has been committed
without any real discussions..
-- 
Michael


-- 
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: Avoid archiving XLOG_RUNNING_XACTS on idle server

[Michael Paquier]

On Mon, Apr 4, 2016 at 3:24 PM, Andres Freund <and...@anarazel.de> wrote:
> On 2016-04-04 06:19:04 +, Simon Riggs wrote:
>> Avoid archiving XLOG_RUNNING_XACTS on idle server
>>
>> If archive_timeout > 0 we should avoid logging XLOG_RUNNING_XACTS if idle.
>>
>> Bug 13685 reported by Laurence Rowe, investigated in detail by Michael 
>> Paquier,
>> though this is not his proposed fix.
>> 20151016203031.3019.72...@wrigleys.postgresql.org
>>
>> Simple non-invasive patch to allow later backpatch to 9.4 and 9.5

Well...

> Uh. This is wrong. For one it breaks cleanup with logical decoding which
> does *NEED* to know that nothing is happening. Although only once, not 
> repeatedly.

On top of that, it does not interact with the snapshots logged by
other backends. We may want something in shared memory instead to
control the whole facility. Nor does this commit fix the checkpoint
skip logic.
-- 
Michael


-- 
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: Enable logical slots to follow timeline switches

[Michael Paquier]

On Thu, Mar 31, 2016 at 8:45 AM, Alvaro Herrera
 wrote:
> Tom Lane wrote:
>> Alvaro Herrera  writes:
>> > Enable logical slots to follow timeline switches
>>
>> Buildfarm doesn't like this one bit :-(
>
> Argh, forgot the alternate expected file when the needed feature is
> disabled.  Will fix.

hamster complains here:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamster=2016-03-31%2016%3A00%3A06
[...]
# Copying slots to replica
after_basebackup|test_decoding||547|0/560|0/598
# Copying slot 
'after_basebackup','test_decoding',NULL,'547','0/560','0/598'
connection error: 'psql::1: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
psql::1: connection to server was lost'
-- 
Michael


-- 
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: Add new replication mode synchronous_commit = 'remote_apply'.

[Michael Paquier]

On Wed, Mar 30, 2016 at 10:39 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Tue, Mar 29, 2016 at 9:37 PM, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
>> On Wed, Mar 30, 2016 at 10:31 AM, Robert Haas <rh...@postgresql.org> wrote:
>>> Add new replication mode synchronous_commit = 'remote_apply'.
>>>
>>> In this mode, the master waits for the transaction to be applied on
>>> the remote side, not just written to disk.  That means that you can
>>> count on a transaction started on the standby to see all commits
>>> previously acknowledged by the master.
>>>
>>> To make this work, the standby sends a reply after replaying each
>>> commit record generated with synchronous_commit >= 'remote_apply'.
>>> This introduces a small inefficiency: the extra replies will be sent
>>> even by standbys that aren't the current synchronous standby.  But
>>> previously-existing synchronous_commit levels make no attempt at all
>>> to optimize which replies are sent based on what the primary cares
>>> about, so this is no worse, and at least avoids any extra replies for
>>> people not using the feature at all.
>>>
>>> Thomas Munro, reviewed by Michael Paquier and by me.  Some additional
>>> tweaks by me.
>>
>> The commit message does not directly mention that the spec of
>> walrcv_receive has been changed in a backward-incompatible way so as
>> the wait control can be done with a latch directly in walreceiver.c
>> and not in libpqwalreceiver.c. That's not worth a mention in the
>> release notes as this is really low-level and compilation on any code
>> using this hook would simply fail on 9.6, so I am just mentioning it
>> for the sake of the archives.
>
> Yeah, I didn't really think that mattered much.  I'm not really sure
> what you even mean by backward-incompatible -- AFAIK, that's a private
> interface which we can whack around whenever we like.

By "Backward-incompatible", I mean that any custom library using this
walrcv hook is not going to compile anymore and we don't provide a
pre-9.5 equivalent. I don't think that's worth worrying though, I have
yet to see this interface being used for something else than
libpqwalreceiver to be honest.
-- 
Michael


-- 
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: Add new replication mode synchronous_commit = 'remote_apply'.

[Michael Paquier]

On Wed, Mar 30, 2016 at 10:31 AM, Robert Haas <rh...@postgresql.org> wrote:
> Add new replication mode synchronous_commit = 'remote_apply'.
>
> In this mode, the master waits for the transaction to be applied on
> the remote side, not just written to disk.  That means that you can
> count on a transaction started on the standby to see all commits
> previously acknowledged by the master.
>
> To make this work, the standby sends a reply after replaying each
> commit record generated with synchronous_commit >= 'remote_apply'.
> This introduces a small inefficiency: the extra replies will be sent
> even by standbys that aren't the current synchronous standby.  But
> previously-existing synchronous_commit levels make no attempt at all
> to optimize which replies are sent based on what the primary cares
> about, so this is no worse, and at least avoids any extra replies for
> people not using the feature at all.
>
> Thomas Munro, reviewed by Michael Paquier and by me.  Some additional
> tweaks by me.

The commit message does not directly mention that the spec of
walrcv_receive has been changed in a backward-incompatible way so as
the wait control can be done with a latch directly in walreceiver.c
and not in libpqwalreceiver.c. That's not worth a mention in the
release notes as this is really low-level and compilation on any code
using this hook would simply fail on 9.6, so I am just mentioning it
for the sake of the archives.
-- 
Michael


-- 
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: Best-guess attempt at fixing MSVC build for 68ab8e8ba4a471d9.

[Michael Paquier]

On Tue, Mar 22, 2016 at 10:20 AM, Andrew Dunstan  wrote:
>
>
> On 03/21/2016 09:25 AM, Tom Lane wrote:
>>
>> Andres Freund  writes:
>>>
>>> On 2016-03-21 22:38:50 +1300, David Rowley wrote:

 Perl is not my native tongue, but after a little study and some
 testing on a windows machine, the attached seems to fix the problem.
>>>
>>> I've pushed this, as I would like the buildfarm to be green before
>>> pushing the latch rework...
>>
>> Thanks!  If Andrew or somebody has a nicer solution, they can improve
>> this later.
>
> I've reviewed the changes briefly. They seem sane enough, although the code
> could do with a little logical reordering.

We're stuck with the list of files defined in OBJS, and psql's
Makefile is listing psqlscan.o, so that's still the best way to go.
-- 
Michael


-- 
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: Convert psql's flex lexer to be re-entrant, and make it compile

[Michael Paquier]

On Sat, Mar 19, 2016 at 10:22 AM, Tom Lane  wrote:
> Also, stop compiling psqlscan as part of mainloop.c, and make it a
> standalone build target instead.  This is a lot cleaner than before, though
> it doesn't really change much in practice as of this commit.  (I'm not sure
> whether the MSVC build scripts will need some help with this part, but the
> buildfarm will soon tell us.)

This is fine, no tweaking is needed. psql is listing directly
psqlscan.l as a file, and .c from generated from .l files with flex
are generated unconditionally before the compilation.
-- 
Michael


-- 
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: Don't vacuum all-frozen pages.

[Michael Paquier]

On Fri, Mar 11, 2016 at 2:08 AM, Amit Langote
 wrote:
> On 2016/03/11 8:19, Peter Geoghegan wrote:
>> On Thu, Mar 10, 2016 at 1:22 PM, Andres Freund  wrote:
>>> Yeha!
>>
>> Fantastic effort, particularly from Masahiko. Well done.
>
> +1!

Yuhu.
-- 
Michael


-- 
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: Checkpoint sorting and balancing.

[Michael Paquier]

On Fri, Mar 11, 2016 at 2:29 AM, Andres Freund  wrote:
> Checkpoint sorting and balancing.

+1.
-- 
Michael


-- 
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: Add pg_size_bytes() to parse human-readable size strings.

[Michael Paquier]

On Sat, Feb 20, 2016 at 7:17 PM, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
> On 20 February 2016 at 10:12, Michael Paquier <michael.paqu...@gmail.com> 
> wrote:
>> Happy first commit.
>
> Arg. Not so much.
>
> Looks like I broke something -- looking into it now :-(

The terabyte conversion is at fault:
Expected:
!  -1tb  |-1099511627776
Result:
!  -1tb  |-1

+   else if (pg_strcasecmp(strptr, "gb") == 0)
+   multiplier = 1024 * 1024 * 1024;
+   else if (pg_strcasecmp(strptr, "tb") == 0)
+   multiplier = 1024 * 1024 * 1024 * 1024L;
Why adding an 'L' here?
-- 
Michael


-- 
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: Add pg_size_bytes() to parse human-readable size strings.

[Michael Paquier]

On Sat, Feb 20, 2016 at 7:12 PM, Michael Paquier
<michael.paqu...@gmail.com> wrote:
> On Sat, Feb 20, 2016 at 7:07 PM, Dean Rasheed <dean.a.rash...@gmail.com> 
> wrote:
>> Add pg_size_bytes() to parse human-readable size strings.
>>
>> This will parse strings in the format produced by pg_size_pretty() and
>> return sizes in bytes. This allows queries to be written with clauses
>> like "pg_total_relation_size(oid) > pg_size_bytes('10 GB')".
>>
>> Author: Pavel Stehule with various improvements by Vitaly Burovoy
>> Discussion: 
>> http://www.postgresql.org/message-id/cafj8prd-tgodknxdygecza4on01_urqprwf-8ldkse-6bdh...@mail.gmail.com
>> Reviewed-by: Vitaly Burovoy, Oleksandr Shulgin, Kyotaro Horiguchi,
>> Michael Paquier and Robert Haas
>
> Happy first commit.

And happy first buildfarm breakage :)
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=termite=2016-02-20%2010%3A10%3A07
-- 
Michael


-- 
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: Add pg_size_bytes() to parse human-readable size strings.

[Michael Paquier]

On Sat, Feb 20, 2016 at 7:07 PM, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
> Add pg_size_bytes() to parse human-readable size strings.
>
> This will parse strings in the format produced by pg_size_pretty() and
> return sizes in bytes. This allows queries to be written with clauses
> like "pg_total_relation_size(oid) > pg_size_bytes('10 GB')".
>
> Author: Pavel Stehule with various improvements by Vitaly Burovoy
> Discussion: 
> http://www.postgresql.org/message-id/cafj8prd-tgodknxdygecza4on01_urqprwf-8ldkse-6bdh...@mail.gmail.com
> Reviewed-by: Vitaly Burovoy, Oleksandr Shulgin, Kyotaro Horiguchi,
> Michael Paquier and Robert Haas

Happy first commit.
-- 
Michael


-- 
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: Support parallel joins, and make related improvements.

[Michael Paquier]

On Thu, Jan 21, 2016 at 7:59 AM, Bruce Momjian  wrote:
> On Wed, Jan 20, 2016 at 07:41:07PM +, Robert Haas wrote:
>> Support parallel joins, and make related improvements.
>
> Wow, that is big news!

Yuhu!
-- 
Michael


-- 
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: some bullshit

[Michael Paquier]

On Tue, Dec 22, 2015 at 8:16 AM, Alvaro Herrera
 wrote:
> Alvaro Herrera wrote:
>> some bullshit
>
> This commit was supposed to be squashed with the next one --- hence
> the, err, not-terribly-descriptive commit message.  This mistake may
> cause future "git bisect" runs to give spurious failures, but I'm
> disinclined to remove it from history nonetheless.  These changes (which
> would cause a compile to fail on this branch) are removed by the next
> commit.

That's not the first time something that breaks compilation of the
backend has been committed, and among all the things that already
happened nobody yet complained about a bisect failing. So no worries
regarding that and take it easy :)
-- 
Michael


-- 
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: Code and docs review for multiple -c and -f options in psql.

[Michael Paquier]

On Mon, Dec 14, 2015 at 11:16 AM, Michael Paquier wrote:
> -   cell = (SimpleActionListCell *)
> -   pg_malloc(offsetof(SimpleActionListCell, val) + vallen + 1);
> Thanks! Among all those things this bit is a bit shameful..

(I am the one at the origin of that FWIW)
-- 
Michael


-- 
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: Code and docs review for multiple -c and -f options in psql.

[Michael Paquier]

On Mon, Dec 14, 2015 at 4:52 AM, Tom Lane  wrote:
> Code and docs review for multiple -c and -f options in psql.
>
> Commit d5563d7df94488bf drew complaints from Coverity, which quite
> correctly complained that one copy of each -c or -f string was being
> leaked.  What's more, simple_action_list_append was allocating enough space
> for still a third copy of each string as part of the SimpleActionListCell,
> even though that coding method had been superseded by a separate strdup
> operation.  There were some other minor coding infelicities too.  The
> documentation needed more work as well, eg it forgot to explain that -c
> causes psql not to accept any interactive input.

-   cell = (SimpleActionListCell *)
-   pg_malloc(offsetof(SimpleActionListCell, val) + vallen + 1);
Thanks! Among all those things this bit is a bit shameful..
-- 
Michael


-- 
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 bug leading to restoring unlogged relations from empty files

[Michael Paquier]

On Mon, Dec 14, 2015 at 2:26 PM, Peter Eisentraut  wrote:
> the call to LWLockHeldByMe() is useless.

Yes, but it should be an Assert. I guess that Andres is on it..
-- 
Michael


-- 
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: pg_rewind: Don't error if the two clusters are already on the sa

[Michael Paquier]

On Sat, Dec 12, 2015 at 9:11 AM, Tom Lane  wrote:
> Peter Eisentraut  writes:
>> pg_rewind: Don't error if the two clusters are already on the same timeline
>> This previously resulted in an error and a nonzero exit status, but
>> after discussion this should rather be a noop with a zero exit status.
>
> Hm, if we're going to do that, shouldn't we back-patch the behavior
> change into 9.5 as well?

+1. It would be good to get consistent across branches here.
-- 
Michael


-- 
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: Refactor Perl test code

[Michael Paquier]

On Thu, Dec 3, 2015 at 8:38 AM, Tom Lane wrote:
> Test Summary Report
> ---
> t/001_initdb.pl (Wstat: 6400 Tests: 8 Failed: 0)
>   Non-zero exit status: 25
>   Parse errors: Bad plan.  You planned 14 tests but ran 8.
> Files=1, Tests=8,  0 wallclock secs ( 0.02 usr  0.00 sys +  0.08 cusr  0.01 
> csys =  0.11 CPU)
> Result: FAIL
> make[2]: *** [check] Error 1
> make[2]: Leaving directory `/home/postgres/pgsql/src/bin/initdb'
> make[1]: *** [check-initdb-recurse] Error 2
> make[1]: Leaving directory `/home/postgres/pgsql/src/bin'
> make: *** [check-world-src/bin-recurse] Error 2
>
> The buildfarm looks pretty unhappy too, though I've not checked to see if
> it's exactly the same symptom everywhere.

Or in more details:
Undefined subroutine ::run called at
/Users/mpaquier/git/postgres/src/bin/initdb/../../../src/test/perl/TestLib.pm
line 146.
I am seeing the same failure on all the machines in the buildfarm.

The issue is fixed by the patch attached. This has been visibly
forgotten in the version pushed.
Regards,
-- 
Michael
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index af46dc8..7edd4c4 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -143,7 +143,7 @@ sub system_or_bail
 sub run_log
 {
 	print("# Running: " . join(" ", @{ $_[0] }) . "\n");
-	return run(@_);
+	return IPC::Run::run(@_);
 }
 
 sub slurp_dir

-- 
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: libpq: Notice errors a backend may have sent just before dying.

[Michael Paquier]

On Fri, Nov 13, 2015 at 1:54 AM, Tom Lane  wrote:
> I wrote:
>> After looking around, I suspect what actually happened in your test
>> was that we kept pumping pqReadData until it realized it was seeing EOF,
>> whereupon it did pqDropConnection(), and guess what that does:
>
>>   /* Discard any unread/unsent data */
>>   conn->inStart = conn->inCursor = conn->inEnd = 0;
>>   conn->outCount = 0;
>
> So after further review, this is a bug I introduced in 210eb9b74:
> the fact that some code paths flushed the buffers and some did not
> was less of an oversight than it appeared.  That explains why the
> problem wasn't noticed years ago, because we'd certainly tested
> pqHandleSendFailure and friends before.
>
> I'm inclined to deal with this by adding a "dropInput" boolean flag
> to pqDropConnection(), rather than reverting the centralization of
> that logic altogether.
>
> I'll go clean this up ...

Interesting lesson. Thanks!
-- 
Michael


-- 
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: Generate parallel sequential scan plans in simple cases.

[Michael Paquier]

On Thu, Nov 12, 2015 at 4:06 AM, Joe Conway  wrote:
> On 11/11/2015 10:54 AM, Robert Haas wrote:
>> On Wed, Nov 11, 2015 at 10:35 AM, Magnus Hagander  
>> wrote:
>>> On Wed, Nov 11, 2015 at 4:32 PM, Simon Riggs  wrote:
 On 11 November 2015 at 14:03, Robert Haas  wrote:
> Generate parallel sequential scan plans in simple cases.
 Congratulations!
>>>
>>> +1. That's an exciting milestone!
>>
>> Thank you, both of you!  I'm pretty excited.
>>
>> Needless to say, there is a ton of work left.  But I hope that getting
>> to this point will make it easier for other people to participate in
>> the work, or anyway, test.
>
> This is a huge milestone! Congratulations and many thanks for making it
> happen.

Yay.
-- 
Michael


-- 
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: Add missing_ok option to the SQL functions for reading files.

[Michael Paquier]

On Fri, Aug 28, 2015 at 9:39 PM, Bruce Momjian br...@momjian.us wrote:
 On Sun, Jul  5, 2015 at 08:44:41PM +0900, Michael Paquier wrote:
 On Mon, Jun 29, 2015 at 3:39 AM, Heikki Linnakangas
 heikki.linnakan...@iki.fi wrote:
  Add missing_ok option to the SQL functions for reading files.
 
  This makes it possible to use the functions without getting errors, if 
  there
  is a chance that the file might be removed or renamed concurrently.
  pg_rewind needs to do just that, although this could be useful for other
  purposes too. (The changes to pg_rewind to use these functions will come in
  a separate commit.)
 
  The read_binary_file() function isn't very well-suited for extensions.c's
  purposes anymore, if it ever was. So bite the bullet and make a copy of it
  in extension.c, tailored for that use case. This seems better than the
  accidental code reuse, even if it's a some more lines of code.

 This has been done done as the part of a fix for the issues with
 pg_rewind, but perhaps it should be mentioned in the release notes?

 Yes, I will pick up that change during the creation of the 9.6 release
 notes.

Even if it has been introduced in 9.5?
-- 
Michael


-- 
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: Reduce lock levels for ALTER TABLE SET autovacuum storage option

[Michael Paquier]

On Fri, Aug 14, 2015 at 10:20 PM, Simon Riggs si...@2ndquadrant.com wrote:
 Reduce lock levels for ALTER TABLE SET autovacuum storage options

 Reduce lock levels down to ShareUpdateExclusiveLock for all autovacuum-related
 relation options when setting them using ALTER TABLE.

 Add infrastructure to allow varying lock levels for relation options in later
 patches. Setting multiple options together uses the highest lock level 
 required
 for any option. Works for both main and toast tables.

Thanks for the last push!
-- 
Michael


-- 
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: Allow pg_create_physical_replication_slot() to reserve WAL.

[Michael Paquier]

On Tue, Aug 11, 2015 at 7:47 PM, Andres Freund and...@anarazel.de wrote:
 Allow pg_create_physical_replication_slot() to reserve WAL.

 When creating a physical slot it's often useful to immediately reserve
 the current WAL position instead of only doing after the first feedback
 message arrives. That e.g. allows slots to guarantee that all the WAL
 for a base backup will be available afterwards.

 Logical slots already have to reserve WAL during creation, so generalize
 that logic into being usable for both physical and logical slots.

Why hasn't this addition been spread as well in the replication
protocol? It seems to me that most of the refactoring work has been
done with ReplicationSlotReserveWal.
-- 
Michael


-- 
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: Allow pg_create_physical_replication_slot() to reserve WAL.

[Michael Paquier]

On Fri, Aug 14, 2015 at 3:50 PM, Andres Freund and...@anarazel.de wrote:
 On 2015-08-14 15:32:17 +0900, Michael Paquier wrote:
 Why hasn't this addition been spread as well in the replication
 protocol? It seems to me that most of the refactoring work has been
 done with ReplicationSlotReserveWal.

 Feel free to send a patch.

I don't mind giving it a try if time allows... CREATE_REPLICATION_SLOT
IDENT K_PHYSICAL slot_options? With slot_options: (reserve = on/off)?
And, actually, here is an unrelated patch, the docs are referring to
confirmed_flush instead of confirmed_flush_lsn ;)
-- 
Michael
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 1e895e4..b4ea227 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -5579,7 +5579,7 @@
  /row
 
  row
-  entrystructfieldconfirmed_flush/structfield/entry
+  entrystructfieldconfirmed_flush_lsn/structfield/entry
   entrytypepg_lsn/type/entry
   entry/entry
   entryThe address (literalLSN/literal) up to which the logical

-- 
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: Optionally don't error out due to preexisting slots in commandli

[Michael Paquier]

On Mon, Jul 13, 2015 at 5:17 AM, Andres Freund and...@anarazel.de wrote:
 Optionally don't error out due to preexisting slots in commandline utilities.

 pg_receivexlog and pg_recvlogical error out when --create-slot is
 specified and a slot with the same name already exists. In some cases,
 especially with pg_receivexlog, that's rather annoying and requires
 additional scripting.

 Backpatch to 9.5 as slot control functions have newly been added to
 pg_receivexlog, and there doesn't seem much point leaving it in a less
 useful state.

Andres, Coverity is pointing out that this commit missed the shot when
sqlstate is NULL in CreateReplicationSlot and this would crash.
Something like the patch attached look adapted to me, vacuumdb.c doing
the necessary checks similarly.
Regards,
-- 
Michael
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 91f919c..ccc6108 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -340,7 +340,9 @@ CreateReplicationSlot(PGconn *conn, const char *slot_name, const char *plugin,
 	{
 		const char *sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
 
-		if (slot_exists_ok  strcmp(sqlstate, ERRCODE_DUPLICATE_OBJECT) == 0)
+		if (sqlstate 
+			slot_exists_ok 
+			strcmp(sqlstate, ERRCODE_DUPLICATE_OBJECT) == 0)
 		{
 			destroyPQExpBuffer(query);
 			PQclear(res);

-- 
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: This routine was calling ecpg_alloc to allocate to memory but di

[Michael Paquier]

On Wed, Aug 12, 2015 at 8:59 PM, Michael Meskes mes...@postgresql.org wrote:
 On 23.07.2015 09:19, Michael Paquier wrote:
 On Thu, Feb 5, 2015 at 11:14 PM, Michael Meskes mes...@postgresql.org 
 wrote:
 This routine was calling ecpg_alloc to allocate to memory but did not
 actually check the returned pointer allocated, potentially NULL which
 could be the result of a malloc call.

 Issue noted by Coverity, fixed by Michael Paquier mich...@otacoo.com

 Coming back to it, perhaps this meritates a backpatch? Even if that's
 unlikely to happen...

 cherry-picked and pushed to all back branches.

 Sorry, the disadvantage of testing in master for a while, I simply
 forgot about this change.

No pb. Thanks! I forgot myself...
-- 
Michael


-- 
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 pg_rewind when pg_xlog is a symlink.

[Michael Paquier]

On Mon, Aug 3, 2015 at 10:37 PM, Heikki Linnakangas wrote:
 That's only on master, though. The TAP tests don't run on Windows in 9.5
 anyway.

Oops, yes I got mistaken by the commit on 9.5.

 I guess the pg_rewind tests used to work, but we didn't really advertise or
 make it easy to run it, so I'm not sure it's worth it to try to maintain
 that. Then again, we might want to backpatch all the TAP-test changes to
 make them work on Windows to 9.5, now that they've gotten some testing in
 the buildfarm and seem to work.

Usually new features are not backpatched, and the support for MSVC is one IMO.

  On systems that don't support symbolic links, raises an exception. To
 check for that, use eval:

 $symlink_exists = eval { symlink(,); 1 };


 I wonder if we should be testing for that, instead of $windows_os.

Yes, good point! The second platform referred as unsupported is RISC OS:
http://perldoc.perl.org/perlport.html#symlink
And Postgres can visibly work on it. It would be good to get for
instance a RaspPI on the buildfarm with it, there is a development
version.
-- 
Michael


-- 
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 pg_rewind when pg_xlog is a symlink.

[Michael Paquier]

On Mon, Aug 3, 2015 at 9:34 PM, Heikki Linnakangas
heikki.linnakan...@iki.fi wrote:
 Fix pg_rewind when pg_xlog is a symlink.

 pg_xlog is often a symlink, typically to a different filesystem. Don't
 get confused and comlain about by that, and just always pretend that it's a
 normal directory, even if it's really a symlink.

 Also add a test case for this.

+   symlink($master_xlogdir, $test_master_datadir/pg_xlog) or die;

This will die on Windows, hence I think that this test should be
skipped in this case.
-- 
Michael


-- 
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: pg_basebackup: Add tests for -X option

[Michael Paquier]

On Wed, Jul 29, 2015 at 9:34 AM, Peter Eisentraut pete...@gmx.net wrote:
 pg_basebackup: Add tests for -X option

One thing that I noticed after more tests is that actually the LSN
position restart_lsn is not necessarily 8-character long as this
tests, but it can be 7-character length as well:
like($lsn, qr!^0/[0-9A-Z]{8}$!, 'restart LSN of slot has advanced');
So you may have random failures depending on how much the LSN has
advanced depending on the number of base backups taken on the server
during the tests (found out the problem on Windows because we need to
skip some tests as there is no symlink support). I would suggest that
instead of checking the format of restart_lsn we encapsulate it within
pg_xlogfile_name and check if result has a correct length of 24
characters with characters 0-9A-F, like in the patch attached.
Sorry for not noticing that before, attached is a patch to fix the issue.
-- 
Michael


20150729_fix_tap_basebackup.patch
Description: binary/octet-stream

-- 
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: This routine was calling ecpg_alloc to allocate to memory but di

[Michael Paquier]

On Thu, Feb 5, 2015 at 11:14 PM, Michael Meskes mes...@postgresql.org wrote:
 This routine was calling ecpg_alloc to allocate to memory but did not
 actually check the returned pointer allocated, potentially NULL which
 could be the result of a malloc call.

 Issue noted by Coverity, fixed by Michael Paquier mich...@otacoo.com

Coming back to it, perhaps this meritates a backpatch? Even if that's
unlikely to happen...
-- 
Michael


-- 
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: pg_upgrade: fix one-byte per empty db memory leak

[Michael Paquier]

On Tue, Jul 21, 2015 at 10:29 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Michael Paquier wrote:
 On Sat, Jan 10, 2015 at 2:12 AM, Bruce Momjian br...@momjian.us wrote:
  pg_upgrade:  fix one-byte per empty db memory leak
 
  Report by Tatsuo Ishii, Coverity

 Shouldn't this commit be backpatched where needed.?

 A one byte leak?  Come again.

Just for the sake of correctness, nothing else (/me leaves to sleep).
-- 
Michael


-- 
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: vacuumlo: Avoid unlikely memory leak.

[Michael Paquier]

On Thu, Jan 15, 2015 at 5:18 AM, Robert Haas rh...@postgresql.org wrote:
 vacuumlo: Avoid unlikely memory leak.

 Spotted by Coverity.  This isn't likely to matter in practice, but
 there's no harm in fixing it.

Coming back to this one, shouldn't this commit be backpatched?
-- 
Michael


-- 
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: pg_upgrade: fix one-byte per empty db memory leak

[Michael Paquier]

On Sat, Jan 10, 2015 at 2:12 AM, Bruce Momjian br...@momjian.us wrote:
 pg_upgrade:  fix one-byte per empty db memory leak

 Report by Tatsuo Ishii, Coverity

Shouldn't this commit be backpatched where needed.?
-- 
Michael


-- 
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: Retain comments on indexes and constraints at ALTER TABLE ... TY

[Michael Paquier]

On Fri, Jul 17, 2015 at 11:16 PM, Kevin Grittner kgri...@ymail.com wrote:
 Heikki Linnakangas heikki.linnakan...@iki.fi wrote:

 This fixes bug #13126, reported by Kirill Simonov.

 It looks like you missed something with the addition of
 AT_ReAddComment:

 test_ddl_deparse.c:80:11: warning: enumeration value 'AT_ReAddComment' not 
 handled in switch [-Wswitch]
 switch (subcmd-subtype)
 ^

Oops. If someone could pick up the attached (backpatch to 9.5 needed)...
-- 
Michael


20150717_testddl_warning.patch
Description: binary/octet-stream

-- 
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: Retain comments on indexes and constraints at ALTER TABLE ... TY

[Michael Paquier]

On Tue, Jul 14, 2015 at 10:02 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Heikki Linnakangas heikki.linnakan...@iki.fi writes:
 Retain comments on indexes and constraints at ALTER TABLE ... TYPE ...

 One or the other of these patches has broken the majority of the
 buildfarm.

Some ORDER BY clauses are missing to make the output consistent it
seems. At the same time the queries could be reformated a bit to not
be longer than 80 characters. A patch is attached.
-- 
Michael
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 6b9291b..bd5069d 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2409,23 +2409,31 @@ CREATE TABLE comment_test (
 CREATE INDEX comment_test_index ON comment_test(indexed_col);
 COMMENT ON COLUMN comment_test.id IS 'Column ''id'' on comment_test';
 COMMENT ON INDEX comment_test_index IS 'Simple index on comment_test';
-COMMENT ON CONSTRAINT comment_test_positive_col_check ON comment_test IS 'CHECK constraint on comment_test.positive_col';
-COMMENT ON CONSTRAINT comment_test_pk ON comment_test IS 'PRIMARY KEY constraint of comment_test';
-COMMENT ON INDEX comment_test_pk IS 'Index backing the PRIMARY KEY of comment_test';
+COMMENT ON CONSTRAINT comment_test_positive_col_check ON comment_test
+IS 'CHECK constraint on comment_test.positive_col';
+COMMENT ON CONSTRAINT comment_test_pk ON comment_test
+IS 'PRIMARY KEY constraint of comment_test';
+COMMENT ON INDEX comment_test_pk
+IS 'Index backing the PRIMARY KEY of comment_test';
 SELECT col_description('comment_test'::regclass, 1) as comment;
comment   
 -
  Column 'id' on comment_test
 (1 row)
 
-SELECT indexrelid::regclass as index, obj_description(indexrelid, 'pg_class') as comment FROM pg_index where indrelid = 'comment_test'::regclass;
+SELECT indexrelid::regclass AS index,
+obj_description(indexrelid, 'pg_class') AS comment
+FROM pg_index where indrelid = 'comment_test'::regclass ORDER BY index;
index|comment
 +---
- comment_test_index | Simple index on comment_test
  comment_test_pk| Index backing the PRIMARY KEY of comment_test
+ comment_test_index | Simple index on comment_test
 (2 rows)
 
-SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment FROM pg_constraint where conrelid = 'comment_test'::regclass;
+SELECT conname as constraint,
+obj_description(oid, 'pg_constraint') AS comment
+FROM pg_constraint where conrelid = 'comment_test'::regclass
+ORDER BY conname;
constraint|comment
 -+---
  comment_test_pk | PRIMARY KEY constraint of comment_test
@@ -2449,18 +2457,23 @@ SELECT col_description('comment_test'::regclass, 1) as comment;
  Column 'id' on comment_test
 (1 row)
 
-SELECT indexrelid::regclass as index, obj_description(indexrelid, 'pg_class') as comment FROM pg_index where indrelid = 'comment_test'::regclass;
+SELECT indexrelid::regclass AS index,
+obj_description(indexrelid, 'pg_class') AS comment
+FROM pg_index where indrelid = 'comment_test'::regclass ORDER BY index;
index|comment
 +---
- comment_test_pk| Index backing the PRIMARY KEY of comment_test
  comment_test_index | Simple index on comment_test
+ comment_test_pk| Index backing the PRIMARY KEY of comment_test
 (2 rows)
 
-SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment FROM pg_constraint where conrelid = 'comment_test'::regclass;
+SELECT conname AS constraint,
+obj_description(oid, 'pg_constraint') AS comment
+FROM pg_constraint where conrelid = 'comment_test'::regclass
+ORDER BY conname;
constraint|comment
 -+---
- comment_test_positive_col_check | CHECK constraint on comment_test.positive_col
  comment_test_pk | PRIMARY KEY constraint of comment_test
+ comment_test_positive_col_check | CHECK constraint on comment_test.positive_col
 (2 rows)
 
 -- Check that we map relation oids to filenodes and back correctly.  Only
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 9f755fa..666ac9c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1605,13 +1605,21 @@ CREATE INDEX comment_test_index ON comment_test(indexed_col);
 
 COMMENT ON COLUMN comment_test.id IS 'Column ''id'' on comment_test';
 COMMENT ON INDEX comment_test_index IS 'Simple index on comment_test';
-COMMENT ON CONSTRAINT 

Re: [COMMITTERS] pgsql: Add missing_ok option to the SQL functions for reading files.

[Michael Paquier]

On Mon, Jun 29, 2015 at 3:39 AM, Heikki Linnakangas
heikki.linnakan...@iki.fi wrote:
 Add missing_ok option to the SQL functions for reading files.

 This makes it possible to use the functions without getting errors, if there
 is a chance that the file might be removed or renamed concurrently.
 pg_rewind needs to do just that, although this could be useful for other
 purposes too. (The changes to pg_rewind to use these functions will come in
 a separate commit.)

 The read_binary_file() function isn't very well-suited for extensions.c's
 purposes anymore, if it ever was. So bite the bullet and make a copy of it
 in extension.c, tailored for that use case. This seems better than the
 accidental code reuse, even if it's a some more lines of code.

This has been done done as the part of a fix for the issues with
pg_rewind, but perhaps it should be mentioned in the release notes?
-- 
Michael


-- 
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: Run the C portions of guc-file.l through pgindent.

[Michael Paquier]

On Mon, Jun 29, 2015 at 9:49 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Run the C portions of guc-file.l through pgindent.

 Yeah, I know, pretty anal-retentive of me.  But we oughta find some
 way to automate this for the .y and .l files.

.y files may be tricky and .l files less. Still one good way to test
such things would be to use something like that and see at least what
happens when doing a run (haven't tested, use at your own risk):
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 0d3859d..3995214 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -519,6 +519,7 @@ File::Find::find(
(($dev, $ino, $mode, $nlink, $uid, $gid) = lstat($_))
   -f _
   /^.*\.[ch]\z/s
+  /^.*\.(h|c|y|l)\z/s
   push(@files, $File::Find::name);
  }
},

Regards,
-- 
Michael


-- 
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: Fixed some memory leaks in ECPG.

[Michael Paquier]

On Sat, Jun 13, 2015 at 6:06 PM, Michael Meskes mes...@postgresql.org wrote:
 On Sat, Jun 13, 2015 at 12:21:49PM +0900, Michael Paquier wrote:
 Shouldn't this commit be cherry-picked on back-branches?

 Yes and it will, I'm currently working on it. I somehow got into the habbit
 of doing the cherry-picks the day after to run the patch through the
 buildfarm first. Wasn't really needed for these two, maybe I should change
 my habbits again.

Well, that's your style. I think that you should keep it if you think
that this is the right way of doing for such patches ;)
-- 
Michael


-- 
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 intoasc() in Informix compat lib. This function used to be a

[Michael Paquier]

On Fri, Jun 12, 2015 at 10:05 PM, Michael Meskes mes...@postgresql.org wrote:
 Fix intoasc() in Informix compat lib. This function used to be a noop.

Shouldn't this commit be cherry-picked on back-branches?
-- 
Michael


-- 
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: pgindent: more doc updates for skipping __asm__ files

[Michael Paquier]

On Mon, May 25, 2015 at 10:51 AM, Bruce Momjian br...@momjian.us wrote:
 pgindent:  more doc updates for skipping __asm__ files

s/they contains/they contain/.
-- 
Michael


-- 
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: Recognize REGRESS_OPTS += ... syntax in MSVC build scripts.

[Michael Paquier]

On Tue, May 19, 2015 at 2:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Recognize REGRESS_OPTS += ... syntax in MSVC build scripts.

 Necessitated by commit b14cf229f4bd7238be2e31d873dc5dd241d3871e.
 Per buildfarm.

Perhaps \+? could be spread to REGRESS as well in fetchTests?
-- 
Michael


-- 
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: Separate block sampling functions

[Michael Paquier]

On Fri, May 15, 2015 at 8:44 PM, Andrew Dunstan and...@dunslane.net wrote:

 On 05/15/2015 06:04 AM, Simon Riggs wrote:

 On 15 May 2015 at 04:59, Tom Lane t...@sss.pgh.pa.us
 mailto:t...@sss.pgh.pa.us wrote:

 The difference there was that that was specifically adding a new
 feature
 of value to FDWs.  This is just drive-by breakage.


 I think that comment is reasonable. I will continue with my commits of
 tablesample, then return to see if we can improve/revert the API breakage.




 OK, good, but I'm not going to accept Michael's pull request until the API
 is stable.

Well, I guess that this is going to be incorrect in any case now.
Still any API change can be done cleanly in a matter of minutes for
this FDW.
-- 
Michael


-- 
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: contrib/tsm_system_time

[Michael Paquier]

On Sat, May 16, 2015 at 5:28 AM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, May 15, 2015 at 3:32 PM, Simon Riggs si...@2ndquadrant.com wrote:
 contrib/tsm_system_time

 I don't want to be overly critical here, but I find that a pretty
 worthless commit message.

And there is no documentation.
-- 
Michael


-- 
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: Add pg_audit, an auditing extension

[Michael Paquier]

On Sat, May 16, 2015 at 4:44 AM, Stephen Frost sfr...@snowman.net wrote:
 Fujii,

 * Fujii Masao (masao.fu...@gmail.com) wrote:
 pg_audit uses 1.0.0 as its version number. But, is the third digit really
 required? Why? We usually uses the version number with two digits in
 contrib modules.

 I have to admit, I didn't look closely at how we handled versions in
 contrib modules and that has been the same since the patch was first
 posted, as I recall.  No problem changing it to 1.0 and I'll take care
 of that soon.

 CREATE EXTENSION pg_audit failed with the following error message
 when shared_preload_libraries and pg_audit.log are set to pg_audit and
 ddl, respectively.

 =# create extension pg_audit ;
 ERROR:  pg_event_trigger_ddl_commands() can only be called in an event
 trigger function
 CONTEXT:  SQL statement SELECT UPPER(object_type), object_identity
   FROM pg_event_trigger_ddl_commands()

 Interesting.  I'm very curious about this error and if it impacts other
 extensions which use event triggers.  I'll look into it.

 In Makefile, PGFILEDESC should be added.

 +# pg_audit/Makefile

 should be # contrib/pg_audit/Makefile for the consistency.

 Good points, will address.

And on top of that the following things should be changed:
- Removal of REGRESS_OPTS which is empty
- Removal of MODULE, which overlaps with MODULE_big
- $(WIN32RES) needs to be added to OBJS for Windows versioning
Please find in the patch attached the fixes needed.
-- 
Michael
diff --git a/contrib/pg_audit/Makefile b/contrib/pg_audit/Makefile
index bd6897e..76c2cd1 100644
--- a/contrib/pg_audit/Makefile
+++ b/contrib/pg_audit/Makefile
@@ -1,13 +1,13 @@
-# pg_audit/Makefile
+# contrib/pg_audit/Makefile
 
-MODULE = pg_audit
 MODULE_big = pg_audit
-OBJS = pg_audit.o
+OBJS = pg_audit.o $(WIN32RES)
 
 EXTENSION = pg_audit
-REGRESS = pg_audit
-REGRESS_OPTS =
 DATA = pg_audit--1.0.0.sql
+PGFILEDESC = pg_audit - facility for object audit logging
+
+REGRESS = pg_audit
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config

-- 
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: Separate block sampling functions

[Michael Paquier]

On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:

 Simon Riggs si...@2ndquadrant.com writes:
  Separate block sampling functions

 This patch broke buildfarm member crake.


 OK, thanks. I missed that amongst the other unrelated failures. Looking now.

This needs a patch to file_text_array_fdw which I think is available here:
https://github.com/adunstan/file_text_array_fdw
Simon, do you mind if I send a pull request?
-- 
Michael


-- 
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: Separate block sampling functions

[Michael Paquier]

On Fri, May 15, 2015 at 12:12 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 04:06, Michael Paquier michael.paqu...@gmail.com wrote:

 On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com
 wrote:
  On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:
 
  Simon Riggs si...@2ndquadrant.com writes:
   Separate block sampling functions
 
  This patch broke buildfarm member crake.
 
 
  OK, thanks. I missed that amongst the other unrelated failures. Looking
  now.

 This needs a patch to file_text_array_fdw which I think is available here:
 https://github.com/adunstan/file_text_array_fdw
 Simon, do you mind if I send a pull request?


 Hmm, guess that explains it then, I was just scratching my head.

 Yes please sort that out.

Sent a patch here:
https://github.com/adunstan/file_text_array_fdw/pull/1
-- 
Michael


-- 
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: Separate block sampling functions

[Michael Paquier]

On Fri, May 15, 2015 at 12:22 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Michael Paquier michael.paqu...@gmail.com writes:
 On Fri, May 15, 2015 at 12:03 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On 15 May 2015 at 03:50, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Separate block sampling functions

 This patch broke buildfarm member crake.

 OK, thanks. I missed that amongst the other unrelated failures. Looking now.

 This needs a patch to file_text_array_fdw which I think is available here:
 https://github.com/adunstan/file_text_array_fdw
 Simon, do you mind if I send a pull request?

 TBH, I think that this patch itself was a bad idea and should be reverted.
 I don't object to changing APIs used by external modules when there's a
 good reason to break them, but having looked at this patch all I see is
 change for the sake of change.  What new functionality have you introduced?

If you look at the TABLESAMPLE patch, separating the block sampling
into a separate facility makes quite some sense.

 Or to put it more baldly: it's likely that you've broken quite a large
 number of third-party FDWs, not just this one.  A lot of people have
 probably copied-and-pasted what was in the contrib FDWs.

make_foreignscan() has been changed as well by 1a8a4e5c..
-- 
Michael


-- 
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: Allow on-the-fly capture of DDL event details

[Michael Paquier]

On Wed, May 13, 2015 at 12:31 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Michael Paquier wrote:

 Mind share more details about that? How would you detect that a given
 test in serial_schedule needs to be considered by test_ddl_deparse
 automatically? WIth a new type of keyword in a schedule file? This
 looks like a different feature to me that's going to need more
 infrastructure...

 Well, I remembered that the list of tests to run can be passed as a
 variable to the make run, so the schedule file is not the best way to go
 about this anyway.  Therefore I have pushed your patch (including the
 .gitignore file)

I would find better the idea to add a new variable, let's say
REGRESS_SCHEDULE to pass a list of schedule paths that will be taken
into account by PGXS and declare them with --schedule (pg_regress can
take multiple schedules), and add logic to parse it in the MSVC files.
No need to add a special case in the MSVC scripts this way.
-- 
Michael


-- 
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: Allow on-the-fly capture of DDL event details

[Michael Paquier]

On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
 Michael Paquier wrote:
 On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera alvhe...@alvh.no-ip.org 
 wrote:
 Not sure what's the real fix here ..

 I think that you should then use MODULE_big instead of MODULES, and
 set OBJS as test_dll_parser.o $(WIN32RES).

Taking it back, listing explicitly the list of tests in the Makefile's
REGRESS works just fine. Patch attached.
-- 
Michael
From aeb75022ba2a8becaab78b55d31d9f54b44f1b80 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Tue, 12 May 2015 12:50:03 +0900
Subject: [PATCH] Fix test_ddl_deparse for MSVC builds

The list of regression tests should be passed explicitly with REGRESS
instead of passing a regression schedule file path.
---
 src/test/modules/test_ddl_deparse/.gitignore   |  2 ++
 src/test/modules/test_ddl_deparse/Makefile | 25 +-
 src/test/modules/test_ddl_deparse/regress_schedule | 21 --
 3 files changed, 26 insertions(+), 22 deletions(-)
 create mode 100644 src/test/modules/test_ddl_deparse/.gitignore
 delete mode 100644 src/test/modules/test_ddl_deparse/regress_schedule

diff --git a/src/test/modules/test_ddl_deparse/.gitignore b/src/test/modules/test_ddl_deparse/.gitignore
new file mode 100644
index 000..19b6c5b
--- /dev/null
+++ b/src/test/modules/test_ddl_deparse/.gitignore
@@ -0,0 +1,2 @@
+# Generated subdirectories
+/results/
diff --git a/src/test/modules/test_ddl_deparse/Makefile b/src/test/modules/test_ddl_deparse/Makefile
index a87b691..25ecedf 100644
--- a/src/test/modules/test_ddl_deparse/Makefile
+++ b/src/test/modules/test_ddl_deparse/Makefile
@@ -1,10 +1,33 @@
+# src/test/modules/test_ddl_deparse/Makefile
+
 MODULES = test_ddl_deparse
 PGFILEDESC = test_ddl_deparse - regression testing for DDL deparsing
 
 EXTENSION = test_ddl_deparse
 DATA = test_ddl_deparse--1.0.sql
 
-REGRESS = --schedule=$(srcdir)/regress_schedule
+# test_ddl_deparse must be first
+REGRESS = \
+	test_ddl_deparse \
+	create_extension \
+	create_schema \
+	create_type \
+	create_conversion \
+	create_domain \
+	create_sequence_1 \
+	create_table \
+	alter_table \
+	create_view \
+	create_trigger \
+	create_rule \
+	comment_on \
+	alter_function \
+	alter_sequence \
+	alter_type_enum \
+	opfamily \
+	defprivs \
+	matviews
+
 EXTRA_INSTALL = contrib/pg_stat_statements
 
 ifdef USE_PGXS
diff --git a/src/test/modules/test_ddl_deparse/regress_schedule b/src/test/modules/test_ddl_deparse/regress_schedule
deleted file mode 100644
index 1819ae5..000
--- a/src/test/modules/test_ddl_deparse/regress_schedule
+++ /dev/null
@@ -1,21 +0,0 @@
-# must be first
-test: test_ddl_deparse
-
-test: create_extension
-test: create_schema
-test: create_type
-test: create_conversion
-test: create_domain
-test: create_sequence_1
-test: create_table
-test: alter_table
-test: create_view
-test: create_trigger
-test: create_rule
-test: comment_on
-test: alter_function
-test: alter_sequence
-test: alter_type_enum
-test: opfamily
-test: defprivs
-test: matviews
-- 
2.4.0


-- 
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: Allow on-the-fly capture of DDL event details

[Michael Paquier]

On Tue, May 12, 2015 at 1:05 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Michael Paquier wrote:
 On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
 michael.paqu...@gmail.com wrote:
  On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
  alvhe...@2ndquadrant.com wrote:
  Michael Paquier wrote:
  On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera 
  alvhe...@alvh.no-ip.org wrote:
  Not sure what's the real fix here ..
 
  I think that you should then use MODULE_big instead of MODULES, and
  set OBJS as test_dll_parser.o $(WIN32RES).

 Taking it back, listing explicitly the list of tests in the Makefile's
 REGRESS works just fine. Patch attached.

 Sure.  I want to avoid doing that, though: we may want to generate a
 schedule based on src/test/regress/serial_schedule, so that newly added
 tests to the regular suite are automatically considered by this module.

Mind share more details about that? How would you detect that a given
test in serial_schedule needs to be considered by test_ddl_deparse
automatically? WIth a new type of keyword in a schedule file? This
looks like a different feature to me that's going to need more
infrastructure...

If you want to continue on this way though, I imagine that you will
need to add a special case in fetchTests of vcregress.pl. In any case,
it is not good to keep the buildfarm machines broken for too long, and
personally I would rather avoid adding one more hack in the MSVC build
scripts.
-- 
Michael


-- 
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: Allow on-the-fly capture of DDL event details

[Michael Paquier]

On Tue, May 12, 2015 at 1:18 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Tue, May 12, 2015 at 1:05 PM, Alvaro Herrera
 alvhe...@2ndquadrant.com wrote:
 Michael Paquier wrote:
 On Tue, May 12, 2015 at 11:43 AM, Michael Paquier
 michael.paqu...@gmail.com wrote:
  On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
  alvhe...@2ndquadrant.com wrote:
  Michael Paquier wrote:
  On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera 
  alvhe...@alvh.no-ip.org wrote:
  Not sure what's the real fix here ..
 
  I think that you should then use MODULE_big instead of MODULES, and
  set OBJS as test_dll_parser.o $(WIN32RES).

 Taking it back, listing explicitly the list of tests in the Makefile's
 REGRESS works just fine. Patch attached.

 Sure.  I want to avoid doing that, though: we may want to generate a
 schedule based on src/test/regress/serial_schedule, so that newly added
 tests to the regular suite are automatically considered by this module.

 Mind share more details about that? How would you detect that a given
 test in serial_schedule needs to be considered by test_ddl_deparse
 automatically? WIth a new type of keyword in a schedule file? This
 looks like a different feature to me that's going to need more
 infrastructure...

 If you want to continue on this way though, I imagine that you will
 need to add a special case in fetchTests of vcregress.pl. In any case,
 it is not good to keep the buildfarm machines broken for too long, and
 personally I would rather avoid adding one more hack in the MSVC build
 scripts.

Btw, just adding that a .gitignore file with /results/ is missing in
src/test/modules/test_ddl_deparse.
-- 
Michael


-- 
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: Allow on-the-fly capture of DDL event details

[Michael Paquier]

On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera alvhe...@alvh.no-ip.org wrote:
 Allow on-the-fly capture of DDL event details

 This feature lets user code inspect and take action on DDL events.
 Whenever a ddl_command_end event trigger is installed, DDL actions
 executed are saved to a list which can be inspected during execution of
 a function attached to ddl_command_end.

Buildfarm machines on Windows are complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mastodondt=2015-05-11%2023%3A00%3A01

Visibly REGRESS and REGRESS_OPTS are messed up in the Makefile of
test_ddl_deparsing. The patch attached make tests pass correctly.
-- 
Michael
diff --git a/src/test/modules/test_ddl_deparse/Makefile b/src/test/modules/test_ddl_deparse/Makefile
index a87b691..d025b49 100644
--- a/src/test/modules/test_ddl_deparse/Makefile
+++ b/src/test/modules/test_ddl_deparse/Makefile
@@ -1,10 +1,13 @@
+# src/test/modules/test_ddl_deparse/Makefile
+
 MODULES = test_ddl_deparse
 PGFILEDESC = test_ddl_deparse - regression testing for DDL deparsing
 
 EXTENSION = test_ddl_deparse
 DATA = test_ddl_deparse--1.0.sql
 
-REGRESS = --schedule=$(srcdir)/regress_schedule
+REGRESS = test_ddl_deparse
+REGRESS_OPTS = --schedule=$(srcdir)/regress_schedule
 EXTRA_INSTALL = contrib/pg_stat_statements
 
 ifdef USE_PGXS

-- 
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: Allow on-the-fly capture of DDL event details

[Michael Paquier]

On Tue, May 12, 2015 at 11:37 AM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Michael Paquier wrote:
 On Tue, May 12, 2015 at 7:16 AM, Alvaro Herrera alvhe...@alvh.no-ip.org 
 wrote:
  Allow on-the-fly capture of DDL event details
 
  This feature lets user code inspect and take action on DDL events.
  Whenever a ddl_command_end event trigger is installed, DDL actions
  executed are saved to a list which can be inspected during execution of
  a function attached to ddl_command_end.

 Buildfarm machines on Windows are complaining:
 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mastodondt=2015-05-11%2023%3A00%3A01

 Visibly REGRESS and REGRESS_OPTS are messed up in the Makefile of
 test_ddl_deparsing. The patch attached make tests pass correctly.

 Um.  In my system, if I set REGRESS to test_ddl_deparse per your patch,
 the test_ddl_deparse test is run twice, at the beginning (per the
 schedule file) and at the end, and it fails on the second run.

 I tried setting REGRESS to empty, and to leave it unset, but that only
 causes make to say nothing to be done for installcheck which isn't
 good either.  I suppose I could delete the last line from the schedule
 file and put it in the REGRESS line in the Makefile, but that seems
 wrong too.

 Not sure what's the real fix here ..

I think that you should then use MODULE_big instead of MODULES, and
set OBJS as test_dll_parser.o $(WIN32RES).
-- 
Michael


-- 
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: Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.

[Michael Paquier]

On Fri, May 8, 2015 at 1:09 PM, Andres Freund and...@anarazel.de wrote:
 On May 7, 2015 9:07:55 PM PDT, Michael Paquier michael.paqu...@gmail.com 
 wrote:
On Fri, May 8, 2015 at 12:43 PM, Andres Freund and...@anarazel.de
wrote:
 Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.

magpie is complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=magpiedt=2015-05-08%2003%3A45%3A15
And I can reproduce the failure as well on my OSX laptop at least.

 Pushed something that hopefully fixes that already. Works for you?

That's fine. Thanks for the quick fix.
-- 
Michael


-- 
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: Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.

[Michael Paquier]

On Fri, May 8, 2015 at 12:43 PM, Andres Freund and...@anarazel.de wrote:
 Add support for INSERT ... ON CONFLICT DO NOTHING/UPDATE.

magpie is complaining:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=magpiedt=2015-05-08%2003%3A45%3A15
And I can reproduce the failure as well on my OSX laptop at least.
-- 
Michael


-- 
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: Move interpreter shared library detection to configure

[Michael Paquier]

On Sat, May 2, 2015 at 10:39 AM, Peter Eisentraut pete...@gmx.net wrote:
 Move interpreter shared library detection to configure

 For building PL/Perl, PL/Python, and PL/Tcl, we need a shared library of
 libperl, libpython, and libtcl, respectively.  Previously, this was
 checked in the makefiles, skipping the PL build with a warning if no
 shared library was available.  Now this is checked in configure, with an
 error if no shared library is available.

 The previous situation arose because in the olden days, the configure
 options --with-perl, --with-python, and --with-tcl controlled whether
 frontend interfaces for those languages would be built.  The procedural
 languages were added later, and shared libraries were often not
 available in the beginning.  So it was decided skip the builds of the
 procedural languages in those cases.  The frontend interfaces have since
 been removed from the tree, and shared libraries are now available most
 of the time, so that setup makes much less sense now.

 Also, the new setup allows contrib modules and pgxs users to rely on the
 respective PLs being available based on configure flags.

This is hurting OSX platforms at least older than 10.6 (I can get
configure working on my 10.8 laptop), and OpenBSD.
-- 
Michael


-- 
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: Copy editing of the replication origins patch.

[Michael Paquier]

On Fri, May 1, 2015 at 7:26 PM, Andres Freund and...@anarazel.de wrote:
 Copy editing of the replication origins patch.

 Michael Paquier and myself.

-* Needs to fit into a uint16, so we don't waste too much space in WAL
+* Needs to fit into an uint16, so we don't waste too much space in WAL
Actually this one should not have been changed I think, an should not
be used in front of u when it is pronounced you (Memories of
English classes at school).
-- 
Michael


-- 
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: Introduce replication progress tracking infrastructure.

[Michael Paquier]

On Thu, Apr 30, 2015 at 2:37 AM, Andres Freund and...@anarazel.de wrote:
 Introduce replication progress tracking infrastructure.
 [...]

Some comments about the docs:
1) the the:
+   entry
+Create a replication origin with the the passed in external
+name, and create an internal id for it.
+   /entry
+  /row
2) Missing markup type for oid.
+   entry
+Lookup replication origin by name and return the internal
+oid. If no corresponding replication origin is found a error
+is thrown.
+   /entry
+  /row
3) Perhaps Check that a replication has been configured in the
current session instead of using a question?
+   entry
+Has a replication origin been configured in the current session?
+   /entry
4) Missing markup type for oid?
+  Replication origins consist out of a name and a oid. The name, which
5) will persist or will be persistent, not will be persist I guess.
+  If that's done replication progress will be persist in a crash safe
6) The use of that, that looks weird to me. There should be only one.
+  system to one other, another problem can be that, that it is hard to avoid

Regards,
-- 
Michael


-- 
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: Add transforms feature

[Michael Paquier]

On Tue, Apr 28, 2015 at 10:01 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Tue, Apr 28, 2015 at 9:55 AM, Andrew Dunstan wrote:
 w.r.t MSVC builds, it looks like we need entries in $contrib_extraincludes
 in src/tools/msvc/Mkvcbuild.pm at the very least.

 If our goal is to put back to green the Windows nodes as quick as
 possible, we could bypass their build this way , and we'll
 additionally need to update the install script and
 vcregress.pl/contribcheck to bypass those modules accordingly. Now I
 don't think that we should make the things produced inconsistent.

OK, attached are two patches to put back the buildfarm nodes using MSVC to green
- 0001 adds support for build and installation of the new transform
modules: hstore_plperl, hstore_plpython and  ltree_plpython. Note that
this patch is enough to put back the buildfarm to a green state for
MSVC, but it disables the regression tests for those modules,
something addressed in the next patch...
- 0002 adds support for regression tests for the new modules. The
thing is that we need to check the major version version of Python
available in configuration and choose what are the extensions to
preload before the tests run. It is a little bit hacky... But it has
the merit to work, and I am not sure we could have a cleaner solution
by looking at the Makefiles of the transform modules that use
currently conditions based on $(python_majorversion).
Regards,
-- 
Michael
From dedbdb5815b3b0450fcf6991156eb8ae022bd04a Mon Sep 17 00:00:00 2001
From: Michael Paquier michael@otacoo.com
Date: Tue, 28 Apr 2015 02:13:47 -0700
Subject: [PATCH 1/2] Fix MSVC build for transform modules

The following contrib/ modules have their build fixed in the MSVC scripts:
- hstore_plperl
- hstore_plpython
- ltree_plpython

With this patch the MSVC build and installation will work correctly with
the transforms. However their test is still disabled as some adjustments
are needed for plpython transforms.
---
 src/tools/msvc/Install.pm   |   9 +-
 src/tools/msvc/Mkvcbuild.pm | 314 
 src/tools/msvc/vcregress.pl |  10 +-
 3 files changed, 212 insertions(+), 121 deletions(-)

diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index bfcdf50..b617835 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -439,9 +439,12 @@ sub CopyContribFiles
 		while (my $d = readdir($D))
 		{
 			# These configuration-based exclusions must match vcregress.pl
-			next if ($d eq uuid-ossp  !defined($config-{uuid}));
-			next if ($d eq sslinfo!defined($config-{openssl}));
-			next if ($d eq xml2   !defined($config-{xml}));
+			next if ($d eq uuid-ossp!defined($config-{uuid}));
+			next if ($d eq sslinfo  !defined($config-{openssl}));
+			next if ($d eq xml2 !defined($config-{xml}));
+			next if ($d eq hstore_plperl!defined($config-{perl}));
+			next if ($d eq hstore_plpython  !defined($config-{python}));
+			next if ($d eq ltree_plpython   !defined($config-{python}));
 			next if ($d eq sepgsql);
 
 			CopySubdirFiles($subdir, $d, $config, $target);
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index cfb9b24..9d66368 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -46,7 +46,11 @@ my $contrib_extraincludes =
 my $contrib_extrasource = {
 	'cube' = [ 'contrib/cube/cubescan.l', 'contrib/cube/cubeparse.y' ],
 	'seg' = [ 'contrib/seg/segscan.l', 'contrib/seg/segparse.y' ], };
-my @contrib_excludes = ('pgcrypto', 'commit_ts', 'intagg', 'sepgsql');
+my @contrib_excludes = (
+	'commit_ts', 'hstore_plperl',
+	'hstore_plpython', 'intagg',
+	'ltree_plpython', 'pgcrypto',
+	'sepgsql');
 
 # Set of variables for frontend modules
 my $frontend_defines = { 'initdb' = 'FRONTEND' };
@@ -176,119 +180,6 @@ sub mkvcbuild
 	$plpgsql-AddFiles('src/pl/plpgsql/src', 'pl_gram.y');
 	$plpgsql-AddReference($postgres);
 
-	if ($solution-{options}-{perl})
-	{
-		my $plperlsrc = src/pl/plperl/;
-		my $plperl =
-		  $solution-AddProject('plperl', 'dll', 'PLs', 'src/pl/plperl');
-		$plperl-AddIncludeDir($solution-{options}-{perl} . '/lib/CORE');
-		$plperl-AddDefine('PLPERL_HAVE_UID_GID');
-		foreach my $xs ('SPI.xs', 'Util.xs')
-		{
-			(my $xsc = $xs) =~ s/\.xs/.c/;
-			if (Solution::IsNewer($plperlsrc$xsc, $plperlsrc$xs))
-			{
-my $xsubppdir = first { -e $_/ExtUtils/xsubpp } @INC;
-print Building $plperlsrc$xsc...\n;
-system( $solution-{options}-{perl}
-	  . '/bin/perl '
-	  . $xsubppdir/ExtUtils/xsubpp -typemap 
-	  . $solution-{options}-{perl}
-	  . '/lib/ExtUtils/typemap '
-	  . $plperlsrc$xs 
-	  . $plperlsrc$xsc);
-if ((!(-f $plperlsrc$xsc)) || -z $plperlsrc$xsc)
-{
-	unlink($plperlsrc$xsc);# if zero size
-	die Failed to create $xsc.\n;
-}
-			}
-		}
-		if (Solution::IsNewer(
-'src/pl/plperl/perlchunks.h',
-'src/pl/plperl/plc_perlboot.pl')
-			|| Solution::IsNewer(
-'src/pl/plperl

Re: [COMMITTERS] pgsql: Add transforms feature

[Michael Paquier]

On Mon, Apr 27, 2015 at 1:36 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Peter Eisentraut pete...@gmx.net writes:
 Add transforms feature

 I don't know why this patch is fooling around with compile/link flags,
 but it's broken at least prairiedog and some of the Windows critters.

It breaks as well on all the Windows machines using MS or VC toolchains...
-- 
Michael


-- 
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: Add transforms feature

[Michael Paquier]

On Tue, Apr 28, 2015 at 9:55 AM, Andrew Dunstan wrote:
 w.r.t MSVC builds, it looks like we need entries in $contrib_extraincludes
 in src/tools/msvc/Mkvcbuild.pm at the very least.

If our goal is to put back to green the Windows nodes as quick as
possible, we could bypass their build this way , and we'll
additionally need to update the install script and
vcregress.pl/contribcheck to bypass those modules accordingly. Now I
don't think that we should make the things produced inconsistent.
-- 
Michael


-- 
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 the pg_rewind regression tests more robust on slow systems.

[Michael Paquier]

On Wed, Apr 22, 2015 at 8:37 PM, Heikki Linnakangas
heikki.linnakan...@iki.fi wrote:
 Make the pg_rewind regression tests more robust on slow systems.

 There were a couple of hard-coded sleeps in the tests: to wait for standby
 to catch up with master, and to wait for promotion with pg_ctl promote
 to complete. Instead of a fixed, hard-coded sleep, poll the server with a
 query once a second. This isn't ideal either, and I wish we had a better
 solution for real-world applications too, but this should fix the
 immediate problem.

Just wondering: why not creating a common routine in TestLib.pm that
returns to the caller the output of stdout, stderr and the exit code?
That would be useful for other tests as well, and would help limiting
the dependency of IPC::Run into TestLib.pm.
-- 
Michael


-- 
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: Add missing installcheck target to pg_rewind's Makefile

[Michael Paquier]

On Wed, Apr 22, 2015 at 5:38 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 (I chanced to notice this while wondering why hamster is failing on
 this test.  I can't reproduce the failure locally, but ...)


With a very slow environment you will be able to reproduce the failure.
-- 
Michael

Re: [COMMITTERS] pgsql: Move pg_test_fsync from contrib/ to src/bin/

[Michael Paquier]

On Mon, Apr 20, 2015 at 11:39 AM, Peter Eisentraut pete...@gmx.net wrote:
 Move pg_test_fsync from contrib/ to src/bin/

Point of detail that I just noticed: wouldn't it be better to have a
header in pg_test_fsync.c of a shape similar to the other files. Now
there is only that:
/*
 *pg_test_fsync.c
 *tests all supported fsync() methods
 */
An idea what this gives as a patch is attached.
-- 
Michael
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index c842762..54e7871 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -1,6 +1,12 @@
-/*
- *	pg_test_fsync.c
- *		tests all supported fsync() methods
+/*-
+ *
+ * pg_test_fsync --- tests all supported fsync() methods
+ *
+ * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
+ *
+ * src/bin/pg_test_fsync/pg_test_fsync.c
+ *
+ *-
  */
 
 #include postgres_fe.h

-- 
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: Mark the second argument of pg_log as the translatable string in

[Michael Paquier]

On Wed, Apr 15, 2015 at 4:55 PM, Heikki Linnakangas wrote:
 On 04/15/2015 06:41 AM, Fujii Masao wrote:
 Another question is; should we output the progress report to stderr rather
 than stdout? I thought this because I found that pg_basebackup reports
 the progress to stderr.


 Yeah, probably. We should go through all the output and figure out where
 each kind of message needs to do. Should follow the principle Alvaro laid
 out
 (http://www.postgresql.org/message-id/20150407205320.gn4...@alvh.no-ip.org),
 and also make sure it's consistent with pg_basebackup and other tools.
 Michael's patch changed some of the logging but we should take a more
 holistic look at the situation. And add a comment somewhere explaining the
 principle.

Isn't what we are looking for here a common set of frontend-only APIs,
let's say as src/common/logging.c? All the tools we have could use it
without knowing if they output on stdout and stderr.
-- 
Michael


-- 
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: Error out in pg_rewind if lstat() fails.

[Michael Paquier]

On Thu, Apr 16, 2015 at 5:15 AM, Heikki Linnakangas
heikki.linnakan...@iki.fi wrote:
 Error out in pg_rewind if lstat() fails.

 A file not found is expected if the source server is running, so don't
 complain about that. But any other error is definitely not expected.

Nitpicking: strings of pg_fatal need to have a newline '\n' for readability.
-- 
Michael


-- 
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 multiple bugs and infelicities in pg_rewind.

[Michael Paquier]

On Mon, Mar 30, 2015 at 9:02 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Fix multiple bugs and infelicities in pg_rewind.

 Bugs all spotted by Coverity, including wrong realloc() size request
 and memory leaks.  Cosmetic improvements by me.

 The usage of the global variable filemap here is still pretty awful,
 but at least I got rid of the gratuitous aliasing in several routines
 (which was helping to annoy Coverity, as well as being a bug risk).

Coverity points out that a call to PQclear() in receiveFileChunks of
libpq_fetch.c is missing as the result still leaks resources when
status is PGRES_TUPLES_OK. Please see the patch attached.
-- 
Michael
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 23c971a..e696554 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -231,6 +231,7 @@ receiveFileChunks(const char *sql)
 break;
 
 			case PGRES_TUPLES_OK:
+PQclear(res);
 continue;		/* final zero-row result */
 
 			default:

-- 
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: Mark the second argument of pg_log as the translatable string in

[Michael Paquier]

On Sun, Apr 12, 2015 at 10:17 AM, Alvaro Herrera wrote:
 What pg_basebackup's progress_report() does is have the message in the
 translatable part not include the \r; the \r is in a separate fprintf()
 call.

Like the attached then.
-- 
Michael
diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
index aba12d8..3e2dc76 100644
--- a/src/bin/pg_rewind/logging.c
+++ b/src/bin/pg_rewind/logging.c
@@ -134,7 +134,8 @@ progress_report(bool force)
 	snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
 			 fetch_size / 1024);
 
-	pg_log(PG_PROGRESS, %*s/%s kB (%d%%) copied\r,
+	pg_log(PG_PROGRESS, %*s/%s kB (%d%%) copied,
 		   (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
 		   percent);
+	printf(\r);
 }

-- 
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: Mark the second argument of pg_log as the translatable string in

[Michael Paquier]

On Wed, Apr 8, 2015 at 11:06 AM, Fujii Masao fu...@postgresql.org wrote:
 Mark the second argument of pg_log as the translatable string in nls.mk.

nls.mk is still missing file_ops.c in GETTEXT_FILES as it contains
some calls to pg_fatal.
-- 
Michael


-- 
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: psql: fix \connect with URIs and conninfo strings

[Michael Paquier]

On Thu, Apr 2, 2015 at 10:29 AM, Michael Paquier michael.paqu...@gmail.com
wrote:




 On Thu, Apr 2, 2015 at 8:10 AM, Alvaro Herrera alvhe...@alvh.no-ip.org
 wrote:

 psql: fix \connect with URIs and conninfo strings

 psql was already accepting conninfo strings as the first parameter in
 \connect, but the way it worked wasn't sane; some of the other
 parameters would get the previous connection's values, causing it to
 connect to a completely unexpected server or, more likely, not finding
 any server at all because of completely wrong combinations of
 parameters.


 This patch has broken the build on OSX:

 http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedarydt=2015-04-02%2000%3A10%3A54
 My box complains as well.

 I haven't looked at that in details, but it looks that there are linking
 problems with -lpgcommon, as it is the first time that libpq has a
 dependency with it.

 I am seeing as well that this is at least missing at the top of
 connstring.c:
 #ifndef FRONTEND
 #error This file is not expected to be compiled for backend code
 #endif
 And that src/tools/msvc has not been updated to have connstring.c show up
 in the list of frontend-only objects for libpgcommon.


On other Linux machines, tests for dblink are failing:
+ ERROR: could not load library
/usr/src/pgfarm/build/HEAD/inst/lib/postgresql/dblink.so:
/usr/src/pgfarm/build/HEAD/inst/lib/libpq.so.5: undefined symbol:
libpq_connstring_is_recognized
-- 
Michael

Re: [COMMITTERS] pgsql: psql: fix \connect with URIs and conninfo strings

[Michael Paquier]

On Thu, Apr 2, 2015 at 10:58 AM, Michael Paquier michael.paqu...@gmail.com
wrote:

 On other Linux machines, tests for dblink are failing:
 + ERROR: could not load library
 /usr/src/pgfarm/build/HEAD/inst/lib/postgresql/dblink.so:
 /usr/src/pgfarm/build/HEAD/inst/lib/libpq.so.5: undefined symbol:
 libpq_connstring_is_recognized


The patch attached fixes all those inconsistencies (tested build on OSX and
Windows).
-- 
Michael
From 188b8e51df428e650d9cc23b4a67d7516a1e5e47 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Thu, 2 Apr 2015 11:13:42 +0900
Subject: [PATCH] Fix libpq build errors caused by fcef1617

libpq build was missing a reference to the new file connstrings.c,
leading to build errors on OSX and Windows, and regression test errors
for dblink as it tried to load a version of libpq missing dependencies.
---
 src/common/connstrings.c| 4 
 src/interfaces/libpq/.gitignore | 1 +
 src/interfaces/libpq/Makefile   | 7 ++-
 src/tools/msvc/Mkvcbuild.pm | 4 ++--
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/common/connstrings.c b/src/common/connstrings.c
index 91170a1..ad714ab 100644
--- a/src/common/connstrings.c
+++ b/src/common/connstrings.c
@@ -6,6 +6,10 @@
  *
  *	src/include/common/connstrings.c
  */
+#ifndef FRONTEND
+#error This file is not expected to be compiled for backend code
+#endif
+
 #include postgres_fe.h
 
 #include string.h
diff --git a/src/interfaces/libpq/.gitignore b/src/interfaces/libpq/.gitignore
index cb96af7..a28dff9 100644
--- a/src/interfaces/libpq/.gitignore
+++ b/src/interfaces/libpq/.gitignore
@@ -1,5 +1,6 @@
 /exports.list
 /chklocale.c
+/connstrings.c
 /crypt.c
 /getaddrinfo.c
 /getpeereid.c
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 6973a20..513f981 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -27,7 +27,7 @@ endif
 # Need to recompile any external C files because we need
 # all object files to use the same compile flags as libpq; some
 # platforms require special flags.
-LIBS := $(LIBS:-lpgport=)
+LIBS := $(filter -lpgport -lpgcommon, $(LIBS))
 
 # We can't use Makefile variables here because the MSVC build system scrapes
 # OBJS from this file.
@@ -43,6 +43,8 @@ OBJS += $(filter crypt.o getaddrinfo.o getpeereid.o inet_aton.o open.o system.o
 OBJS += ip.o md5.o
 # utils/mb
 OBJS += encnames.o wchar.o
+# common/
+OBJS += connstrings.o
 
 ifeq ($(with_openssl),yes)
 OBJS += fe-secure-openssl.o
@@ -96,6 +98,9 @@ backend_src = $(top_srcdir)/src/backend
 chklocale.c crypt.c getaddrinfo.c getpeereid.c inet_aton.c inet_net_ntop.c noblock.c open.c system.c pgsleep.c pgstrcasecmp.c pqsignal.c snprintf.c strerror.c strlcpy.c thread.c win32error.c win32setlocale.c: % : $(top_srcdir)/src/port/%
 	rm -f $@  $(LN_S) $ .
 
+connstrings.c: % : $(top_srcdir)/src/common/%
+	rm -f $@  $(LN_S) $ .
+
 ip.c md5.c: % : $(backend_src)/libpq/%
 	rm -f $@  $(LN_S) $ .
 
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 7f319df..5175d30 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -95,8 +95,8 @@ sub mkvcbuild
 	  exec.c pg_crc.c pg_lzcompress.c pgfnames.c psprintf.c relpath.c rmtree.c
 	  string.c username.c wait_error.c);
 
-	our @pgcommonfrontendfiles = (@pgcommonallfiles, qw(fe_memutils.c
-	  restricted_token.c));
+	our @pgcommonfrontendfiles = (@pgcommonallfiles, qw(connstrings.c
+	  fe_memutils.c restricted_token.c));
 
 	our @pgcommonbkndfiles = @pgcommonallfiles;
 
-- 
2.3.5


-- 
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: psql: fix \connect with URIs and conninfo strings

[Michael Paquier]

On Thu, Apr 2, 2015 at 8:10 AM, Alvaro Herrera alvhe...@alvh.no-ip.org
wrote:

 psql: fix \connect with URIs and conninfo strings

 psql was already accepting conninfo strings as the first parameter in
 \connect, but the way it worked wasn't sane; some of the other
 parameters would get the previous connection's values, causing it to
 connect to a completely unexpected server or, more likely, not finding
 any server at all because of completely wrong combinations of
 parameters.


This patch has broken the build on OSX:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dromedarydt=2015-04-02%2000%3A10%3A54
My box complains as well.

I haven't looked at that in details, but it looks that there are linking
problems with -lpgcommon, as it is the first time that libpq has a
dependency with it.

I am seeing as well that this is at least missing at the top of
connstring.c:
#ifndef FRONTEND
#error This file is not expected to be compiled for backend code
#endif
And that src/tools/msvc has not been updated to have connstring.c show up
in the list of frontend-only objects for libpgcommon.
-- 
Michael

Re: [COMMITTERS] pgsql: psql: fix \connect with URIs and conninfo strings

[Michael Paquier]

On Thu, Apr 2, 2015 at 11:49 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 Michael Paquier michael.paqu...@gmail.com writes:
  The patch attached fixes all those inconsistencies (tested build on OSX
 and
  Windows).

 I think this is going in the wrong direction entirely, ie doubling down
 on Alvaro's original mistake.  libpq *must not* depend on libpgcommon,
 because the latter is not compiled to be position-independent code
 (on machines where that matters).


Hm, OK. I was not aware of that.


 Furthermore, proposing to add this:

 +#ifndef FRONTEND
 +#error This file is not expected to be compiled for backend code
 +#endif

 seems to me to prove that connstrings.c didn't belong in src/common
 in the first place.

 I'm too tired to think through exactly what this should be like instead,
 but we do have rules about what goes where, and the response to violating
 those rules should not be to break down the divisions even more.


libpgport sounds like the only place then. I guess that it would be right
to revert this patch on master and replace it with a version similar to
what has been done in backbranches for now, and look again at this
refactoring stuff...
-- 
Michael

Re: [COMMITTERS] pgsql: Centralize definition of integer limits.

[Michael Paquier]

On Mon, Mar 30, 2015 at 7:48 PM, Andres Freund and...@anarazel.de wrote:

 Hi,

 On 2015-03-30 14:01:25 +0900, Michael Paquier wrote:
  On Thu, Mar 26, 2015 at 6:49 AM, Andres Freund and...@anarazel.de
 wrote:
   Centralize definition of integer limits.
  
   Several submitted and even committed patches have run into the problem
   that C89, our baseline, does not provide minimum/maximum values for
   various integer datatypes. C99's stdint.h does, but we can't rely on
   it.
  
   Several parts of the code defined limits locally, so instead centralize
   the definitions to c.h.
  
   This patch also changes the more obvious usages of literal limit
 values;
   there's more places that could be changed, but it's less clear whether
   it's beneficial to change those.
 
  My OSX dev box is generating a couple of warnings since this commit:
  pg_dump.c:14548:45: warning: format specifies type 'long' but the
 argument
  has type 'long long' [-Wformat]
  snprintf(bufm, sizeof(bufm), INT64_FORMAT, SEQ_MINVALUE);
  ~~~^
  ../../../src/include/pg_config_manual.h:52:22: note: expanded from macro
  'SEQ_MINVALUE'
  #define SEQ_MINVALUE(-SEQ_MAXVALUE)
  ^
  /usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
__builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
   ^
  pg_dump.c:14549:45: warning: format specifies type 'long' but the
 argument
  has type 'long long' [-Wformat]
  snprintf(bufx, sizeof(bufx), INT64_FORMAT, SEQ_MAXVALUE);
  ~~~^~~~
  ../../../src/include/pg_config_manual.h:51:22: note: expanded from macro
  'SEQ_MAXVALUE'
  #define SEQ_MAXVALUEINT64_MAX
  ^
  /usr/include/stdint.h:122:26: note: expanded from macro 'INT64_MAX'
  #define INT64_MAX9223372036854775807LL
   ^
  /usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
__builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)

 Hm. Can you send config.log and the generated pg_config.h?


Sure.
-- 
Michael


configs.tar.gz
Description: GNU Zip compressed 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: Centralize definition of integer limits.

[Michael Paquier]

On Thu, Mar 26, 2015 at 6:49 AM, Andres Freund and...@anarazel.de wrote:
 Centralize definition of integer limits.

 Several submitted and even committed patches have run into the problem
 that C89, our baseline, does not provide minimum/maximum values for
 various integer datatypes. C99's stdint.h does, but we can't rely on
 it.

 Several parts of the code defined limits locally, so instead centralize
 the definitions to c.h.

 This patch also changes the more obvious usages of literal limit values;
 there's more places that could be changed, but it's less clear whether
 it's beneficial to change those.

My OSX dev box is generating a couple of warnings since this commit:
pg_dump.c:14548:45: warning: format specifies type 'long' but the argument
has type 'long long' [-Wformat]
snprintf(bufm, sizeof(bufm), INT64_FORMAT, SEQ_MINVALUE);
~~~^
../../../src/include/pg_config_manual.h:52:22: note: expanded from macro
'SEQ_MINVALUE'
#define SEQ_MINVALUE(-SEQ_MAXVALUE)
^
/usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
  __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
 ^
pg_dump.c:14549:45: warning: format specifies type 'long' but the argument
has type 'long long' [-Wformat]
snprintf(bufx, sizeof(bufx), INT64_FORMAT, SEQ_MAXVALUE);
~~~^~~~
../../../src/include/pg_config_manual.h:51:22: note: expanded from macro
'SEQ_MAXVALUE'
#define SEQ_MAXVALUEINT64_MAX
^
/usr/include/stdint.h:122:26: note: expanded from macro 'INT64_MAX'
#define INT64_MAX9223372036854775807LL
 ^
/usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
  __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)

Thoughts?
-- 
Michael

Re: [COMMITTERS] pgsql: Centralize definition of integer limits.

[Michael Paquier]

On Mon, Mar 30, 2015 at 2:01 PM, Michael Paquier michael.paqu...@gmail.com
wrote:




 On Thu, Mar 26, 2015 at 6:49 AM, Andres Freund and...@anarazel.de wrote:
  Centralize definition of integer limits.
 
  Several submitted and even committed patches have run into the problem
  that C89, our baseline, does not provide minimum/maximum values for
  various integer datatypes. C99's stdint.h does, but we can't rely on
  it.
 
  Several parts of the code defined limits locally, so instead centralize
  the definitions to c.h.
 
  This patch also changes the more obvious usages of literal limit values;
  there's more places that could be changed, but it's less clear whether
  it's beneficial to change those.

 My OSX dev box is generating a couple of warnings since this commit:
 pg_dump.c:14548:45: warning: format specifies type 'long' but the argument
 has type 'long long' [-Wformat]
 snprintf(bufm, sizeof(bufm), INT64_FORMAT, SEQ_MINVALUE);
 ~~~^
 ../../../src/include/pg_config_manual.h:52:22: note: expanded from macro
 'SEQ_MINVALUE'
 #define SEQ_MINVALUE(-SEQ_MAXVALUE)
 ^
 /usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
   __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)
  ^
 pg_dump.c:14549:45: warning: format specifies type 'long' but the argument
 has type 'long long' [-Wformat]
 snprintf(bufx, sizeof(bufx), INT64_FORMAT, SEQ_MAXVALUE);
 ~~~^~~~
 ../../../src/include/pg_config_manual.h:51:22: note: expanded from macro
 'SEQ_MAXVALUE'
 #define SEQ_MAXVALUEINT64_MAX
 ^
 /usr/include/stdint.h:122:26: note: expanded from macro 'INT64_MAX'
 #define INT64_MAX9223372036854775807LL
  ^
 /usr/include/secure/_stdio.h:56:62: note: expanded from macro 'snprintf'
   __builtin___snprintf_chk (str, len, 0, __darwin_obsz(str), __VA_ARGS__)

 Thoughts?


INT64_MODIFIER is l on OSX, causing this warning. Perhaps there is
something better to do instead of casting blindly to int64. Thoughts?
-- 
Michael
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 7da5c41..a15c875 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -14545,8 +14545,8 @@ dumpSequence(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
 	/* Make sure we are in proper schema */
 	selectSourceSchema(fout, tbinfo-dobj.namespace-dobj.name);
 
-	snprintf(bufm, sizeof(bufm), INT64_FORMAT, SEQ_MINVALUE);
-	snprintf(bufx, sizeof(bufx), INT64_FORMAT, SEQ_MAXVALUE);
+	snprintf(bufm, sizeof(bufm), INT64_FORMAT, (int64) SEQ_MINVALUE);
+	snprintf(bufx, sizeof(bufx), INT64_FORMAT, (int64) SEQ_MAXVALUE);
 
 	if (fout-remoteVersion = 80400)
 	{

-- 
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: Merge the various forms of transaction commit abort records.

[Michael Paquier]

On Mon, Mar 16, 2015 at 1:38 AM, Andres Freund and...@anarazel.de wrote:
 Merge the various forms of transaction commit  abort records.

Coverity is complaining about the following block of code:
+   xact_info = XLogRecGetInfo(record)  XLOG_XACT_COMMIT;
+
+   if (xact_info != XLOG_XACT_COMMIT 
+   xact_info != XLOG_XACT_COMMIT_PREPARED)
return false;

Instead of XLOG_XACT_COMMIT, shouldn't this function use XLOG_XACT_OPMASK?
-- 
Michael


-- 
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: vacuumdb: enable parallel mode

[Michael Paquier]

On Sat, Jan 24, 2015 at 3:06 AM, Alvaro Herrera alvhe...@alvh.no-ip.org wrote:
 vacuumdb: enable parallel mode

 This mode allows vacuumdb to open several server connections to vacuum
 or analyze several tables simultaneously.

Coverity is still complaining about this block of code where the
return code of PQsendQuery() is not checked:
if (async)
{
if (echo)
printf(%s\n, sql);

PQsendQuery(conn, sql);
}
Could it be possible to get that fixed soon? Alvaro, attached is the
patch that you wrote to fix this stuff.
Regards,
-- 
Michael
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 8e4e613..a320b55 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -668,14 +668,21 @@ run_vacuum_command(PGconn *conn, const char *sql, bool echo,
    const char *dbname, const char *table,
    const char *progname, bool async)
 {
+	int		status;
+
 	if (async)
 	{
 		if (echo)
 			printf(%s\n, sql);
 
-		PQsendQuery(conn, sql);
+		status = PQsendQuery(conn, sql);
 	}
-	else if (!executeMaintenanceCommand(conn, sql, echo))
+	else
+	{
+		status = executeMaintenanceCommand(conn, sql, echo);
+	}
+
+	if (!status)
 	{
 		if (table)
 			fprintf(stderr,

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