Re: parse mistake in ecpg connect string

At Thu, 4 Feb 2021 09:25:00 +0000, "Wang, Shenhao" <wangsh.fnst@cn.fujitsu.com> wrote in 
> Hi, hacker
> 
> I found in function ECPGconnect, the connect string in comment is written as:
> 
> /*------
>  * new style:
>  *    <tcp|unix>:postgresql://server[:port|:/unixsocket/path:]
>  *    [/db-name][?options]
>  *------
> */
> 
> But, the parse logical seems wrong, like:

Actually it looks like broken, but..
  
> [1] https://www.postgresql.org/docs/13/ecpg-connect.html#ECPG-CONNECTING

The comment and related code seem to be remnants of an ancient syntax
of hostname/socket-path style, which should have been cleaned up in
2000. I guess that the tcp: and unix: style target remains just for
backward compatibility, I'm not sure, though.  Nowadays you can do
that by using the "dbname[@hostname][:port]" style target.

EXEC SQL CONNECT TO 'postgres@/tmp:5432';
EXEC SQL CONNECT TO 'unix:postgresql://localhost:5432/postgres?host=/tmp';

FWIW, directly embedding /unixsocket/path syntax in a URL is broken in
the view of URI. It is the reason why the current connection URI takes
the way shown above. So I think we want to remove that code rather
than to fix it.

And, since the documentation is saying that the bare target
specification is somewhat unstable, I'm not sure we dare to *fix* the
ecpg syntax.

In [1]
> In practice, it is probably less error-prone to use a (single-quoted)
> string literal or a variable reference.

That being said, we might need a description about how we can specify
a unix socket directory in ecpg-connect.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 6b0a3067e6..f45892304d 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -360,8 +360,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 
                 /*------
                  * new style:
-                 *    <tcp|unix>:postgresql://server[:port|:/unixsocket/path:]
-                 *    [/db-name][?options]
+                 *    <tcp|unix>:postgresql://server[:port][/db-name][?options]
                  *------
                  */
                 offset += strlen("postgresql://");
@@ -385,41 +384,11 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
                 }
 
                 tmp = strrchr(dbname + offset, ':');
-                if (tmp != NULL)    /* port number or Unix socket path given */
+                if (tmp != NULL)    /* port number given */
                 {
-                    char       *tmp2;
-
                     *tmp = '\0';
-                    if ((tmp2 = strchr(tmp + 1, ':')) != NULL)
-                    {
-                        *tmp2 = '\0';
-                        host = ecpg_strdup(tmp + 1, lineno);
-                        connect_params++;
-                        if (strncmp(dbname, "unix:", 5) != 0)
-                        {
-                            ecpg_log("ECPGconnect: socketname %s given for TCP connection on line %d\n", host,
lineno);
-                            ecpg_raise(lineno, ECPG_CONNECT,
ECPG_SQLSTATE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION,realname ? realname : ecpg_gettext("<DEFAULT>"));
 
-                            if (host)
-                                ecpg_free(host);
-
-                            /*
-                             * port not set yet if (port) ecpg_free(port);
-                             */
-                            if (options)
-                                ecpg_free(options);
-                            if (realname)
-                                ecpg_free(realname);
-                            if (dbname)
-                                ecpg_free(dbname);
-                            free(this);
-                            return false;
-                        }
-                    }
-                    else
-                    {
-                        port = ecpg_strdup(tmp + 1, lineno);
-                        connect_params++;
-                    }
+                    port = ecpg_strdup(tmp + 1, lineno);
+                    connect_params++;
                 }
 
                 if (strncmp(dbname, "unix:", 5) == 0)