Re: [HACKERS] Another review of URI for libpq, v7 submission
Excerpts from Alvaro Herrera's message of lun abr 09 16:41:50 -0300 2012: There are three minor things that need to be changed for this to be committable: Committed this patch after some more editorialization; in particular the test was rewritten so that instead of trying to connect, it uses PQconninfoParse to figure out how the URI is parsed, which makes a lot more sense. Also some other changes to the accepted URI, in particular so that username, pwd, and port are possible to be specified when using unix-domain sockets. Now that it is a done deal I'm sure people will start complaining how bad the documentation change was; please keep the flames up. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Excerpts from Peter Eisentraut's message of vie abr 06 03:09:10 -0300 2012: On fre, 2012-04-06 at 00:25 -0300, Alvaro Herrera wrote: Some moments of radical thinking later, I became unhappy with the fact that the conninfo stuff and parameter keywords are all crammed in the PQconnectdbParams description. This feels wrong to me, even more so after we expand it even more to add URIs to the mix. I think it's better to create a separate sect1 (which I've entitled Connection Strings) which explains the conninfo and URI formats as well as accepted keywords. The new section is referenced from the multiple places that need it, without having to point to PQconnectdbParams. Yes, it should be split out. But the libpq chapter already has too many tiny sect1s. I think it should be a sect2 under Database Connection Control. Thanks, that seems a good idea. I have tweaked things slightly and it looks pretty decent to me. Wording improvements are welcome. The file in its entirety can be seen here: https://github.com/alvherre/postgres/blob/uri/doc/src/sgml/libpq.sgml The new bits start at line 1224. I also attach the HTML output for easy reading. (I wonder if it's going to be visible in the archives). There are three minor things that need to be changed for this to be committable: 1. it depends on strtok_r which is likely to be a problem in MSVC++ and perhaps older Unix platforms as well. 2. The ssl=true trick being converted into sslmode=require doesn't work if the URI specifies them uri-encoded, which seems bogus. 3. if an unknown keyword is uri-encoded, the error message displays it still uri-encoded. Seems to me it'd be better to uri-decode it before throwing error. Alexander says he's going to work on these and then I'll finally commit it. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support Title: Database Connection Control Functions PostgreSQL 9.2devel DocumentationPrevUpChapter 31. libpq - C LibraryNext31.1. Database Connection Control Functions The following functions deal with making a connection to a PostgreSQL backend server. An application program can have several backend connections open at one time. (One reason to do that is to access more than one database.) Each connection is represented by a PGconn object, which is obtained from the function PQconnectdb, PQconnectdbParams, or PQsetdbLogin. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the PGconn object. The PQstatus function should be called to check the return value for a successful connection before queries are sent via the connection object. Warning On Unix, forking a process with open libpq connections can lead to unpredictable results because the parent and child processes share the same sockets and operating system resources. For this reason, such usage is not recommended, though doing an exec from the child process to load a new executable is safe. Note: On Windows, there is a way to improve performance if a single database connection is repeatedly started and shutdown. Internally, libpq calls WSAStartup() and WSACleanup() for connection startup and shutdown, respectively. WSAStartup() increments an internal Windows library reference count which is decremented by WSACleanup(). When the reference count is just one, calling WSACleanup() frees all resources and all DLLs are unloaded. This is an expensive operation. To avoid this, an application can manually call WSAStartup() so resources will not be freed when the last database connection is closed. PQconnectdbParams Makes a new connection to the database server. PGconn *PQconnectdbParams(const char * const *keywords, const char * const *values, int expand_dbname); This function opens a new database connection using the parameters taken from two NULL-terminated arrays. The first, keywords, is defined as an array of strings, each one being a key word. The second, values, gives the value for each key word. Unlike PQsetdbLogin below, the parameter set can be extended without changing the function signature, so use of this function (or its nonblocking analogs PQconnectStartParams and PQconnectPoll) is preferred for new application programming. The currently recognized parameter key words are listed in Section 31.1.1. When expand_dbname is non-zero, the dbname key word value is allowed to be recognized as a connection string. More details on the possible formats appear in Section 31.1.2. The passed arrays can be empty to use all default
Re: [HACKERS] Another review of URI for libpq, v7 submission
On fre, 2012-04-06 at 00:25 -0300, Alvaro Herrera wrote: Some moments of radical thinking later, I became unhappy with the fact that the conninfo stuff and parameter keywords are all crammed in the PQconnectdbParams description. This feels wrong to me, even more so after we expand it even more to add URIs to the mix. I think it's better to create a separate sect1 (which I've entitled Connection Strings) which explains the conninfo and URI formats as well as accepted keywords. The new section is referenced from the multiple places that need it, without having to point to PQconnectdbParams. Yes, it should be split out. But the libpq chapter already has too many tiny sect1s. I think it should be a sect2 under Database Connection Control. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
On ons, 2012-03-28 at 01:49 +0300, Alex wrote: Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. The compiler warning is still there: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4122:20: error: unused variable ‘option’ [-Werror=unused-variable] -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Excerpts from Peter Eisentraut's message of jue abr 05 13:52:13 -0300 2012: On ons, 2012-03-28 at 01:49 +0300, Alex wrote: Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. The compiler warning is still there: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4122:20: error: unused variable ‘option’ [-Werror=unused-variable] Here's an updated patch, including this fix and others. (Most notable the fact that I got rid of conninfo_store_uri_encoded_value, instead expanding conninfo_storeval a bit). I also fixed the test script to run in VPATH. I intend to commit this shortly, barring objection from Peter who is listed as committer in the CF app. I think the only thing I'm not really sure about is the usage of the synopsis tag to mark example URIs in the docs. It looks to me that they should mostly be literal instead, but I'm not really sure about that either -- I'm suspecting the PDF output would look rather horrible. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support libpq-uri-v11.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
On 22.03.2012 23:42, Alex wrote: Okay, at last here's v9, rebased against current master branch. Some quick comments on this patch: I see a compiler warning: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4113: warning: unused variable ‘option’ Docs are missing. I wonder if you should get an error if you try specify the same option multiple times. At least the behavior needs to be documented. Should %00 be forbidden? The error message is a bit confusing for postgres://localhost?dbname=%XXfoo: WARNING: ignoring unrecognized URI query parameter: dbname There is in fact nothing wrong with dbname, it's the %XX in the value that's bogus. Looking at conninfo_uri_decode(), I think it's missing a bounds check, and peeks at two bytes beyond the end of string if the input ends in a '%'. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes: On 22.03.2012 23:42, Alex wrote: Okay, at last here's v9, rebased against current master branch. Some quick comments on this patch: Heikki, thank you for taking a look at this! I see a compiler warning: fe-connect.c: In function ‘conninfo_parse’: fe-connect.c:4113: warning: unused variable ‘option’ The warning is due to the earlier commit e9ce658b. I believe the above line supposed to go away. Docs are missing. Yes, my plan is to focus on the documentation and code comments while sorting out any remaining issues with the code. I wonder if you should get an error if you try specify the same option multiple times. At least the behavior needs to be documented. Since conninfo strings may contain duplicated keywords and the latter just takes precedence, I think we should just do the same with URIs (which we already do.) I don't see the behavior of conninfo strings documented anywhere, however. Should %00 be forbidden? Probably yes, good spot. The error message is a bit confusing for postgres://localhost?dbname=%XXfoo: WARNING: ignoring unrecognized URI query parameter: dbname There is in fact nothing wrong with dbname, it's the %XX in the value that's bogus. Hm, yes, that's a bug. Looks like conninfo_uri_parse_params needs to be adjusted to properly pass the error message generated by conninfo_store_uri_encoded_value. I wonder if examining the errorMessage buffer to tell if it's a hard error (it's going to be empty in the case of ignoreMissing=true) is a good practice. Looking at conninfo_uri_decode(), I think it's missing a bounds check, and peeks at two bytes beyond the end of string if the input ends in a %'. No, in that case what happens on L4919 is this: we dereference q which is pointing at NUL terminator and pass the value to the first get_hexdigit in the if condition, the pointer itself is then incremented and does point beyond the end of string, but since get_hexdigit returns FALSE we don't call the second get_hexdigit, so we don't dereference the invalid pointer. There is a comment right before that if which says just that. -- Alex -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
On tor, 2012-03-22 at 23:42 +0200, Alex wrote: Okay, at last here's v9, rebased against current master branch. Attached is a patch on top of your v9 with two small fixes: - Don't provide a check target in libpq/Makefile if it's not implemented. - Use the configured port number for running the tests (otherwise it runs against my system installation, for example). diff --git i/src/interfaces/libpq/Makefile w/src/interfaces/libpq/Makefile index f19f272..266e3db 100644 --- i/src/interfaces/libpq/Makefile +++ w/src/interfaces/libpq/Makefile @@ -121,7 +121,7 @@ install: all installdirs install-lib $(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)' $(INSTALL_DATA) $(srcdir)/pg_service.conf.sample '$(DESTDIR)$(datadir)/pg_service.conf.sample' -check installcheck: +installcheck: $(MAKE) -C test $@ installdirs: installdirs-lib diff --git i/src/interfaces/libpq/test/Makefile w/src/interfaces/libpq/test/Makefile index 964bb20..869a2f0 100644 --- i/src/interfaces/libpq/test/Makefile +++ w/src/interfaces/libpq/test/Makefile @@ -5,7 +5,7 @@ include $(top_builddir)/src/Makefile.global all: installcheck installcheck: - BINDIR='$(bindir)' SUBDIR='$(subdir)' $(SHELL) ./regress.sh + BINDIR='$(bindir)' PGPORT='$(DEF_PGPORT)' SUBDIR='$(subdir)' $(SHELL) ./regress.sh clean distclean maintainer-clean: rm -f regress.out regress.diff -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Peter Eisentraut pete...@gmx.net writes: On tor, 2012-03-22 at 23:42 +0200, Alex wrote: Okay, at last here's v9, rebased against current master branch. Attached is a patch on top of your v9 with two small fixes: - Don't provide a check target in libpq/Makefile if it's not implemented. - Use the configured port number for running the tests (otherwise it runs against my system installation, for example). Neat, thank you. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Alex a...@commandprompt.com writes: Peter Eisentraut pete...@gmx.net writes: Attached is a patch on top of your v9 with two small fixes: - Don't provide a check target in libpq/Makefile if it's not implemented. - Use the configured port number for running the tests (otherwise it runs against my system installation, for example). Neat, thank you. Attached is a gzipped v10, complete with the above fixes and fixes to bugs found by Heikki. Documentation and code comments are also finally updated. Of all the command-line utilities which can accept conninfo string, only psql mentions that, so only its documentation was updated. Other utilities, like pg_dump and pg_restore can also work with either conninfo or URI, however this remains undocumented. -- Alex libpq-uri-v10.patch.gz Description: libpq-uri-v10.patch.gz -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Trivial libpq refactoring patch (was: Re: [HACKERS] Another review of URI for libpq, v7 submission)
Alex Shulgin a...@commandprompt.com writes: While working on this fix I've figured out that I need my conninfo_uri_parse to support use_defaults parameter, like conninfo(_array)_parse functions do. The problem is that the block of code which does the defaults handling is duplicated in both of the above functions. What I'd like to do is extract it into a separate function to call. What I wouldn't like is bloating the original URI patch with this unrelated change. Applied with minor adjustments --- notably, I thought conninfo_add_defaults was a better name for the function. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Alex a...@commandprompt.com writes: Marko Kreen mark...@gmail.com writes: On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not this work: db = PQconnectdb(postgres://localhost); ? Good catch. I've figured out that we'll need a bit more intrusive change than simply overriding the expand_dbname check in conninfo_array_parse (like the current version does) to support URIs in all PQconnect* variants. Okay, at last here's v9, rebased against current master branch. What's new in this version is parse_connection_string function to be called instead of conninfo_parse. It will check for possible URI in the connection string and dispatch to conninfo_uri_parse if URI was found, otherwise it will fall back to conninfo_parse. In two places in code we don't want to parse the string unless it is deemed a real connection string and not plain dbname. The new function recognized_connection_string is added to check this. Thanks Marko for spotting the problem and thanks Tom for committing the refactoring patch! -- Alex binFI5wAVF1ys.bin Description: libpq-uri-v9.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Trivial libpq refactoring patch (was: Re: [HACKERS] Another review of URI for libpq, v7 submission)
Alex a...@commandprompt.com writes:
Marko Kreen mark...@gmail.com writes:
On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote:
https://github.com/a1exsh/postgres/commits/uri
The point of the patch is to have one string with all connection options,
in standard format, yes? So why does not this work:
db = PQconnectdb(postgres://localhost);
?
Good catch.
I've figured out that we'll need a bit more intrusive change than simply
overriding the expand_dbname check in conninfo_array_parse (like the
current version does) to support URIs in all PQconnect* variants.
I still need to figure out some details, but this is to give people a
status update.
While working on this fix I've figured out that I need my
conninfo_uri_parse to support use_defaults parameter, like
conninfo(_array)_parse functions do.
The problem is that the block of code which does the defaults handling
is duplicated in both of the above functions. What I'd like to do is
extract it into a separate function to call. What I wouldn't like is
bloating the original URI patch with this unrelated change.
So here's a trivial patch to do the refactoring. Also, it uses the
newly added conninfo_fill_defaults directly in PQconndefaults, instead
of doing the parse empty conninfo string trick. If this could be
applied, I'd rebase my patch against the updated master branch and
submit the new version.
As it goes, the patch adds a little duplication when it comes to
creating a working copy of PQconninfoOptions array. Attached is the
second patch on top of the first one to extract this bit of code into
conninfo_init. Not sure if we should go all the way through this, so
it's not critical if this one is not applied.
--
Regards,
Alex
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***
*** 297,302 static PQconninfoOption *conninfo_parse(const char *conninfo,
--- 297,304
static PQconninfoOption *conninfo_array_parse(const char *const * keywords,
const char *const * values,
PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
+ static bool conninfo_fill_defaults(PQconninfoOption *options,
+ PQExpBuffer errorMessage);
static char *conninfo_getval(PQconninfoOption *connOptions,
const char *keyword);
static void defaultNoticeReceiver(void *arg, const PGresult *res);
***
*** 836,842 PQconndefaults(void)
initPQExpBuffer(errorBuf);
if (PQExpBufferDataBroken(errorBuf))
return NULL;/* out of memory already :-( */
! connOptions = conninfo_parse(, errorBuf, true);
termPQExpBuffer(errorBuf);
return connOptions;
}
--- 838,860
initPQExpBuffer(errorBuf);
if (PQExpBufferDataBroken(errorBuf))
return NULL;/* out of memory already :-( */
!
! /* Make a working copy of PQconninfoOptions */
! connOptions = malloc(sizeof(PQconninfoOptions));
! if (connOptions == NULL)
! {
! printfPQExpBuffer(errorBuf,
! libpq_gettext(out of
memory\n));
! return NULL;
! }
! memcpy(connOptions, PQconninfoOptions, sizeof(PQconninfoOptions));
!
! if (!conninfo_fill_defaults(connOptions, errorBuf))
! {
! PQconninfoFree(connOptions);
! connOptions = NULL;
! }
!
termPQExpBuffer(errorBuf);
return connOptions;
}
***
*** 4002,4008 conninfo_parse(const char *conninfo, PQExpBuffer
errorMessage,
char *pname;
char *pval;
char *buf;
- char *tmp;
char *cp;
char *cp2;
PQconninfoOption *options;
--- 4020,4025
***
*** 4170,4245 conninfo_parse(const char *conninfo, PQExpBuffer
errorMessage,
free(buf);
/*
!* Stop here if caller doesn't want defaults filled in.
!*/
! if (!use_defaults)
! return options;
!
! /*
!* If there's a service spec, use it to obtain any not-explicitly-given
!* parameters.
*/
! if (parseServiceInfo(options, errorMessage))
{
PQconninfoFree(options);
return NULL;
}
- /*
-* Get the fallback resources for parameters not specified in the
conninfo
-* string nor the service.
-*/
- for (option = options; option-keyword != NULL; option++)
- {
- if (option-val != NULL)
- continue; /* Value was in
conninfo or service */
-
- /*
-* Try to get the environment variable fallback
-*/
- if (option-envvar != NULL)
- {
Re: [HACKERS] Another review of URI for libpq, v7 submission
Marko Kreen mark...@gmail.com writes: On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not this work: db = PQconnectdb(postgres://localhost); ? Good catch. I've figured out that we'll need a bit more intrusive change than simply overriding the expand_dbname check in conninfo_array_parse (like the current version does) to support URIs in all PQconnect* variants. I still need to figure out some details, but this is to give people a status update. -- Alex -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
On Thu, Mar 15, 2012 at 11:29:31PM +0200, Alex wrote: https://github.com/a1exsh/postgres/commits/uri The point of the patch is to have one string with all connection options, in standard format, yes? So why does not this work: db = PQconnectdb(postgres://localhost); ? -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Another review of URI for libpq, v7 submission
I reviewed this and so far have not found any serious problems, although as is par for the course it contains some of the fiddly bits involved in any string manipulations in C. I made a few edits -- none strictly necessary for correctness -- that the original author is free audit and/or include[0]. I did put in some defensive programming choices (instead of if/else if/elseif/else raise an error, even if the latter is allegedly impossible) that I think are a good idea. Loops around pointer increments are very fastidiously checked for NUL-byteness, and those that aren't are carefully guarded by invariants that seem like they should prevent an overrun. The nature of the beast, I suppose, short of giving libpq a StringData like struct and a small lexer to make it more clear that a subtle overrun is not creeping in. One thing I found puzzling was that in the latest revision the tests appeared to be broken for me: all @ signs were translated to (at). Is that mangling applied by the archives, or something? The test suite neatly tries to copy pg_regress's general make installcheck style, but it likes to use my username as the database rather than the standard regression as seen by pg_regress. It is nice that a place to test connection strings and such is there, where there was none before. I am happy with the range and style of accepted URIs, and I think this can stem the bleeding of the fragmentation already taking place at large. [0]: https://github.com/fdr/postgres/tree/uri, commit e50ef375b7a731ca79bf5d3ca8b0bd69c97a9e71, aka the 'uri' branch -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Excerpts from Daniel Farina's message of jue mar 15 05:49:50 -0300 2012: One thing I found puzzling was that in the latest revision the tests appeared to be broken for me: all @ signs were translated to (at). Is that mangling applied by the archives, or something? Ugh, ouch. Yeah, that was done by the archives. It seems that when attachments are text/plain Mhonarc applies anti-spamming to them :-( The original message doesn't have that problem. Sorry about that. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Daniel Farina dan...@heroku.com writes: I reviewed this and so far have not found any serious problems, although as is par for the course it contains some of the fiddly bits involved in any string manipulations in C. I made a few edits -- none strictly necessary for correctness -- that the original author is free audit and/or include[0]. Thank you for the review, Daniel! Apparently, I was on drugs when I've submitted v7, as it still contains the bug for which to fix I was forced to move parts of the code back into the main parser routine... I did put in some defensive programming choices (instead of if/else if/elseif/else raise an error, even if the latter is allegedly impossible) that I think are a good idea. Yes, this is a good idea, I'll incorporate them in the patch. However, this one doesn't work: https://github.com/fdr/postgres/commit/4fad90fb243d9266b1003cfbcf8397f67269fad3 Neither '@' or '/' are mandatory in the URI anywhere after scheme://, so we can't just say it should never happen. Running the regression test with this commit applied highlights the problem. One thing I found puzzling was that in the latest revision the tests appeared to be broken for me: all @ signs were translated to (at). Is that mangling applied by the archives, or something? This is definitely mangling by the lists. I can see this has happened to people before, e.g.: http://archives.postgresql.org/pgsql-hackers/2010-01/msg00938.php But that discussion didn't seem to lead to a solution for the problem. I wonder if there's any evidence as to that mangling the email addresses helps to reduce spam at all? I mean replacing (at) back to @ and (dot) to . is piece of cake for a spam crawler. What I see though, is that most of the message-id URLs are broken by the mangling. Also I don't think everyone should just tolerate that it messes with the attachments. Should we all suddenly use application/octet-stream or application/gzip instead of text/plain? The test suite neatly tries to copy pg_regress's general make installcheck style, but it likes to use my username as the database rather than the standard regression as seen by pg_regress. It is nice that a place to test connection strings and such is there, where there was none before. Oh, I don't see why we can't use regression too. While testing this I've also noticed that there was some funny behavior when you left out the final slash after hostname, e.g.: postgres://localhost/ vs. postgres://localhost. It turned out in the former case we were setting dbname conninfo parameter to an empty string and in the latter one we didn't set it at all. The difference is that when we do set it, the default dbname is derived from username, but when we don't--$PGDATABASE is used. I've fixed this, so now both URIs work the same way (both do not set dbname.) It also appeared to me that with the current code, a wide range of funny-looking URIs are considered valid, e.g.: postgres://user@ postgres://@host postgres://host:/ and, taking approach to the extreme: postgres://:@: This specifies empty user, password, host and port, and looks really funny. I've added (actually revived) some checks to forbid setting empty URI parts where it doesn't make much sense. Finally, attached is v8. Hopefully I didn't mess things up too much. -- Regards, Alex binaKreQKSDSW.bin Description: libpq-uri-v8.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
On Thu, Mar 15, 2012 at 7:36 AM, Alex Shulgin a...@commandprompt.com wrote: I wonder if there's any evidence as to that mangling the email addresses helps to reduce spam at all? I mean replacing (at) back to @ and (dot) to . is piece of cake for a spam crawler. I suspect we're long past the point in Internet history where such simple obfuscation could possibly matter. Finally, attached is v8. Hopefully I didn't mess things up too much. I'll give it another look-over. Do you have these in git somewhere? It will help me save time on some of the incremental changes. -- fdr -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Another review of URI for libpq, v7 submission
Daniel Farina dan...@heroku.com writes: Finally, attached is v8. Hopefully I didn't mess things up too much. I'll give it another look-over. Do you have these in git somewhere? It will help me save time on some of the incremental changes. Yes, I've just pushed my dev branch to this fork of mine: https://github.com/a1exsh/postgres/commits/uri -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
