ODBC-473 SQLGetData could write data of the wrong length in case of

resultset streaming and cached resultset. Cached here means - cached in
the ODBC driver, not on the client side in general. That caching occurs
if not the whole result has been fetched, and other statement is
executed.

Also contains more fixes of the testcases for the RS streaming and other
small fixes.

Turned on testing in RS streaming mode in github actions. Added in
github actions pause after driver installation on windows. Apparently
it's not always finished at the moment of DSN creation.

Author: lawrinn

.github/workflows/ci.yml

@@ -124,6 +124,7 @@ jobs:
             #$procLog.Kill()
           }
           echo "=========="
+          Start-Sleep -Milliseconds 300
           Get-OdbcDriver -Platform "64-bit"
           Add-OdbcDsn -Name $env:TEST_DSN -DriverName "MariaDB ODBC 3.2 Driver" -DsnType "User" -SetPropertyValue @( "SERVER=$env:TEST_SERVER", "DATABASE=$env:TEST_SCHEMA", "USER=$env:TEST_UID", "PASSWORD=$env:TEST_PASSWORD", "PORT=$env:TEST_PORT" )
 
@@ -159,7 +160,7 @@ jobs:
           ctest --verbose
 
           export TEST_ADD_PARAM="STREAMRS=1;FORWARDONLY=1"
-          #ctest --output-on-failure
+          ctest --output-on-failure
 
         env:
           DB_TYPE: ${{ matrix.db-type }}

driver/class/BinRow.cpp

@@ -1115,8 +1115,8 @@ namespace mariadb
         rowDataCache.emplace_back();
       }
       else {
-        //rowDataCache.emplace_back(bind[i].length_value, static_cast<const char*>(bind[i].buffer));
-        rowDataCache.emplace_back(b.length && *b.length > 0 ? *b.length : b.buffer_length);
+        // If we could guarantee, that "b.length && *b.length > 0 ? *b.length :" is for current row, we could use it
+        rowDataCache.emplace_back(b.buffer_length);
         b.buffer= const_cast<char*>(rowDataCache.back().arr);
         mysql_stmt_fetch_column(stmt, &b, static_cast<unsigned int>(i), 0);
       }

driver/class/Results.h

@@ -32,15 +32,15 @@ namespace mariadb
 class Results  {
 
   PreparedStatement* statement= nullptr;
-  ServerPrepareResult* serverPrepResult= nullptr; //?
+  ServerPrepareResult* serverPrepResult= nullptr;
   int32_t fetchSize= 0;
   bool batch= false;
   std::size_t expectedSize= 1;
   Unique::CmdInformation cmdInformation;
   std::deque<Unique::ResultSet> executionResults;
   Unique::ResultSet currentRs;
   ResultSet* resultSet= nullptr;
-  Unique::ResultSet callableResultSet; //?
+  Unique::ResultSet callableResultSet;
   bool binaryFormat= false;
   int32_t resultSetScrollType;
   bool rewritten= false;

driver/class/SSPSDirectExec.cpp

@@ -49,31 +49,7 @@ namespace mariadb
   {
   }
 
-  /* For cloning */
-  /*SSPSDirectExec::SSPSDirectExec(
-    Protocol* _connection,
-    int32_t resultSetScrollType)
-    : ServerSidePreparedStatement(_connection, resultSetScrollType)
-  {
-  }*/
-
-  /**
-    * Clone statement.
-    *
-    * @param connection connection
-    * @return Clone statement.
-    * @throws CloneNotSupportedException if any error occur.
-    */
-  /*SSPSDirectExec* SSPSDirectExec::clone(Protocol* connection)
-  {
-    SSPSDirectExec* clone= new SSPSDirectExec(connection, this->resultSetScrollType);
-    clone->metadata.reset(new ResultSetMetaData(*metadata));
-    clone->prepare(*sql);
-
-    return clone;
-  }*/
-
-
+  
   ResultSetMetaData* SSPSDirectExec::getMetaData()
   {
     return metadata.release();
@@ -119,25 +95,6 @@ namespace mariadb
   }
 
 
-  /*void SSPSDirectExec::executeQueryPrologue(ServerPrepareResult* serverPrepareResult)
-  {
-    checkClose();
-  }*/
-
-
-  /*void SSPSDirectExec::getResult()
-  {
-    if (mysql_stmt_field_count(serverPrepareResult->getStatementId()) == 0) {
-      results->addStats(mysql_stmt_affected_rows(serverPrepareResult->getStatementId()), hasMoreResults());
-    }
-    else {
-      serverPrepareResult->reReadColumnInfo();
-      ResultSet *rs= ResultSet::create(results.get(), guard, serverPrepareResult);
-      results->addResultSet(rs, hasMoreResults() || results->getFetchSize() > 0);
-    }
-  }*/
-
-
   bool SSPSDirectExec::executeInternal(int32_t fetchSize)
   {
     checkClose();
@@ -187,90 +144,4 @@ namespace mariadb
       }
     }
   }
-
-  //const char* SSPSDirectExec::getError()
-  //{
-  //  return mysql_stmt_error(serverPrepareResult->getStatementId());
-  //}
-
-  //uint32_t SSPSDirectExec::getErrno()
-  //{
-  //  return mysql_stmt_errno(serverPrepareResult->getStatementId());
-  //}
-
-  //const char* SSPSDirectExec::getSqlState()
-  //{
-  //  return mysql_stmt_sqlstate(serverPrepareResult->getStatementId());
-  //}
-
-  //bool SSPSDirectExec::bind(MYSQL_BIND* param)
-  //{
-  //  this->param= param;
-  //  return mysql_stmt_bind_param(serverPrepareResult->getStatementId(), param) != '\0';
-  //}
-
-//  bool SSPSDirectExec::sendLongData(uint32_t paramNum, const char* data, std::size_t length)
-//  {
-//    return mysql_stmt_send_long_data(serverPrepareResult->getStatementId(), paramNum, data, static_cast<unsigned long>(length)) != '\0';
-//  }
-//
-//
-//  bool SSPSDirectExec::hasMoreResults()
-//  {
-//    return results && results->hasMoreResults(guard);//It maybe still safer to call this || mysql_stmt_more_results(serverPrepareResult->getStatementId());
-//  }
-//
-//
-//  void SSPSDirectExec::moveToNextResult()
-//  {
-//    guard->moveToNextResult(results.get(), serverPrepareResult);
-//  }
-//
-////----------------------------- For param callbacks ------------------------------
-// 
-//  bool SSPSDirectExec::setParamCallback(ParamCodec* callback, uint32_t param)
-//  {
-//    if (param == uint32_t(-1)) {
-//      parRowCallback= callback;
-//      if (callback != nullptr) {
-//        mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_USER_DATA, (void*)this);
-//        return mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_PARAM,
-//          (const void*)withRowCheckCallback);
-//      }
-//      else {
-//        // Let's say - NULL as row callbback resets everything. That seems to be least ambiguous behavior
-//        mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_USER_DATA, (void*)NULL);
-//        return mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_PARAM,
-//          (const void*)NULL);
-//      }
-//    }
-//    
-//    if (param >= serverPrepareResult->getParamCount()) {
-//      throw SQLException("Invalid parameter number");
-//    }
-//
-//    parColCodec.insert({param, callback});
-//    if (parRowCallback == nullptr && parColCodec.size() == 1) {
-//      // Needed not to overwrite callback with row check
-//      mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_USER_DATA, (void*)this);
-//      return mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_PARAM, (const void*)defaultParamCallback);
-//    }
-//    return false;
-//  }
-//
-//
-//  bool SSPSDirectExec::setCallbackData(void * data)
-//  {
-//    callbackData= data;
-//    // if C/C does not support callbacks 
-//    return mysql_stmt_attr_set(serverPrepareResult->getStatementId(), STMT_ATTR_CB_USER_DATA, (void*)this);
-//  }
-//
-//  /* Checking if next result is the last one. That would mean, that the current is out params.
-//   * Method does not do any other checks and asusmes they are done by caller - i.e. it's callable result
-//   */
-//  bool SSPSDirectExec::isOutParams()
-//  {
-//    return results->nextIsLast(guard);
-//  }
 } // namespace mariadb

driver/ma_dll.c

@@ -19,6 +19,9 @@
 #include "ma_platform_win32.h"
 #include <mysql.h>
 
+void DriverGlobalInit(void);
+void DriverGlobalClean(void);
+
 BOOL __stdcall DllMain ( HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved )
 {
   switch (fdwReason) {

driver/ma_helper.cpp

@@ -869,14 +869,14 @@ void* GetBindOffset(MADB_Header& Header, void* Ptr, SQLULEN RowNumber, std::size
   if (Header.BindType == SQL_BIND_BY_COLUMN ||
     Header.BindType == SQL_PARAM_BIND_BY_COLUMN)
   {
-    BindOffset+= PtrSize * RowNumber;
+    BindOffset+= PtrSize*RowNumber;
   }
   else
   {
-    BindOffset+= Header.BindType * RowNumber;
+    BindOffset+= Header.BindType*RowNumber;
   }
 
-  return (char *)Ptr + BindOffset;
+  return (char*)Ptr + BindOffset;
 }
 
 /* Checking if column ignored in all bound rows. Should hel*/

driver/ma_statement.cpp

@@ -83,10 +83,9 @@ void MADB_CloseCursor(MADB_Stmt *Stmt)
       MDBUG_C_PRINT(Stmt->Connection, "Closing resultset", Stmt->stmt.get());
       try
       {
-        // TODO: that's not right to mess here with Protocol's lock. Protocol should take care of that
         Stmt->rs.reset();
-        if (Stmt->stmt->hasMoreResults()) {
-          Stmt->Connection->guard->skipAllResults();
+        while (Stmt->stmt->getMoreResults()) {
+          //Stmt->Connection->guard->skipAllResults();
         }
       }
       catch (...)
@@ -1339,7 +1338,6 @@ SQLRETURN MADB_StmtExecute(MADB_Stmt *Stmt, bool ExecDirect)
       Stmt->stmt.reset(ssps);
     }
     catch (int32_t) {
-      // going further with csps
     }
   }
 

test/bulk.c

@@ -610,7 +610,7 @@ ODBC_TEST(t_odbc149)
   SQLWCHAR w[][8]= {{'x', 'x', 'x', 0}, {'y', 'y', 0}, {'z', 'z', 'z', 'a', 0}}, wBuf[16];
 
   OK_SIMPLE_STMT(Stmt, "DROP TABLE IF EXISTS odbc149");
-  OK_SIMPLE_STMT(Stmt, "CREATE TABLE odbc149 (id int not null, ts timestamp, c varchar(16), b tinyblob, w varchar(16))");
+  OK_SIMPLE_STMT(Stmt, "CREATE TABLE odbc149 (id INT NOT NULL, ts TIMESTAMP, c VARCHAR(16), b TINYBLOB, w VARCHAR(16))");
 
   /* This cursor closing is required, otherwise DM(not on Windows) freaks out */
   CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
@@ -619,7 +619,8 @@ ODBC_TEST(t_odbc149)
   CHECK_STMT_RC(Stmt, SQLSetStmtAttr(Stmt, SQL_ATTR_PARAMSET_SIZE, (SQLPOINTER)MAODBC_ROWS, 0));
 
   CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, 1, SQL_PARAM_INPUT, SQL_C_LONG, SQL_INTEGER, 0, 0, id, 0, NULL));
-  CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, 2, SQL_PARAM_INPUT, SQL_C_TIMESTAMP, SQL_TIMESTAMP, 0, 0, Val, tsLen[0]/* Garbage as buffer len */, tsLen));
+  CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, 2, SQL_PARAM_INPUT, SQL_C_TIMESTAMP, SQL_TIMESTAMP, 0, 0, Val,
+    tsLen[0]/* Garbage as buffer len */, tsLen));
   CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, 3, SQL_PARAM_INPUT, SQL_C_CHAR, SQL_CHAR, 0, 0, c, sizeof(c[0]), cInd));
   CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, 4, SQL_PARAM_INPUT, SQL_C_BINARY, SQL_BINARY, 0, 0, b, sizeof(b[0]), bLen));
   CHECK_STMT_RC(Stmt, SQLBindParameter(Stmt, 5, SQL_PARAM_INPUT, SQL_C_WCHAR, SQL_WCHAR, 0, 0, w, sizeof(w[0]), cInd));

test/catalog1.c

@@ -39,14 +39,14 @@ ODBC_TEST(my_columns_null)
     (SQLCHAR *)"my_column_null", SQL_NTS,
     NULL, SQL_NTS));
 
-  CHECK_STMT_RC(Stmt, SQLRowCount(Stmt, &rowCount));
-
-  is_num(rowCount, 2);
+  if (!RSSTREAMING)
+  {
+    CHECK_STMT_RC(Stmt, SQLRowCount(Stmt, &rowCount));
+    is_num(rowCount, 2);
+  }
 
   is_num(2, my_print_non_format_result(Stmt));
 
-  CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
-
   OK_SIMPLE_STMT(Stmt, "DROP TABLE IF EXISTS my_column_null");
 
   return OK;
@@ -107,8 +107,11 @@ ODBC_TEST(my_table_dbs)
   /* Added calls to SQLRowCount just to have tests of it with SQLTables. */
   CHECK_STMT_RC(Stmt, SQLRowCount(Stmt, &rowCount));
   nrows = my_print_non_format_result(Stmt);
-
-  is_num(rowCount, nrows);
+  // With RS streaming SQLRowCount can't return number of rows in the RS
+  if (!RSSTREAMING)
+  {
+    is_num(rowCount, nrows);
+  }
   CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
 
   CHECK_STMT_RC(Stmt, SQLTables(Stmt,(SQLCHAR *)SQL_ALL_CATALOGS, SQL_NTS, "", 0, "", 0,
@@ -117,12 +120,13 @@ ODBC_TEST(my_table_dbs)
   is_num(nrows, my_print_non_format_result(Stmt));
   CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
 
-  CHECK_STMT_RC(Stmt, SQLTables(Stmt,(SQLCHAR *)my_schema, (SQLSMALLINT)strlen(my_schema), NULL, 0, NULL, 0, NULL, 0));
-
-  CHECK_STMT_RC(Stmt, SQLRowCount(Stmt, &rowCount));
-  is_num(rowCount, my_print_non_format_result(Stmt));
-
-  CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
+  if (!RSSTREAMING)
+  {
+    CHECK_STMT_RC(Stmt, SQLTables(Stmt, (SQLCHAR *)my_schema, (SQLSMALLINT)strlen(my_schema), NULL, 0, NULL, 0, NULL, 0));
+    CHECK_STMT_RC(Stmt, SQLRowCount(Stmt, &rowCount));
+    is_num(rowCount, my_print_non_format_result(Stmt));
+    CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
+  }
 
   /* test fails on Win2003 x86 w/DM if len=5, SQL_NTS is used instead */
   CHECK_STMT_RC(Stmt, SQLTables(Stmt,(SQLCHAR *)"mysql", SQL_NTS, NULL, 0, NULL, 0, NULL, 0));
@@ -131,7 +135,6 @@ ODBC_TEST(my_table_dbs)
   CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
 
   CHECK_STMT_RC(Stmt, SQLTables(Stmt, (SQLCHAR *)"%", 1, "", 0, "", 0, NULL, 0));
-
   CHECK_STMT_RC(Stmt, SQLFetch(Stmt));
 
   memset(database,0,sizeof(database));
@@ -421,10 +424,8 @@ ODBC_TEST(tmysql_specialcols)
   OK_SIMPLE_STMT(Stmt,"CREATE TABLE tmysql_specialcols(col1 int primary key, col2 varchar(30), col3 int)");
 
 
-  OK_SIMPLE_STMT(Stmt,"create index tmysql_ind1 on tmysql_specialcols(col1)");
-    
+  OK_SIMPLE_STMT(Stmt,"CREATE INDEX tmysql_ind1 ON tmysql_specialcols(col1)");
   OK_SIMPLE_STMT(Stmt,"INSERT INTO tmysql_specialcols VALUES(100,'venu',1)");
-
   OK_SIMPLE_STMT(Stmt,"INSERT INTO tmysql_specialcols VALUES(200,'MySQL',2)");
 
   CHECK_DBC_RC(Connection, SQLTransact(NULL,Connection,SQL_COMMIT));
@@ -445,7 +446,7 @@ ODBC_TEST(tmysql_specialcols)
 
   CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt,SQL_CLOSE));
 
-  OK_SIMPLE_STMT(Stmt,"drop table tmysql_specialcols");
+  OK_SIMPLE_STMT(Stmt,"DROP TABLE tmysql_specialcols");
 
   CHECK_DBC_RC(Connection, SQLTransact(NULL,Connection,SQL_COMMIT));