On Tue, Nov 19, 2013 at 11:51 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Fri, Nov 15, 2013 at 9:01 PM, FabrÃzio de Royes Mello > <fabriziome...@gmail.com> wrote: >> On Wed, Nov 13, 2013 at 9:37 PM, Josh Berkus <j...@agliodbs.com> wrote: >>> handyrep@john:~/handyrep$ pg_isready --version >>> pg_isready (PostgreSQL) 9.3.1 >>> >>> handyrep@john:~/handyrep$ pg_isready -h john -p 5432 -U postgres -d >>> postgres -q >>> pg_isready: missing "=" after "postgres" in connection info string >>> >>> handyrep@john:~/handyrep$ pg_isready --host=john --port=5432 >>> --user=postgres --dbname=postgres >>> pg_isready: missing "=" after "postgres" in connection info string >>> >>> handyrep@john:~/handyrep$ pg_isready -h john -p 5432 -U postgres >>> john:5432 - accepting connections >>> >>> so apparently the -d option: >>> >>> a) doesn't work, and >>> b) doesn't do anything >>> >>> I suggest simply removing it from the utility. >>> >>> I'll note that the -U option doesn't appear to do anything relevant >>> either, but at least it doesn't error unnecessarily: >>> >>> handyrep@john:~/handyrep$ pg_isready -h john -p 5432 -U no_such_user >>> john:5432 - accepting connections >>> >> >> The attached patch fix it. > > Well, there still seem to be some problems. > > [rhaas pgsql]$ src/bin/scripts/pg_isready -d host=sgdasasg > /tmp:5432 - no attempt > > I added some debugging instrumentation and it looks like there's a > problem with this code: > > if (strcmp(def->keyword, "hostaddr") == 0 || > strcmp(def->keyword, "host") == 0) > > The problem is that we end up executing the code contained in that > block twice, once for host and once for hostaddr. Thus: > > [rhaas pgsql]$ src/bin/scripts/pg_isready -d host=sgdasasg > def->keyword=host pghost_str=sgdasasg > def->keyword=hostaddr pghost_str=/tmp > def->keyword=port pgport_str=5432 > /tmp:5432 - no attempt > > Oops. > > Nevertheless, that's kind of a separate problem, so we could address > in a separate commit. But even ignoring that, this still isn't right: > according to the fine documentation, -d will be expanded not only if > it contains an =, but also if it starts with a valid URI prefix. This > would break that documented behavior. > > http://www.postgresql.org/docs/9.3/static/app-pg-isready.html
Attached is the updated version of the patch. Is there any other bug? Regards, -- Fujii Masao
*** a/src/bin/scripts/pg_isready.c --- b/src/bin/scripts/pg_isready.c *************** *** 31,36 **** main(int argc, char **argv) --- 31,37 ---- const char *connect_timeout = DEFAULT_CONNECT_TIMEOUT; const char *pghost_str = NULL; + const char *pghostaddr_str = NULL; const char *pgport_str = NULL; #define PARAMS_ARRAY_SIZE 7 *************** *** 130,136 **** main(int argc, char **argv) /* * Get the host and port so we can display them in our output */ ! if (pgdbname) { opts = PQconninfoParse(pgdbname, &errmsg); if (opts == NULL) --- 131,140 ---- /* * Get the host and port so we can display them in our output */ ! if (pgdbname && ! (strncmp(pgdbname, "postgresql://", 13) == 0 || ! strncmp(pgdbname, "postgres://", 11) == 0 || ! strchr(pgdbname, '=') != NULL)) { opts = PQconninfoParse(pgdbname, &errmsg); if (opts == NULL) *************** *** 149,156 **** main(int argc, char **argv) for (opt = opts, def = defs; def->keyword; def++) { ! if (strcmp(def->keyword, "hostaddr") == 0 || ! strcmp(def->keyword, "host") == 0) { if (opt && opt->val) pghost_str = opt->val; --- 153,159 ---- for (opt = opts, def = defs; def->keyword; def++) { ! if (strcmp(def->keyword, "host") == 0) { if (opt && opt->val) pghost_str = opt->val; *************** *** 161,166 **** main(int argc, char **argv) --- 164,176 ---- else pghost_str = DEFAULT_PGSOCKET_DIR; } + else if (strcmp(def->keyword, "hostaddr") == 0) + { + if (opt && opt->val) + pghostaddr_str = opt->val; + else if (def->val) + pghostaddr_str = def->val; + } else if (strcmp(def->keyword, "port") == 0) { if (opt && opt->val) *************** *** 179,185 **** main(int argc, char **argv) if (!quiet) { ! printf("%s:%s - ", pghost_str, pgport_str); switch (rv) { --- 189,197 ---- if (!quiet) { ! printf("%s:%s - ", ! pghostaddr_str != NULL ? pghostaddr_str : pghost_str, ! pgport_str); switch (rv) {
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers