Thread: Postgres odbc driver bug
We have found a reproducible problem with the Postgres ODBC driver. It has to do with the parameter replacement feature in the driver. Below is a piece of test code that shows the problem. We were able to fix the problem by making a change to execute.c in the ODBC driver code (attached).
odbc driver version : Found in both 6.50 and 7.01.0004.
postgresql database version : Found with both 7.0.2 and 7.0.3 versions of Postgres.
Server Operating System: Windows2000
Client Operation System: Windows2000
Explanation of the odbc driver bug:
/*****************************************
* ERROR HAPPENS HERE:
* If we set the buffer that we binded to column "text1" now, it will FAIL.
* The SQLExecute in doQuery() will succeed (finding 0 rows), and the
* SQLFetchScroll call will return SQL_NO_DATA_FOUND.
*
* This is because of a bug in the odbc driver. The call to SQLNumResultCols
* will execute the query in the driver, but the buffer that is binded is
* empty, so no rows will be found. This of course assumes that the table
* doesn't contain any empty records for the "text1" column, which is correct
* for our test. Then when we call SQLExcute, the driver will realize it has
* already done the query work and will not do it again, even though our
* buffer for parameter substitution has changed (next line of code below
* this comment).
*****************************************/
Zip file is attached. The code is currently set up to fail, but I describe in my comments that you can uncomment the code that sets the buffer before SQLNumResultCols to make things will work.
Thanks for your attention,
Keith Millard
Attachment
-----Original Message-----
From: pgsql-interfaces-owner@postgresql.org [mailto:pgsql-interfaces-owner@postgresql.org]On Behalf Of Keith Millard
Sent: Tuesday, May 01, 2001 10:37 PM
To: 'pgsql-interfaces@postgresql.org'
Subject: [INTERFACES] Postgres odbc driver bugWe have found a reproducible problem with the Postgres ODBC driver. It has to do with the parameter replacement feature in the driver. Below is a piece of test code that shows the problem. We were able to fix the problem by making a change to execute.c in the ODBC driver code (attached).
odbc driver version : Found in both 6.50 and 7.01.0004.
postgresql database version : Found with both 7.0.2 and 7.0.3 versions of Postgres.
Server Operating System: Windows2000
Client Operation System: Windows2000
Explanation of the odbc driver bug:
/*****************************************
* ERROR HAPPENS HERE:
* If we set the buffer that we binded to column "text1" now, it will FAIL.
* The SQLExecute in doQuery() will succeed (finding 0 rows), and the
* SQLFetchScroll call will return SQL_NO_DATA_FOUND.
*
* This is because of a bug in the odbc driver. The call to SQLNumResultCols
* will execute the query in the driver, but the buffer that is binded is
* empty, so no rows will be found. This of course assumes that the table
* doesn't contain any empty records for the "text1" column, which is correct
* for our test. Then when we call SQLExcute, the driver will realize it has
* already done the query work and will not do it again, even though our
* buffer for parameter substitution has changed (next line of code below
* this comment).
*****************************************/
Zip file is attached. The code is currently set up to fail, but I describe in my comments that you can uncomment the code that sets the buffer before SQLNumResultCols to make things will work.
Thanks for your attention,
Keith Millard
OK, I have found another person who reported this problem. I have attached their patch. Can you compare yours to theirs and let me know what you think? Thanks. > We have found a reproducible problem with the Postgres ODBC driver. It has > to do with the parameter replacement feature in the driver. Below is a > piece of test code that shows the problem. We were able to fix the problem > by making a change to execute.c in the ODBC driver code (attached). > > odbc driver version : Found in both 6.50 and 7.01.0004. > postgresql database version : Found with both 7.0.2 and 7.0.3 versions of > Postgres. > Server Operating System: Windows2000 > Client Operation System: Windows2000 > > Explanation of the odbc driver bug: > > > /***************************************** > * ERROR HAPPENS HERE: > * If we set the buffer that we binded to column "text1" now, it will > FAIL. > * The SQLExecute in doQuery() will succeed (finding 0 rows), and the > * SQLFetchScroll call will return SQL_NO_DATA_FOUND. > * > * This is because of a bug in the odbc driver. The call to > SQLNumResultCols > * will execute the query in the driver, but the buffer that is binded is > > * empty, so no rows will be found. This of course assumes that the > table > * doesn't contain any empty records for the "text1" column, which is > correct > * for our test. Then when we call SQLExcute, the driver will realize it > has > * already done the query work and will not do it again, even though our > * buffer for parameter substitution has changed (next line of code below > > * this comment). > *****************************************/ > > Zip file is attached. The code is currently set up to fail, but I describe > in my comments that you can uncomment the code that sets the buffer before > SQLNumResultCols to make things will work. > > Thanks for your attention, > Keith Millard > > > _____ > > <http://www.swifttouch.com/SwiftCardJump.asp?SwiftLink=95NN9NB1Q1FWA> Alex > Shore.. > Senior Software Engineer.. > Pumatech.. > tel 603-888-0666 x1017.. > .. > ashore@pumatech.com.. > www.pumatech.com.. > > [ Attachment, skipping... ] [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 Index: bind.c =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/bind.c,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.2 diff -c -r1.1.1.3 -r1.1.1.3.2.2 *** bind.c 2001/03/24 08:14:27 1.1.1.3 --- bind.c 2001/03/24 17:08:26 1.1.1.3.2.2 *************** *** 144,153 **** } /* Data at exec macro only valid for C char/binary data */ ! if ((fSqlType == SQL_LONGVARBINARY || fSqlType == SQL_LONGVARCHAR) && pcbValue && *pcbValue <= SQL_LEN_DATA_AT_EXEC_OFFSET) stmt->parameters[ipar].data_at_exec = TRUE; else stmt->parameters[ipar].data_at_exec = FALSE; mylog("SQLBindParamater: ipar=%d, paramType=%d, fCType=%d, fSqlType=%d, cbColDef=%d, ibScale=%d, rgbValue=%d, *pcbValue= %d, data_at_exec = %d\n", ipar, fParamType, fCType, fSqlType, cbColDef, ibScale, rgbValue, pcbValue ? *pcbValue: -777, stmt->parameters[ipar].data_at_exec); --- 144,158 ---- } /* Data at exec macro only valid for C char/binary data */ ! if (pcbValue && (*pcbValue == SQL_DATA_AT_EXEC || *pcbValue <= SQL_LEN_DATA_AT_EXEC_OFFSET)) stmt->parameters[ipar].data_at_exec = TRUE; else stmt->parameters[ipar].data_at_exec = FALSE; + + /* Clear premature result */ + if (stmt->status == STMT_PREMATURE) + SC_recycle_statement(stmt); mylog("SQLBindParamater: ipar=%d, paramType=%d, fCType=%d, fSqlType=%d, cbColDef=%d, ibScale=%d, rgbValue=%d, *pcbValue= %d, data_at_exec = %d\n", ipar, fParamType, fCType, fSqlType, cbColDef, ibScale, rgbValue, pcbValue ? *pcbValue: -777, stmt->parameters[ipar].data_at_exec); Index: convert.c =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/convert.c,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.1 diff -c -r1.1.1.3 -r1.1.1.3.2.1 *** convert.c 2001/03/24 08:14:31 1.1.1.3 --- convert.c 2001/03/24 15:35:10 1.1.1.3.2.1 *************** *** 838,844 **** param_number++; if (param_number >= stmt->parameters_allocated) ! break; /* Assign correct buffers based on data at exec param or not */ if (stmt->parameters[param_number].data_at_exec) --- 838,857 ---- param_number++; if (param_number >= stmt->parameters_allocated) ! { ! if (stmt->pre_executing) ! { ! strcpy(&new_statement[npos], "NULL"); ! npos += 4; ! stmt->inaccurate_result = TRUE; ! continue; ! } ! else ! { ! new_statement[npos++] = '?'; ! continue; ! } ! } /* Assign correct buffers based on data at exec param or not */ if (stmt->parameters[param_number].data_at_exec) *************** *** 866,873 **** */ if (!buffer) { ! new_statement[npos++] = '?'; ! continue; } param_ctype = stmt->parameters[param_number].CType; --- 879,896 ---- */ if (!buffer) { ! if (stmt->pre_executing) ! { ! strcpy(&new_statement[npos], "NULL"); ! npos += 4; ! stmt->inaccurate_result = TRUE; ! continue; ! } ! else ! { ! new_statement[npos++] = '?'; ! continue; ! } } param_ctype = stmt->parameters[param_number].CType; Index: execute.c =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/execute.c,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.2 diff -c -r1.1.1.3 -r1.1.1.3.2.2 *** execute.c 2001/03/24 08:14:35 1.1.1.3 --- execute.c 2001/03/24 16:40:47 1.1.1.3.2.2 *************** *** 34,39 **** --- 34,40 ---- #include "qresult.h" #include "convert.h" #include "bind.h" + #include "pgtypes.h" #include "lobj.h" extern GLOBAL_VALUES globals; *************** *** 223,239 **** */ if (stmt->prepare && stmt->status == STMT_PREMATURE) { ! stmt->status = STMT_FINISHED; ! if (stmt->errormsg == NULL) ! { ! mylog("%s: premature statement but return SQL_SUCCESS\n", func); ! return SQL_SUCCESS; ! } else { ! SC_log_error(func, "", stmt); ! mylog("%s: premature statement so return SQL_ERROR\n", func); ! return SQL_ERROR; } } --- 224,245 ---- */ if (stmt->prepare && stmt->status == STMT_PREMATURE) { ! if (stmt->inaccurate_result) ! SC_recycle_statement(stmt); else { ! stmt->status = STMT_FINISHED; ! if (stmt->errormsg == NULL) ! { ! mylog("%s: premature statement but return SQL_SUCCESS\n", func); ! return SQL_SUCCESS; ! } ! else ! { ! SC_log_error(func, "", stmt); ! mylog("%s: premature statement so return SQL_ERROR\n", func); ! return SQL_ERROR; ! } } } *************** *** 284,314 **** } ! /* ! * The bound parameters could have possibly changed since the last ! * execute of this statement? Therefore check for params and re-copy. ! */ ! stmt->data_at_exec = -1; ! for (i = 0; i < stmt->parameters_allocated; i++) { ! /* Check for data at execution parameters */ ! if (stmt->parameters[i].data_at_exec == TRUE) { ! if (stmt->data_at_exec < 0) ! stmt->data_at_exec = 1; ! else ! stmt->data_at_exec++; } ! } ! /* If there are some data at execution parameters, return need data */ ! /* ! * SQLParamData and SQLPutData will be used to send params and execute ! * the statement. ! */ ! if (stmt->data_at_exec > 0) ! return SQL_NEED_DATA; mylog("%s: copying statement params: trans_status=%d, len=%d, stmt='%s'\n", func, conn->transact_status, strlen(stmt->statement),stmt->statement); --- 290,326 ---- } ! /* Check if statement has any data-at-execute parameters when it is not in SC_pre_execute. */ ! if (!stmt->pre_executing) { ! ! /* ! * The bound parameters could have possibly changed since the last ! * execute of this statement? Therefore check for params and re-copy. ! */ ! stmt->data_at_exec = -1; ! for (i = 0; i < stmt->parameters_allocated; i++) { ! /* Check for data at execution parameters */ ! if (stmt->parameters[i].data_at_exec == TRUE) ! { ! if (stmt->data_at_exec < 0) ! stmt->data_at_exec = 1; ! else ! stmt->data_at_exec++; ! } } ! /* If there are some data at execution parameters, return need data */ ! /* ! * SQLParamData and SQLPutData will be used to send params and execute ! * the statement. ! */ ! if (stmt->data_at_exec > 0) ! return SQL_NEED_DATA; + } + mylog("%s: copying statement params: trans_status=%d, len=%d, stmt='%s'\n", func, conn->transact_status, strlen(stmt->statement),stmt->statement); *************** *** 778,785 **** } else ! { /* for handling text fields and small ! * binaries */ if (cbValue == SQL_NTS) { --- 790,796 ---- } else ! { /* for handling fields */ if (cbValue == SQL_NTS) { *************** *** 794,809 **** } else { ! current_param->EXEC_buffer = malloc(cbValue + 1); ! if (!current_param->EXEC_buffer) { ! stmt->errornumber = STMT_NO_MEMORY_ERROR; ! stmt->errormsg = "Out of memory in SQLPutData (2)"; ! SC_log_error(func, "", stmt); ! return SQL_ERROR; } - memcpy(current_param->EXEC_buffer, rgbValue, cbValue); - current_param->EXEC_buffer[cbValue] = '\0'; } } } --- 805,839 ---- } else { ! Int2 ctype = current_param->CType; ! if (ctype == SQL_C_DEFAULT) ! ctype = sqltype_to_default_ctype(current_param->SQLType); ! if (ctype == SQL_C_CHAR || ctype == SQL_C_BINARY) { ! current_param->EXEC_buffer = malloc(cbValue + 1); ! if (!current_param->EXEC_buffer) ! { ! stmt->errornumber = STMT_NO_MEMORY_ERROR; ! stmt->errormsg = "Out of memory in SQLPutData (2)"; ! SC_log_error(func, "", stmt); ! return SQL_ERROR; ! } ! memcpy(current_param->EXEC_buffer, rgbValue, cbValue); ! current_param->EXEC_buffer[cbValue] = '\0'; ! } ! else ! { ! Int4 used = ctype_length(ctype); ! current_param->EXEC_buffer = malloc(used); ! if (!current_param->EXEC_buffer) ! { ! stmt->errornumber = STMT_NO_MEMORY_ERROR; ! stmt->errormsg = "Out of memory in SQLPutData (2)"; ! SC_log_error(func, "", stmt); ! return SQL_ERROR; ! } ! memcpy(current_param->EXEC_buffer, rgbValue, used); } } } } Index: pgtypes.c =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/pgtypes.c,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.2 diff -c -r1.1.1.3 -r1.1.1.3.2.2 *** pgtypes.c 2001/03/24 08:14:46 1.1.1.3 --- pgtypes.c 2001/03/24 16:40:47 1.1.1.3.2.2 *************** *** 956,958 **** --- 956,1011 ---- return SQL_C_CHAR; } } + + Int4 + ctype_length(Int2 ctype) + { + switch (ctype) + { + case SQL_C_SSHORT: + case SQL_C_SHORT: + return sizeof(SWORD); + + case SQL_C_USHORT: + return sizeof(UWORD); + + case SQL_C_SLONG: + case SQL_C_LONG: + return sizeof(SDWORD); + + case SQL_C_ULONG: + return sizeof(UDWORD); + + case SQL_C_FLOAT: + return sizeof(SFLOAT); + + case SQL_C_DOUBLE: + return sizeof(SDOUBLE); + + case SQL_C_BIT: + return sizeof(UCHAR); + + case SQL_C_STINYINT: + case SQL_C_TINYINT: + return sizeof(SCHAR); + + case SQL_C_UTINYINT: + return sizeof(UCHAR); + + case SQL_C_DATE: + return sizeof(DATE_STRUCT); + + case SQL_C_TIME: + return sizeof(TIME_STRUCT); + + case SQL_C_TIMESTAMP: + return sizeof(TIMESTAMP_STRUCT); + + case SQL_C_BINARY: + case SQL_C_CHAR: + return 0; + + default: /* should never happen */ + return 0; + } + } Index: pgtypes.h =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/pgtypes.h,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.1 diff -c -r1.1.1.3 -r1.1.1.3.2.1 *** pgtypes.h 2001/03/24 08:14:46 1.1.1.3 --- pgtypes.h 2001/03/24 15:35:12 1.1.1.3.2.1 *************** *** 93,97 **** --- 93,98 ---- char *pgtype_create_params(StatementClass *stmt, Int4 type); Int2 sqltype_to_default_ctype(Int2 sqltype); + Int4 ctype_length(Int2 ctype); #endif Index: statement.c =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/statement.c,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.1 diff -c -r1.1.1.3 -r1.1.1.3.2.1 *** statement.c 2001/03/24 08:14:53 1.1.1.3 --- statement.c 2001/03/24 15:35:12 1.1.1.3.2.1 *************** *** 292,297 **** --- 292,300 ---- /* Clear Statement Options -- defaults will be set in AllocStmt */ memset(&rv->options, 0, sizeof(StatementOptions)); + + rv->pre_executing = FALSE; + rv->inaccurate_result = FALSE; } return rv; } *************** *** 519,524 **** --- 522,528 ---- QR_Destructor(self->result); self->result = NULL; } + self->inaccurate_result = FALSE; /****************************************************************/ /* Reset only parameters that have anything to do with results */ *************** *** 551,568 **** void SC_pre_execute(StatementClass *self) { - mylog("SC_pre_execute: status = %d\n", self->status); if (self->status == STMT_READY) { mylog(" preprocess: status = READY\n"); ! SQLExecute(self); ! if (self->status == STMT_FINISHED) { ! mylog(" preprocess: after status = FINISHED, so set PREMATURE\n"); self->status = STMT_PREMATURE; } } --- 555,587 ---- void SC_pre_execute(StatementClass *self) { mylog("SC_pre_execute: status = %d\n", self->status); if (self->status == STMT_READY) { mylog(" preprocess: status = READY\n"); + + if (self->statement_type == STMT_TYPE_SELECT) + { + char old_pre_executing = self->pre_executing; + self->pre_executing = TRUE; + self->inaccurate_result = FALSE; ! SQLExecute(self); ! self->pre_executing = old_pre_executing; ! ! if (self->status == STMT_FINISHED) ! { ! mylog(" preprocess: after status = FINISHED, so set PREMATURE\n"); ! self->status = STMT_PREMATURE; ! } ! } ! else { ! self->result = QR_Constructor(); ! QR_set_status(self->result, PGRES_TUPLES_OK); ! self->inaccurate_result = TRUE; self->status = STMT_PREMATURE; } } Index: statement.h =================================================================== RCS file: /usr/local/src/cvsroot/psqlodbc/statement.h,v retrieving revision 1.1.1.3 retrieving revision 1.1.1.3.2.1 diff -c -r1.1.1.3 -r1.1.1.3.2.1 *** statement.h 2001/03/24 08:14:54 1.1.1.3 --- statement.h 2001/03/24 15:35:13 1.1.1.3.2.1 *************** *** 213,218 **** --- 213,221 ---- * parameter * substitution */ + char pre_executing; /* This statement is prematurely executing */ + char inaccurate_result; /* Current status is PREMATURE + * but result is inaccurate */ }; #define SC_get_conn(a) (a->hdbc)
Bruce Momjian wrote: > > OK, I have found another person who reported this problem. I have > attached their patch. Can you compare yours to theirs and let me know > what you think? Thanks. > This patch is already committed but it doesn't fix the bug reported by Keith. regards, Hiroshi Inoue
Keith, forget my earlier request about reviewing the patch. One of our ODBC guys (Hiroshi) says your patch fixes a different problem. If no one objects, I will be applying yours in a day or two. Can you make sure my version of your patch is correct? I will change the #if 0 and just remove that code, and remove your //chuck comments. Thanks. > We have found a reproducible problem with the Postgres ODBC driver. It has > to do with the parameter replacement feature in the driver. Below is a > piece of test code that shows the problem. We were able to fix the problem > by making a change to execute.c in the ODBC driver code (attached). > > odbc driver version : Found in both 6.50 and 7.01.0004. > postgresql database version : Found with both 7.0.2 and 7.0.3 versions of > Postgres. > Server Operating System: Windows2000 > Client Operation System: Windows2000 > > Explanation of the odbc driver bug: > > > /***************************************** > * ERROR HAPPENS HERE: > * If we set the buffer that we binded to column "text1" now, it will > FAIL. > * The SQLExecute in doQuery() will succeed (finding 0 rows), and the > * SQLFetchScroll call will return SQL_NO_DATA_FOUND. > * > * This is because of a bug in the odbc driver. The call to > SQLNumResultCols > * will execute the query in the driver, but the buffer that is binded is > > * empty, so no rows will be found. This of course assumes that the > table > * doesn't contain any empty records for the "text1" column, which is > correct > * for our test. Then when we call SQLExcute, the driver will realize it > has > * already done the query work and will not do it again, even though our > * buffer for parameter substitution has changed (next line of code below > > * this comment). > *****************************************/ > > Zip file is attached. The code is currently set up to fail, but I describe > in my comments that you can uncomment the code that sets the buffer before > SQLNumResultCols to make things will work. > > Thanks for your attention, > Keith Millard > > > _____ > > <http://www.swifttouch.com/SwiftCardJump.asp?SwiftLink=95NN9NB1Q1FWA> Alex > Shore.. > Senior Software Engineer.. > Pumatech.. > tel 603-888-0666 x1017.. > .. > ashore@pumatech.com.. > www.pumatech.com.. > > [ Attachment, skipping... ] [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania 19026 *** e2 Tue May 8 12:49:51 2001 --- execute.c Tue May 8 12:47:48 2001 *************** *** 204,209 **** --- 204,210 ---- it from an SQLPrepare/SQLDescribeCol type of scenario. So just return success. */ + #if 0 //chuck if ( stmt->prepare && stmt->status == STMT_PREMATURE) { stmt->status = STMT_FINISHED; if (stmt->errormsg == NULL) { *************** *** 216,221 **** --- 217,223 ---- return SQL_ERROR; } } + #endif mylog("%s: clear errors...\n", func); *************** *** 248,255 **** } /* Check if the statement is in the correct state */ ! if ((stmt->prepare && stmt->status != STMT_READY) || ! (stmt->status != STMT_ALLOCATED && stmt->status != STMT_READY)) { stmt->errornumber = STMT_STATUS_ERROR; stmt->errormsg = "The handle does not point to a statement that is ready to be executed"; --- 250,259 ---- } /* Check if the statement is in the correct state */ ! //chuck if ((stmt->prepare && stmt->status != STMT_READY) || ! //chuck (stmt->status != STMT_ALLOCATED && stmt->status != STMT_READY)) { ! if ((stmt->prepare && (stmt->status != STMT_READY && stmt->status != STMT_PREMATURE)) || //chuck ! (stmt->status != STMT_ALLOCATED && stmt->status != STMT_READY && stmt->status != STMT_PREMATURE)) { //chuck stmt->errornumber = STMT_STATUS_ERROR; stmt->errormsg = "The handle does not point to a statement that is ready to be executed"; *************** *** 262,270 **** --- 266,277 ---- /* The bound parameters could have possibly changed since the last execute of this statement? Therefore check for params and re-copy. */ + mylog("%s: Check for data at execution parameters\n", func); + mylog("%s: stmt->parameters_allocated = %d\n", func,stmt->parameters_allocated); stmt->data_at_exec = -1; for (i = 0; i < stmt->parameters_allocated; i++) { /* Check for data at execution parameters */ + mylog("%s: stmt->parameters[%d].data_at_exec = %d\n", func,i,stmt->parameters[i].data_at_exec); if ( stmt->parameters[i].data_at_exec == TRUE) { if (stmt->data_at_exec < 0) stmt->data_at_exec = 1;