Send proper datatypes to the server when binding parameters.

We used to always send 0 as the parameter type, which means that the server
should deduce the datatype from the context. That usually works well, but
we can do better. Sometimes there is ambiguity, like in "SELECT '555' > ?".
If the parameter is bound as a string, '6' would return true, but if it's
bound as an integer, it returns false.

When the application calls SQLBindParameter, it specifies the datatype, so
we should make use of that.

Add a test case for the "SELECT '555' > ?" case.

diff --git a/convert.c b/convert.c
index 1782d1e4326b949293e60104562b1bacfa17c575..3cc3f2e7598641427c6609b02cd95be6ec7edac2 100644 (file)
--- a/convert.c
+++ b/convert.c
@@ -2144,7 +2144,8 @@ convert_escape(QueryParse *qp, QueryBuild *qb);
 static int
 inner_process_tokens(QueryParse *qp, QueryBuild *qb);
 static int
-ResolveOneParam(QueryBuild *qb, QueryParse *qp, BOOL *isnull, BOOL *usebinary);
+ResolveOneParam(QueryBuild *qb, QueryParse *qp, BOOL *isnull, BOOL *usebinary,
+               Oid *pgType);
 static int
 processParameters(QueryParse *qp, QueryBuild *qb,
    size_t *output_count, SQLLEN param_pos[][2]);
@@ -3168,6 +3169,7 @@ inner_process_tokens(QueryParse *qp, QueryBuild *qb)
    char    literal_quote = LITERAL_QUOTE, dollar_quote = DOLLAR_QUOTE, escape_in_literal = '\0';
    BOOL        isnull;
    BOOL        isbinary;
+   Oid         dummy;
 
    if (stmt && stmt->ntab > 0)
        bestitem = GET_NAME(stmt->ti[0]->bestitem);
@@ -3523,7 +3525,7 @@ inner_process_tokens(QueryParse *qp, QueryBuild *qb)
    /*
     * It's a '?' parameter alright
     */
-   retval = ResolveOneParam(qb, qp, &isnull, &isbinary);
+   retval = ResolveOneParam(qb, qp, &isnull, &isbinary, &dummy);
    if (retval < 0)
        return retval;
 
@@ -3543,7 +3545,9 @@ cleanup:
  */
 BOOL
 build_libpq_bind_params(StatementClass *stmt, const char *plan_name,
-                       int *nParams, char ***paramValues,
+                       int *nParams,
+                       OID **paramTypes,
+                       char ***paramValues,
                        int **paramLengths,
                        int **paramFormats,
                        int *resultFormat)
@@ -3557,6 +3561,7 @@ build_libpq_bind_params(StatementClass *stmt, const char *plan_name,
    RETCODE     retval;
    const       IPDFields *ipdopts = SC_get_IPDF(stmt);
 
+   *paramTypes = NULL;
    *paramValues = NULL;
    *paramLengths = NULL;
    *paramFormats = NULL;
@@ -3576,6 +3581,9 @@ build_libpq_bind_params(StatementClass *stmt, const char *plan_name,
    if (QB_initialize(&qb, MIN_ALC_SIZE, stmt, NULL) < 0)
        return FALSE;
 
+   *paramTypes = malloc(sizeof(OID) * num_params);
+   if (*paramTypes == NULL)
+       goto cleanup;
    *paramValues = malloc(sizeof(char *) * num_params);
    if (*paramValues == NULL)
        goto cleanup;
@@ -3607,6 +3615,7 @@ inolog("num_p=%d\n", num_p);
            BOOL        isnull;
            BOOL        isbinary;
            char       *val_copy;
+           OID         pgType;
 
            inolog("%dth parameter type oid is %u\n", i, PIC_dsp_pgtype(conn, parameters[i]));
 
@@ -3614,7 +3623,7 @@ inolog("num_p=%d\n", num_p);
                continue;
 
            qb.npos = 0;
-           retval = ResolveOneParam(&qb, NULL, &isnull, &isbinary);
+           retval = ResolveOneParam(&qb, NULL, &isnull, &isbinary, &pgType);
            if (SQL_ERROR == retval)
            {
                QB_replace_SC_error(stmt, &qb, func);
@@ -3630,11 +3639,13 @@ inolog("num_p=%d\n", num_p);
                memcpy(val_copy, qb.query_statement, qb.npos);
                val_copy[qb.npos] = '\0';
 
+               (*paramTypes)[i] = pgType;
                (*paramValues)[i] = val_copy;
                (*paramLengths)[i] = qb.npos;
            }
            else
            {
+               (*paramTypes)[i] = pgType;
                (*paramValues)[i] = NULL;
                (*paramLengths)[i] = 0;
            }
@@ -3866,9 +3877,12 @@ parse_to_numeric_struct(const char *wv, SQL_NUMERIC_STRUCT *ns, BOOL *overflow)
  * *isnull is set to TRUE if it was NULL.
  * *isbinary is set to TRUE, if the binary output format was used. (binary
  *   output is only produced if the FLGB_BINARY_AS_POSSIBLE flag is set)
+ * *pgType is set to the PostgreSQL type OID that should be used when binding
+ * (or 0, to let the server decide)
  */
 static int
-ResolveOneParam(QueryBuild *qb, QueryParse *qp, BOOL *isnull, BOOL *isbinary)
+ResolveOneParam(QueryBuild *qb, QueryParse *qp, BOOL *isnull, BOOL *isbinary,
+               OID *pgType)
 {
    CSTR func = "ResolveOneParam";
 
@@ -4096,6 +4110,9 @@ inolog("ipara=%p paramType=%d %d proc_return=%d\n", ipara, ipara ? ipara->paramT
    param_sqltype = ipara->SQLType;
    param_pgtype = PIC_dsp_pgtype(qb->conn, *ipara);
 
+   /* XXX: should we use param_pgtype here instead? */
+   *pgType = sqltype_to_bind_pgtype(conn, param_sqltype);
+
    mylog("%s: from(fcType)=%d, to(fSqlType)=%d(%u)\n", func,
                param_ctype, param_sqltype, param_pgtype);
 

diff --git a/convert.h b/convert.h
index acf11b7605cc520407290b299098c51779153e77..5d6103ac614ad81d83e8ee2729611c0f1ed00be6 100644 (file)
--- a/convert.h
+++ b/convert.h
@@ -59,7 +59,8 @@ int convert_lo(StatementClass *stmt, const void *value, SQLSMALLINT fCType,
 size_t     findTag(const char *str, char dollar_quote, int ccsc);
 
 BOOL build_libpq_bind_params(StatementClass *stmt, const char *plan_name,
-                       int *nParams, char ***paramValues,
+                       int *nParams, OID **paramTypes,
+                       char ***paramValues,
                        int **paramLengths,
                        int **paramFormats,
                        int *resultFormat);

diff --git a/pgtypes.c b/pgtypes.c
index 37889e5278a972bc15758f44478b5bf7bd0bc41a..4a6ba575b2e20ffdf7ab34314247c9e07700817d 100644 (file)
--- a/pgtypes.c
+++ b/pgtypes.c
@@ -1232,6 +1232,130 @@ pgtype_attr_transfer_octet_length(const ConnectionClass *conn, OID type, int att
 }
 
 
+
+/*
+ * This is used when binding a query parameter, to decide which PostgreSQL
+ * datatype to send to the server, depending on the SQL datatype that was
+ * used in the SQLBindParameter call.
+ *
+ * For most types, this returns 0, which means that the server should treat
+ * the parameter the same as an untyped literal string, and deduce the type
+ * based on the context.
+ *
+ * This is different from sqltype_to_pgtype(), which doesn't return 0 but
+ * tries to give a closer match to the actual datatype.
+ */
+OID
+sqltype_to_bind_pgtype(const ConnectionClass *conn, SQLSMALLINT fSqlType)
+{
+   OID     pgType;
+   const ConnInfo  *ci = &(conn->connInfo);
+
+   pgType = 0; /* ??? */
+   switch (fSqlType)
+   {
+       case SQL_BINARY:
+       case SQL_VARBINARY:
+           pgType = PG_TYPE_BYTEA;
+           break;
+
+       case SQL_BIT:
+           pgType = ci->drivers.bools_as_char ? PG_TYPE_CHAR : PG_TYPE_BOOL;
+           break;
+
+       case SQL_TYPE_DATE:
+       case SQL_DATE:
+           pgType = PG_TYPE_DATE;
+           break;
+
+       case SQL_DOUBLE:
+       case SQL_FLOAT:
+           pgType = PG_TYPE_FLOAT8;
+           break;
+
+       case SQL_DECIMAL:
+       case SQL_NUMERIC:
+           pgType = PG_TYPE_NUMERIC;
+           break;
+
+       case SQL_BIGINT:
+           pgType = PG_TYPE_INT8;
+           break;
+
+       case SQL_INTEGER:
+           pgType = PG_TYPE_INT4;
+           break;
+
+       case SQL_LONGVARBINARY:
+           if (ci->bytea_as_longvarbinary)
+               pgType = PG_TYPE_BYTEA;
+           else
+               pgType = conn->lobj_type;
+           break;
+
+       case SQL_REAL:
+           pgType = PG_TYPE_FLOAT4;
+           break;
+
+       case SQL_SMALLINT:
+       case SQL_TINYINT:
+           pgType = PG_TYPE_INT2;
+           break;
+
+       case SQL_TIME:
+       case SQL_TYPE_TIME:
+           pgType = PG_TYPE_TIME;
+           break;
+
+       case SQL_TIMESTAMP:
+       case SQL_TYPE_TIMESTAMP:
+           pgType = PG_TYPE_DATETIME;
+           break;
+
+       case SQL_GUID:
+           if (PG_VERSION_GE(conn, 8.3))
+               pgType = PG_TYPE_UUID;
+           else
+               pgType = 0;
+           break;
+
+       case SQL_INTERVAL_MONTH:
+       case SQL_INTERVAL_YEAR:
+       case SQL_INTERVAL_YEAR_TO_MONTH:
+       case SQL_INTERVAL_DAY:
+       case SQL_INTERVAL_HOUR:
+       case SQL_INTERVAL_MINUTE:
+       case SQL_INTERVAL_SECOND:
+       case SQL_INTERVAL_DAY_TO_HOUR:
+       case SQL_INTERVAL_DAY_TO_MINUTE:
+       case SQL_INTERVAL_DAY_TO_SECOND:
+       case SQL_INTERVAL_HOUR_TO_MINUTE:
+       case SQL_INTERVAL_HOUR_TO_SECOND:
+       case SQL_INTERVAL_MINUTE_TO_SECOND:
+           pgType = PG_TYPE_INTERVAL;
+           break;
+
+       case SQL_CHAR:
+#ifdef UNICODE_SUPPORT
+       case SQL_WCHAR:
+#endif /* UNICODE_SUPPORT */
+       case SQL_LONGVARCHAR:
+#ifdef UNICODE_SUPPORT
+       case SQL_WLONGVARCHAR:
+#endif /* UNICODE_SUPPORT */
+       case SQL_VARCHAR:
+#if    UNICODE_SUPPORT
+       case SQL_WVARCHAR:
+#endif /* UNICODE_SUPPORT */
+       default:
+           /* The default is to let the server choose */
+           pgType = 0;
+           break;
+   }
+
+   return pgType;
+}
+
 OID
 sqltype_to_pgtype(const ConnectionClass *conn, SQLSMALLINT fSqlType)
 {

diff --git a/pgtypes.h b/pgtypes.h
index 8d5b33fd409309d4f533eff4a97e26fed13b7811..34931d49d13a178dddd7bf4309a9dbad5c8ae645 100644 (file)
--- a/pgtypes.h
+++ b/pgtypes.h
@@ -53,7 +53,7 @@
 #define PG_TYPE_DATE           1082
 #define PG_TYPE_TIME           1083
 #define PG_TYPE_TIMESTAMP_NO_TMZONE    1114        /* since 7.2 */
-#define PG_TYPE_DATETIME       1184
+#define PG_TYPE_DATETIME       1184    /* timestamptz */
 #define PG_TYPE_INTERVAL       1186
 #define PG_TYPE_TIME_WITH_TMZONE   1266        /* since 7.1 */
 #define PG_TYPE_TIMESTAMP      1296    /* deprecated since 7.0 */
@@ -83,6 +83,7 @@ extern SQLSMALLINT sqlTypes[];
 
 OID        pg_true_type(const ConnectionClass *, OID, OID);
 OID        sqltype_to_pgtype(const ConnectionClass *conn, SQLSMALLINT fSqlType);
+OID        sqltype_to_bind_pgtype(const ConnectionClass *conn, SQLSMALLINT fSqlType);
 
 SQLSMALLINT    pgtype_to_concise_type(const StatementClass *stmt, OID type, int col);
 SQLSMALLINT    pgtype_to_sqldesctype(const StatementClass *stmt, OID type, int col);

diff --git a/statement.c b/statement.c
index 106e968a725a52914bae2d6b55532b643e1b4b64..803d5e6ad57350cb78ed95c1157d987c56bb348f 100644 (file)
--- a/statement.c
+++ b/statement.c
@@ -2460,6 +2460,7 @@ libpq_bind_and_exec(StatementClass *stmt, const char *plan_name,
    CSTR        func = "libpq_bind_and_exec";
    ConnectionClass *conn = SC_get_conn(stmt);
    int         nParams;
+   Oid        *paramTypes = NULL;
    char      **paramValues = NULL;
    int        *paramLengths = NULL;
    int        *paramFormats = NULL;
@@ -2487,7 +2488,9 @@ libpq_bind_and_exec(StatementClass *stmt, const char *plan_name,
    /* 1. Bind */
    mylog("%s: bind plan_name=%s\n", func, plan_name);
    if (!build_libpq_bind_params(stmt, plan_name,
-                                &nParams, &paramValues,
+                                &nParams,
+                                &paramTypes,
+                                &paramValues,
                                 &paramLengths, &paramFormats,
                                 &resultFormat))
    {
@@ -2522,8 +2525,10 @@ libpq_bind_and_exec(StatementClass *stmt, const char *plan_name,
        pgres = PQexecParams(conn->pqconn,
                             pstmt->query,
                             pstmt->num_params,
-                            NULL,
-                            (const char **) paramValues, paramLengths, paramFormats,
+                            paramTypes,
+                            (const char **) paramValues,
+                            paramLengths,
+                            paramFormats,
                             resultFormat);
    }
    else
@@ -2621,6 +2626,8 @@ cleanup:
        }
        free(paramValues);
    }
+   if (paramTypes)
+       free(paramTypes);
    if (paramLengths)
        free(paramLengths);
    if (paramFormats)
@@ -2709,7 +2716,10 @@ mylog("sta_pidx=%d end_pidx=%d num_p=%d\n", sta_pidx, end_pidx, num_params);
                SQL_PARAM_OUTPUT == ipdopts->parameters[i].paramType)
                paramTypes[j++] = PG_TYPE_VOID;
            else
-               paramTypes[j++] = 0;
+           {
+               paramTypes[j++] = sqltype_to_bind_pgtype(conn,
+                                                        ipdopts->parameters[i].SQLType);
+           }
        }
    }
 

diff --git a/test/expected/param-conversions.out b/test/expected/param-conversions.out
index 99d92edcf297ead7e7bca5792ce121685e76b5de..1071af75fe1a52fb19f577e96d4007fbbd6c6358 100644 (file)
--- a/test/expected/param-conversions.out
+++ b/test/expected/param-conversions.out
@@ -39,6 +39,21 @@ Testing "SELECT 1 > ?" with SQL_C_CHAR -> SQL_SMALLINT param "-32768"...
 Result set:
 1
 
+
+Testing conversions whose result depend on whether the
+parameter is treated as a string or an integer...
+Testing "SELECT '555' > ?" with SQL_C_CHAR -> SQL_INTEGER param "6"...
+Result set:
+1
+
+Testing "SELECT '555' > ?" with SQL_C_CHAR -> SQL_SMALLINT param "6"...
+Result set:
+1
+
+Testing "SELECT '555' > ?" with SQL_C_CHAR -> SQL_CHAR param "6"...
+Result set:
+0
+
 Testing "SELECT 1 > ?" with SQL_C_CHAR -> SQL_INTEGER param "99999999999999999999999"...
 SQLExecDirect failed
 22003=ERROR: value "99999999999999999999999" is out of range for type integer;
@@ -58,12 +73,12 @@ Error while executing the query
 
 Testing "SELECT 1.3 > ?" with SQL_C_CHAR -> SQL_FLOAT param "3', 'injected, BAD!', '1"...
 SQLExecDirect failed
-22P02=ERROR: invalid input syntax for type numeric: "3', 'injected, BAD!', '1";
+22P02=ERROR: invalid input syntax for type double precision: "3', 'injected, BAD!', '1";
 Error while executing the query
 
 Testing "SELECT 1.4 > ?" with SQL_C_CHAR -> SQL_FLOAT param "4 \'bad', '1"...
 SQLExecDirect failed
-22P02=ERROR: invalid input syntax for type numeric: "4 \'bad', '1";
+22P02=ERROR: invalid input syntax for type double precision: "4 \'bad', '1";
 Error while executing the query
 
 Testing "SELECT 1-?" with SQL_C_CHAR -> SQL_INTEGER param "-1"...

diff --git a/test/expected/param-conversions_1.out b/test/expected/param-conversions_1.out
index fb28da65805052f3d66cb665856e81e88e4b3ae1..d89889c186dcdbdec8a51f2f5687459820ee18bc 100644 (file)
--- a/test/expected/param-conversions_1.out
+++ b/test/expected/param-conversions_1.out
@@ -39,6 +39,21 @@ Testing "SELECT 1 > ?" with SQL_C_CHAR -> SQL_SMALLINT param "-32768"...
 Result set:
 1
 
+
+Testing conversions whose result depend on whether the
+parameter is treated as a string or an integer...
+Testing "SELECT '555' > ?" with SQL_C_CHAR -> SQL_INTEGER param "6"...
+Result set:
+1
+
+Testing "SELECT '555' > ?" with SQL_C_CHAR -> SQL_SMALLINT param "6"...
+Result set:
+1
+
+Testing "SELECT '555' > ?" with SQL_C_CHAR -> SQL_CHAR param "6"...
+Result set:
+0
+
 Testing "SELECT 1 > ?" with SQL_C_CHAR -> SQL_INTEGER param "99999999999999999999999"...
 Result set:
 0

diff --git a/test/src/param-conversions-test.c b/test/src/param-conversions-test.c
index 76425723d236e353b8fcdc6f66fffac08cf2e33f..89faf679244c381e1de078ff43e44951bd3a8749 100644 (file)
--- a/test/src/param-conversions-test.c
+++ b/test/src/param-conversions-test.c
@@ -45,6 +45,16 @@ int main(int argc, char **argv)
    TEST_CONVERT("SELECT 1 > ?", SQL_C_CHAR, SQL_SMALLINT, "32767");
    TEST_CONVERT("SELECT 1 > ?", SQL_C_CHAR, SQL_SMALLINT, "-32768");
 
+   /*
+    * The result of these depend on whether the server treats the parameters
+    * as a string or an integer.
+    */
+   printf("\nTesting conversions whose result depend on whether the\n");
+   printf("parameter is treated as a string or an integer...\n");
+   TEST_CONVERT("SELECT '555' > ?", SQL_C_CHAR, SQL_INTEGER, "6");
+   TEST_CONVERT("SELECT '555' > ?", SQL_C_CHAR, SQL_SMALLINT, "6");
+   TEST_CONVERT("SELECT '555' > ?", SQL_C_CHAR, SQL_CHAR, "6");
+
    /*
     * The result of this test depends on what datatype the server thinks
     * it's dealing with. If the driver sends it as a naked literal, the