Re: [COMMITTERS] pgsql: psql: fix \connect with URIs and conninfo strings
On Thu, Apr 2, 2015 at 10:38 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Apr 2, 2015 at 10:19:51AM -0400, Robert Haas wrote: On Thu, Apr 2, 2015 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. I was thinking of changing master to look like the 9.4 version. [ shrug... ] IMO, a quick git revert is less work and leaves a cleaner state for Alvaro to apply whatever final solution he settles on. But do what you wish. OK, I've just reverted it. Can I ask about the logic of why this bug fix was backpatched, or is that clear to everyone but me. What is your question, exactly? There was a fair amount of discussion of whether and how to back-patch this on pgsql-hackers. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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
Robert Haas wrote: On Thu, Apr 2, 2015 at 10:57 AM, Bruce Momjian br...@momjian.us wrote: Uh, I didn't see a huge amount of discussion, but I guess it is sufficient: http://www.postgresql.org/message-id/flat/20150109191551.ga32...@fetter.org#20150109191551.ga32...@fetter.org There are at least a dozen messages on that thread that are entirely dedicated to whether this should be back-patched. How much discussion did you want? Yeah, this patch did get more discussion than many other patches do. Sometimes you just have to go against what looks like apathy, and move forward. That's what commitfests are supposed to be good for, after all: personally, I care not one bit about this functionality, but somebody went huge lengths to get it to work, which must mean that there definitely is value in it. Now, mind you, my view did not prevail, so I can't be completely happy with the outcome, but I was in the minority. At least the version that broke essentially the entire build-farm only went into master, so that's something. Yeah, I realized that backpatching the refactoring was the wrong approach, though I didn't notice the breakage in master until it was much too late. I pushed the 9.4 version to master now. Apologies for not fixing it sooner -- had a funeral to attend, a drive 5 hours away. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: psql: fix \connect with URIs and conninfo strings
On Thu, Apr 2, 2015 at 10:57 AM, Bruce Momjian br...@momjian.us wrote: Uh, I didn't see a huge amount of discussion, but I guess it is sufficient: http://www.postgresql.org/message-id/flat/20150109191551.ga32...@fetter.org#20150109191551.ga32...@fetter.org There are at least a dozen messages on that thread that are entirely dedicated to whether this should be back-patched. How much discussion did you want? Now, mind you, my view did not prevail, so I can't be completely happy with the outcome, but I was in the minority. At least the version that broke essentially the entire build-farm only went into master, so that's something. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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
eOn Thu, Apr 2, 2015 at 10:41:55AM -0400, Robert Haas wrote: On Thu, Apr 2, 2015 at 10:38 AM, Bruce Momjian br...@momjian.us wrote: On Thu, Apr 2, 2015 at 10:19:51AM -0400, Robert Haas wrote: On Thu, Apr 2, 2015 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. I was thinking of changing master to look like the 9.4 version. [ shrug... ] IMO, a quick git revert is less work and leaves a cleaner state for Alvaro to apply whatever final solution he settles on. But do what you wish. OK, I've just reverted it. Can I ask about the logic of why this bug fix was backpatched, or is that clear to everyone but me. What is your question, exactly? There was a fair amount of discussion of whether and how to back-patch this on pgsql-hackers. Uh, I didn't see a huge amount of discussion, but I guess it is sufficient: http://www.postgresql.org/message-id/flat/20150109191551.ga32...@fetter.org#20150109191551.ga32...@fetter.org -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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
On Thu, Apr 2, 2015 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: This is really inconvenient. I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. I was thinking of changing master to look like the 9.4 version. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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
On Thu, Apr 2, 2015 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. I was thinking of changing master to look like the 9.4 version. [ shrug... ] IMO, a quick git revert is less work and leaves a cleaner state for Alvaro to apply whatever final solution he settles on. But do what you wish. OK, I've just reverted it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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
On Thu, Apr 2, 2015 at 10:19:51AM -0400, Robert Haas wrote: On Thu, Apr 2, 2015 at 10:09 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. I was thinking of changing master to look like the 9.4 version. [ shrug... ] IMO, a quick git revert is less work and leaves a cleaner state for Alvaro to apply whatever final solution he settles on. But do what you wish. OK, I've just reverted it. Can I ask about the logic of why this bug fix was backpatched, or is that clear to everyone but me. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- 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
On Wed, Apr 1, 2015 at 9:58 PM, 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 I can't even build the server. ar crs libpq.a fe-auth.o fe-connect.o fe-exec.o fe-misc.o fe-print.o fe-lobj.o fe-protocol2.o fe-protocol3.o pqexpbuffer.o fe-secure.o libpq-events.o chklocale.o inet_net_ntop.o noblock.o pgstrcasecmp.o pqsignal.o thread.o ip.o md5.o encnames.o wchar.o fe-secure-openssl.o Undefined symbols for architecture x86_64: _libpq_connstring_is_recognized, referenced from: _PQconnectStartParams in fe-connect.o _PQsetdbLogin in fe-connect.o _libpq_connstring_uri_prefix_length, referenced from: _parse_connection_string in fe-connect.o ld: symbol(s) not found for architecture x86_64 collect2: ld returned 1 exit status This is really inconvenient. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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
Robert Haas robertmh...@gmail.com writes: This is really inconvenient. I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. regards, tom lane -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: psql: fix \connect with URIs and conninfo strings
Robert Haas robertmh...@gmail.com writes: On Thu, Apr 2, 2015 at 10:00 AM, Tom Lane t...@sss.pgh.pa.us wrote: I'm going to revert that commit in HEAD shortly, unless Alvaro pops up and promises a fix PDQ. Or you could do the same. I was thinking of changing master to look like the 9.4 version. [ shrug... ] IMO, a quick git revert is less work and leaves a cleaner state for Alvaro to apply whatever final solution he settles on. But do what you wish. regards, tom lane -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
Re: [COMMITTERS] pgsql: psql: fix \connect with URIs and conninfo strings
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
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
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
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: psql: fix \connect with URIs and conninfo strings
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). 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. regards, tom lane -- Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-committers
