From 1ccd04c70b8037b268eefcaae09ce131c8c33b59 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 16 Jan 2015 23:18:47 +0200 Subject: [PATCH] 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. --- convert.c | 27 ++++-- convert.h | 3 +- pgtypes.c | 124 ++++++++++++++++++++++++++ pgtypes.h | 3 +- statement.c | 18 +++- test/expected/param-conversions.out | 19 +++- test/expected/param-conversions_1.out | 15 ++++ test/src/param-conversions-test.c | 10 +++ 8 files changed, 206 insertions(+), 13 deletions(-) diff --git a/convert.c b/convert.c index 1782d1e..3cc3f2e 100644 --- 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 acf11b7..5d6103a 100644 --- 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 37889e5..4a6ba57 100644 --- 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 8d5b33f..34931d4 100644 --- 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 106e968..803d5e6 100644 --- 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, ¶mValues, + &nParams, + ¶mTypes, + ¶mValues, ¶mLengths, ¶mFormats, &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 99d92ed..1071af7 100644 --- 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 fb28da6..d89889c 100644 --- 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 7642572..89faf67 100644 --- 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 -- 2.39.5