Re: Postgres odbc driver bug

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)