Thread: Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Mon, Aug 15, 2016 at 12:12 AM, Ryan Murphy <ryanfmurphy@gmail.com> wrote:
> This is to fix an issue that came up for me when running initdb.
>
> At the end of a successful initdb it says:
>
>     Success. You can now start the database server using:
>         pg_ctl -D /some/path/to/data -l logfile start
>
> but this command did not work for me because my data directory
> contained a space.  The src/bin/initdb/initdb.c source code
> did already have a QUOTE_PATH constant, but it was the empty
> string for non-windows cases.
>
> Therefore, added quotes via existing QUOTE_PATH constant:
>
>     pg_ctl -D '/some/path/to/data' -l logfile start

You may want to register this patch to the next commit fest:
https://commitfest.postgresql.org/10/
-- 
Michael

Hi,

On 2016-08-14 10:12:45 -0500, Ryan Murphy wrote:
> Hello Postgres Team!
> but this command did not work for me because my data directory
> contained a space.  The src/bin/initdb/initdb.c source code
> did already have a QUOTE_PATH constant, but it was the empty
> string for non-windows cases.
> 
> Therefore, added quotes via existing QUOTE_PATH constant:
> 
>     pg_ctl -D '/some/path/to/data' -l logfile start

> From 275d045bc41b136df8c413eedba12fbd21609de1 Mon Sep 17 00:00:00 2001
> From: ryanfmurphy <ryanfmurphy@gmail.com>
> Date: Sun, 14 Aug 2016 08:56:50 -0500
> Subject: [PATCH] initdb: "'" for QUOTE_PATH (non-windows)
> 
> fix issue when running initdb
> 
> at the end of a successful initdb it says:
> 
> Success. You can now start the database server using:
>     pg_ctl -D /some/path/to/data -l logfile start
> 
> but this command will not work if the data directory contains a space
> therefore, added quotes via existing QUOTE_PATH constant:
> 
>     pg_ctl -D '/some/path/to/data' -l logfile start
> ---
>  src/bin/initdb/initdb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
> index 73cb7ee..6dc1e23 100644
> --- a/src/bin/initdb/initdb.c
> +++ b/src/bin/initdb/initdb.c
> @@ -332,7 +332,7 @@ do { \
>  } while (0)
>  
>  #ifndef WIN32
> -#define QUOTE_PATH    ""
> +#define QUOTE_PATH    "'"
>  #define DIR_SEP "/"
>  #else
>  #define QUOTE_PATH    "\""

ISTM that the correct fix would be to actually introduce something like
quote_path_for_shell() which either adds proper quotes, or fails if
that'd be hard (e.g. if the path contains quotes, and we're on
windows). 

On Wed, Aug 17, 2016 at 8:05 AM, Andres Freund <andres@anarazel.de> wrote:
> ISTM that the correct fix would be to actually introduce something like
> quote_path_for_shell() which either adds proper quotes, or fails if
> that'd be hard (e.g. if the path contains quotes, and we're on
> windows).

You are looking for appendShellString in fe_utils/string_utils.c.
-- 
Michael

On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy <ryanfmurphy@gmail.com> wrote:
> I have created a better patch (attached) that correctly escapes the shell
> arguments using PQExpBufferStr and the appendShellString function, as per
> Michael and Andres' suggestions.
>
> Further suggestions welcome of course.

As far as I know, it is perfectly possible to have LF/CR in a path
name (that's bad practice btw...), and your patch would make initdb
fail in such cases. Do we want to authorize that? If we bypass the
error checks in appendShellString with an extra option, and have
initdb use that, the generated command would be actually correct.
-- 
Michael

That's a fair point Michael.  I would be willing to make such a change,<span></span> but since c doesn't have optional
functionarguments I'm not sure the least intrusive way to do that.  Do you have a suggestion?<br /><br />On Wednesday,
August17, 2016, Michael Paquier <<a href="mailto:michael.paquier@gmail.com">michael.paquier@gmail.com</a>>
wrote:<br/><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On
Thu,Aug 18, 2016 at 12:21 AM, Ryan Murphy <<a href="javascript:;">ryanfmurphy@gmail.com</a>> wrote:<br /> > I
havecreated a better patch (attached) that correctly escapes the shell<br /> > arguments using PQExpBufferStr and
theappendShellString function, as per<br /> > Michael and Andres' suggestions.<br /> ><br /> > Further
suggestionswelcome of course.<br /><br /> As far as I know, it is perfectly possible to have LF/CR in a path<br /> name
(that'sbad practice btw...), and your patch would make initdb<br /> fail in such cases. Do we want to authorize that?
Ifwe bypass the<br /> error checks in appendShellString with an extra option, and have<br /> initdb use that, the
generatedcommand would be actually correct.<br /> --<br /> Michael<br /></blockquote> 

On Thu, Aug 18, 2016 at 9:30 AM, Ryan Murphy <ryanfmurphy@gmail.com> wrote:
(please avoid top-posting)

>> As far as I know, it is perfectly possible to have LF/CR in a path
>> name (that's bad practice btw...), and your patch would make initdb
>> fail in such cases. Do we want to authorize that? If we bypass the
>> error checks in appendShellString with an extra option, and have
>> initdb use that, the generated command would be actually correct.
>
> That's a fair point Michael.  I would be willing to make such a change, but
> since c doesn't have optional function arguments I'm not sure the least
> intrusive way to do that.  Do you have a suggestion?

You could just add a boolean flag to appendShellString called for
example no_error that the code path of initdb sets to true, and the
other ones to false.
--
Michael

On 2016-08-18 09:14:44 +0900, Michael Paquier wrote:
> On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy <ryanfmurphy@gmail.com> wrote:
> > I have created a better patch (attached) that correctly escapes the shell
> > arguments using PQExpBufferStr and the appendShellString function, as per
> > Michael and Andres' suggestions.
> >
> > Further suggestions welcome of course.
> 
> As far as I know, it is perfectly possible to have LF/CR in a path
> name (that's bad practice btw...), and your patch would make initdb
> fail in such cases. Do we want to authorize that?

I think that's actually a good thing to forbid.

On Thu, Aug 18, 2016 at 11:35 AM, Ryan Murphy <ryanfmurphy@gmail.com> wrote:

Be careful of top-posting, this is not this ML style:
http://www.idallen.com/topposting.html

>> I think that's actually a good thing to forbid.
>
> I think I agree Andres, there are already comments in the appendShellString
> function to this effect - they say that CR/LF chars in a file name are
> mostly used for malicious hacking attempts anyways - I know I've hardly ever
> needed a newline in a file name.
>
> Did you see anything else in my code that you have recommendations about?  I
> made sure to free the PQExpBufferStr vars that I allocated.

+        { /* pg_ctl command w path, properly quoted */
+            PQExpBuffer pg_ctl_path = createPQExpBuffer();
+            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
+                bin_dir,
+                (strlen(bin_dir) > 0) ? DIR_SEP : ""
+            );
+            appendShellString(start_db_cmd, pg_ctl_path->data);
+            destroyPQExpBuffer(pg_ctl_path);
+        }

This is not really project-style to have an independent block. Usually
those are controlled by for, while or if. And you could use the same
PQExpBuffer for everything.
-- 
Michael

On August 17, 2016 8:15:56 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:

>+        { /* pg_ctl command w path, properly quoted */
>+            PQExpBuffer pg_ctl_path = createPQExpBuffer();
>+            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
>+                bin_dir,
>+                (strlen(bin_dir) > 0) ? DIR_SEP : ""
>+            );
>+            appendShellString(start_db_cmd, pg_ctl_path->data);
>+            destroyPQExpBuffer(pg_ctl_path);
>+        }
>
>This is not really project-style to have an independent block. Usually
>those are controlled by for, while or if. 

Besides the comment positioning I'd not say that that is against the usual style, there's a number of such blocks
already. Don't think it's necessarily needed here though...
 
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund <andres@anarazel.de> wrote:
> On August 17, 2016 8:15:56 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:
>
>>+        { /* pg_ctl command w path, properly quoted */
>>+            PQExpBuffer pg_ctl_path = createPQExpBuffer();
>>+            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
>>+                bin_dir,
>>+                (strlen(bin_dir) > 0) ? DIR_SEP : ""
>>+            );
>>+            appendShellString(start_db_cmd, pg_ctl_path->data);
>>+            destroyPQExpBuffer(pg_ctl_path);
>>+        }
>>
>>This is not really project-style to have an independent block. Usually
>>those are controlled by for, while or if.
>
> Besides the comment positioning I'd not say that that is against the usual style, there's a number of such blocks
already. Don't think it's necessarily needed here though...
 

Really?  I'd remove such blocks before committing anything, or ask for
them to be removed, unless there were some special reason for having
them.

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

On 2016-08-18 16:11:27 -0400, Robert Haas wrote:
> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund <andres@anarazel.de> wrote:
> > On August 17, 2016 8:15:56 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:
> >
> >>+        { /* pg_ctl command w path, properly quoted */
> >>+            PQExpBuffer pg_ctl_path = createPQExpBuffer();
> >>+            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
> >>+                bin_dir,
> >>+                (strlen(bin_dir) > 0) ? DIR_SEP : ""
> >>+            );
> >>+            appendShellString(start_db_cmd, pg_ctl_path->data);
> >>+            destroyPQExpBuffer(pg_ctl_path);
> >>+        }
> >>
> >>This is not really project-style to have an independent block. Usually
> >>those are controlled by for, while or if.
> >
> > Besides the comment positioning I'd not say that that is against the usual style, there's a number of such blocks
already. Don't think it's necessarily needed here though...
 
> 
> Really?  I'd remove such blocks before committing anything, or ask for
> them to be removed, unless there were some special reason for having
> them.

Well, reducing the scope of variables *can* be such a reason, no? As I
said, I don't see any reason here, but in general, it's imo a reasonable
tool on one's belt.

On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2016-08-18 16:11:27 -0400, Robert Haas wrote:
>> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund <andres@anarazel.de> wrote:
>> > On August 17, 2016 8:15:56 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:
>> >
>> >>+        { /* pg_ctl command w path, properly quoted */
>> >>+            PQExpBuffer pg_ctl_path = createPQExpBuffer();
>> >>+            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
>> >>+                bin_dir,
>> >>+                (strlen(bin_dir) > 0) ? DIR_SEP : ""
>> >>+            );
>> >>+            appendShellString(start_db_cmd, pg_ctl_path->data);
>> >>+            destroyPQExpBuffer(pg_ctl_path);
>> >>+        }
>> >>
>> >>This is not really project-style to have an independent block. Usually
>> >>those are controlled by for, while or if.
>> >
>> > Besides the comment positioning I'd not say that that is against the usual style, there's a number of such blocks
already. Don't think it's necessarily needed here though...
 
>>
>> Really?  I'd remove such blocks before committing anything, or ask for
>> them to be removed, unless there were some special reason for having
>> them.
>
> Well, reducing the scope of variables *can* be such a reason, no? As I
> said, I don't see any reason here, but in general, it's imo a reasonable
> tool on one's belt.

I think it's worth reducing the scope of variables when that's as
simple as putting them inside a block that you have to create anyway,
but I'm skeptical about the idea that one would create a block just to
reduce the scope of the variables.  I don't think that's our usual
practice, and I would expect the compiler to detect where the variable
is referenced first and last anyway.

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

Ryan Murphy <ryanfmurphy@gmail.com> writes:
> On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>>> +        { /* pg_ctl command w path, properly quoted */
>>>> +            PQExpBuffer pg_ctl_path = createPQExpBuffer();
>>>> +            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",

>> I think it's worth reducing the scope of variables when that's as
>> simple as putting them inside a block that you have to create anyway,
>> but I'm skeptical about the idea that one would create a block just to
>> reduce the scope of the variables.  I don't think that's our usual
>> practice, and I would expect the compiler to detect where the variable
>> is referenced first and last anyway.

> I enjoy adding the blocks for explicit variable scoping and for quick
> navigation in vim (the % key navigates between matching {}'s).  But I want
> to fit in with the style conventions of the project.

Another point here is that code like this will look quite a bit different
after pgindent gets done with it --- that comment will not stay where
you put it, for example.  Some of our worst formatting messes come from
code wherein somebody adhered to their own favorite layout style without
any thought for how it would play with pgindent.
        regards, tom lane

On Sat, Aug 20, 2016 at 4:39 AM, Ryan Murphy <ryanfmurphy@gmail.com> wrote:
> Here is another version of my initdb shell quoting patch.  I have removed
> the unnecessary {} block.  I also ran pgindent on the code prior to creating
> the patch.

Could you please *not* top-post? This breaks the logic of the thread,
this is the third time that I mention it, and that's not the style of
this mailing list.

Regarding your patch, with a bit of clean up it gives the attached.
You should declare variables at the beginning of a code block or
function. One call to appendPQExpBufferStr can as well be avoided, and
you added too many newlines. I have switched that as ready for
committer.
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> Regarding your patch, with a bit of clean up it gives the attached.

This fails to build for me, with

gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security-fno-strict-aliasing -fwrapv -g -O1 initdb.o findtimezone.o localtime.o encnames.o
-L../../../src/port-L../../../src/common -Wl,--as-needed -Wl,-rpath,'/home/postgres/testversion/lib',--enable-new-dtags
-L../../../src/fe_utils-lpgfeutils -lpq  -lpgcommon -lpgport -lz -lreadline -lrt -lcrypt -ldl -lm  -o initdb
 
/usr/bin/ld: cannot find -lpq
collect2: ld returned 1 exit status
make: *** [initdb] Error 1

evidently because the link command omits the necessary -L switch, because
you didn't use the approved macro for linking in libpq.  It should be
$(libpq_pgport) instead, I believe.  (Probably the reason it seems to work
for you is you have a version of libpq.so in /usr/lib; but then you are
really doing the link against the wrong version of libpq.)

A bigger issue here is that it seems fundamentally wrong for initdb to be
including libpq, because it surely is never meant to be communicating
with a running postmaster.  Not sure what to do about that.  We could
consider moving pqexpbuffer out of libpq into fe_utils, but I wonder
whether that would break any third-party code.  We've never advertised
pqexpbuffer.h as a supported API of libpq, but it's probably handy enough
that people use it anyway.  I suppose we could duplicate it in fe_utils
and libpq, though that's a tad ugly.  Thoughts?

Another perhaps-only-cosmetic issue is that now initdb prints quotes
whether they are needed or not.  I find this output pretty ugly:

Success. You can now start the database server using:
         'pg_ctl' -D '/home/postgres/testversion/data' -l logfile start

That's not really the fault of this patch perhaps.  Maybe we could adjust
appendShellString so it doesn't add quotes if they are clearly
unnecessary.
        regards, tom lane

I wrote:
> A bigger issue here is that it seems fundamentally wrong for initdb to be
> including libpq, because it surely is never meant to be communicating
> with a running postmaster.  Not sure what to do about that.  We could
> consider moving pqexpbuffer out of libpq into fe_utils, but I wonder
> whether that would break any third-party code.  We've never advertised
> pqexpbuffer.h as a supported API of libpq, but it's probably handy enough
> that people use it anyway.  I suppose we could duplicate it in fe_utils
> and libpq, though that's a tad ugly.  Thoughts?

I looked into this and soon found that fe_utils/string_utils.o has
dependencies on libpq that are much wider than just pqexpbuffer :-(.
It might be a project to think about sorting that out sometime, but it
looks like it would be an awful lot of work just to avoid having initdb
depend on libpq.so.  So I now think this objection is impractical.
I'll just annotate the makefile entry to say that initdb itself doesn't
use libpq.

> Another perhaps-only-cosmetic issue is that now initdb prints quotes
> whether they are needed or not.  I find this output pretty ugly:
> ...
> That's not really the fault of this patch perhaps.  Maybe we could adjust
> appendShellString so it doesn't add quotes if they are clearly
> unnecessary.

I still think this is worth fixing, but it's a simple modification.
Will take care of it.

This item is listed in the commitfest as a bug fix, but given the lack of
previous complaints, and the fact that the printed command isn't really
meant to be blindly copied-and-pasted anyway (you're at least meant to
replace "logfile" with something), I do not think it needs back-patching.
        regards, tom lane

I looked into this and soon found that fe_utils/string_utils.o has
dependencies on libpq that are much wider than just pqexpbuffer :-(.
It might be a project to think about sorting that out sometime, but it
looks like it would be an awful lot of work just to avoid having initdb
depend on libpq.so.  So I now think this objection is impractical.
I'll just annotate the makefile entry to say that initdb itself doesn't
use libpq.

> Another perhaps-only-cosmetic issue is that now initdb prints quotes
> whether they are needed or not.

I still think this is worth fixing, but it's a simple modification.
Will take care of it.

This item is listed in the commitfest as a bug fix, but given the lack of
previous complaints, and the fact that the printed command isn't really
meant to be blindly copied-and-pasted anyway (you're at least meant to
replace "logfile" with something), I do not think it needs back-patching.

                        regards, tom lane

On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>> A bigger issue here is that it seems fundamentally wrong for initdb to be
>> including libpq, because it surely is never meant to be communicating
>> with a running postmaster.  Not sure what to do about that.  We could
>> consider moving pqexpbuffer out of libpq into fe_utils, but I wonder
>> whether that would break any third-party code.  We've never advertised
>> pqexpbuffer.h as a supported API of libpq, but it's probably handy enough
>> that people use it anyway.  I suppose we could duplicate it in fe_utils
>> and libpq, though that's a tad ugly.  Thoughts?
>
> I looked into this and soon found that fe_utils/string_utils.o has
> dependencies on libpq that are much wider than just pqexpbuffer :-(.
> It might be a project to think about sorting that out sometime, but it
> looks like it would be an awful lot of work just to avoid having initdb
> depend on libpq.so.  So I now think this objection is impractical.
> I'll just annotate the makefile entry to say that initdb itself doesn't
> use libpq.

pqexpbuffer.c is an independent piece of facility, so we could move it
to src/common and leverage the dependency a bit, and have libpq link
to the source file itself at build phase. The real problem is the call
to PQmblen in psqlscan.l... And this, I am not sure how this could be
refactored cleanly.

>> Another perhaps-only-cosmetic issue is that now initdb prints quotes
>> whether they are needed or not.  I find this output pretty ugly:
>> ...
>> That's not really the fault of this patch perhaps.  Maybe we could adjust
>> appendShellString so it doesn't add quotes if they are clearly
>> unnecessary.
>
> I still think this is worth fixing, but it's a simple modification.
> Will take care of it.
>
> This item is listed in the commitfest as a bug fix, but given the lack of
> previous complaints, and the fact that the printed command isn't really
> meant to be blindly copied-and-pasted anyway (you're at least meant to
> replace "logfile" with something), I do not think it needs back-patching.

Agreed on that. Only first-time users do a copy-paste of the command
anyway, so the impact is very narrow.

And actually, I had a look at the build failure that you reported in
3855.1471713949@sss.pgh.pa.us. While that was because of a copy of
libpq.so that I had in my own LD_LIBRARY_PATH, shouldn't all the other
frontend utilities depending on fe_utils also use $(libpq_pgport)
instead of -lpq? I think that you saw the error for initdb because
that's the first one to be built in src/bin/, and I think that Ryan
used -lpq in his patch because the same pattern is used everywhere
else.

It seems to me as well that submake-libpq and submake-libpgfeutils
should be listed in initdb's Makefile when building the binary.

Am I missing something? Please see the patch attached to see what I mean.

Thinking harder about that, it may be better to even add a variable in
Makefile.global.in for utilities in need of fe_utils, like that for
example:
libpg_feutils = -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
Thoughts?
--
Michael

Michael Paquier <michael.paquier@gmail.com> writes:
> On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I looked into this and soon found that fe_utils/string_utils.o has
>> dependencies on libpq that are much wider than just pqexpbuffer :-(.

> pqexpbuffer.c is an independent piece of facility, so we could move it
> to src/common and leverage the dependency a bit, and have libpq link
> to the source file itself at build phase. The real problem is the call
> to PQmblen in psqlscan.l... And this, I am not sure how this could be
> refactored cleanly.

I see all of these as libpq dependencies in string_utils.o:
                U PQclientEncoding                U PQescapeStringConn                U PQmblen                U
PQserverVersion

Maybe we could split that file into two parts (libpq-dependent and not)
but it would be pretty arbitrary.

> And actually, I had a look at the build failure that you reported in
> 3855.1471713949@sss.pgh.pa.us. While that was because of a copy of
> libpq.so that I had in my own LD_LIBRARY_PATH, shouldn't all the other
> frontend utilities depending on fe_utils also use $(libpq_pgport)
> instead of -lpq?

All the rest of them already have that, because their link commands
look like, eg for psql,

LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq

psql: $(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils$(CC) $(CFLAGS) $(OBJS) $(libpq_pgport) $(LDFLAGS)
$(LDFLAGS_EX)$(LIBS) -o $@$(X)                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
 

The extra reference to -lpq just makes sure that references to libpq from
libpgfeutils get resolved properly.  (And yeah, there are some platforms
that need that :-(.)  We don't need an extra copy of the -L flag.

This is all pretty messy, not least because of the way libpq_pgport
is set up; as Makefile.global notes,

# ...  This does cause duplicate -lpgport's to appear
# on client link lines.

Likely it would be better to refactor all of this so we get just one
reference to libpq and one reference to libpgport, but that'd require
a more thoroughgoing cleanup than you have here.  (Now that I think
about it, adding -L/-l to LDFLAGS is pretty duff coding style to
begin with --- we should be adding those things to LIBS, or at least
putting them just before LIBS in the command lines.)

You're right that I missed the desirability of invoking
submake-libpq and submake-libpgfeutils in initdb's build.
Will fix that.
        regards, tom lane

On Mon, Aug 22, 2016 at 8:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Likely it would be better to refactor all of this so we get just one
> reference to libpq and one reference to libpgport, but that'd require
> a more thoroughgoing cleanup than you have here.  (Now that I think
> about it, adding -L/-l to LDFLAGS is pretty duff coding style to
> begin with --- we should be adding those things to LIBS, or at least
> putting them just before LIBS in the command lines.)

Agreed, this needs more thoughts. As that's messy, I won't high-jack
more this thread and begin a new one with a more fully-bloomed patch
once I get time to look at it.
-- 
Michael

Ryan Murphy wrote:
> Thanks for committing it Tom!  Thank you all for your help.
> 
> I really like the Postgres project!  If there's anything that especially
> needs worked on let me know, I'd love to help.

https://wiki.postgresql.org/wiki/Todo

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

On Wed, Aug 24, 2016 at 9:55 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Ryan Murphy wrote:
>> Thanks for committing it Tom!  Thank you all for your help.
>>
>> I really like the Postgres project!  If there's anything that especially
>> needs worked on let me know, I'd love to help.
>
> https://wiki.postgresql.org/wiki/Todo

Even with that, there are always patches waiting for reviews, tests or input:
https://commitfest.postgresql.org/10/
-- 
Michael