ODBC-482 Problems with SQLSetPos on block cursor with NULL values.

If position was changed, SQLGetData would still read if value is NULL
from first row in the block cursor. That could lead to different issues
and even crash on SQL_DELETE or in SQLGetData() after SQL_POSITION.
Part of fix is also in the part that has been moved to c/c++ repo -
get() method returned true, i.e. error on NULL field value. Plus
resultset method isNull has been made public.

Author: lawrinn

driver/cppmini

@@ -2756,8 +2756,8 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT     StatementHandle,
   {
     return MADB_SetError(&Stmt->Error, MADB_ERR_HY109, nullptr, 0);
   }
-  /* Will it be set with all dummies? */
-  if (Stmt->result[Offset].is_null && *Stmt->result[Offset].is_null != '\0')
+
+  if (Stmt->rs->isNull(Col_or_Param_Num))
   {
     if (!StrLen_or_IndPtr)
     {
@@ -2985,6 +2985,10 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT     StatementHandle,
         Bind.length=       &Bind.length_value;
 
         Stmt->rs->get(&Bind, Offset, Stmt->CharOffset[Offset]);
+        //Should not happen
+        if (IsNull) {
+          break;
+        }
 
         if (InternalUse)
         {
@@ -2998,16 +3002,16 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT     StatementHandle,
           }
           *StrLen_or_IndPtr= Stmt->Lengths[Offset] - Stmt->CharOffset[Offset];
         }
-
-        MADB_SetError(&Stmt->Error, MADB_ERR_01004, nullptr, 0);
-
-        return SQL_SUCCESS_WITH_INFO;
+        return MADB_SetError(&Stmt->Error, MADB_ERR_01004, nullptr, 0);
       }
 
       if (Stmt->rs->get(&Bind, Offset, CurrentOffset))
       {
-        MADB_SetNativeError(&Stmt->Error, SQL_HANDLE_STMT, Stmt->stmt.get());
-        return Stmt->Error.ReturnValue;
+        return MADB_SetNativeError(&Stmt->Error, SQL_HANDLE_STMT, Stmt->stmt.get());
+      }
+      // Should not happen
+      if (IsNull) {
+        break;
       }
       /* Dirty temporary hack before we know what is going on. Yes, there is nothing more eternal, than temporary
          It's not that bad, after all */
@@ -3160,6 +3164,14 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT     StatementHandle,
     return MADB_SetError(&Stmt->Error, MADB_ERR_22003, oor.what(), 0);
   }
 
+  if (IsNull)
+  {
+    if (!StrLen_or_IndPtr)
+    {
+      return MADB_SetError(&Stmt->Error, MADB_ERR_22002, nullptr, 0);
+    }
+    *StrLen_or_IndPtr = SQL_NULL_DATA;
+  }
   /* Marking fixed length fields to be able to return SQL_NO_DATA on subsequent calls, as standard prescribes
      "SQLGetData cannot be used to return fixed-length data in parts. If SQLGetData is called more than one time
       in a row for a column containing fixed-length data, it returns SQL_NO_DATA for all calls after the first."
@@ -3168,16 +3180,6 @@ SQLRETURN MADB_StmtGetData(SQLHSTMT     StatementHandle,
   {
     Stmt->CharOffset[Offset]= MAX((unsigned long)Bind.buffer_length, Bind.length_value);
   }
-
-  if (IsNull)
-  {
-    if (!StrLen_or_IndPtr)
-    {
-      return MADB_SetError(&Stmt->Error, MADB_ERR_22002, nullptr, 0);
-    }
-    *StrLen_or_IndPtr= SQL_NULL_DATA;
-  }
-
   return Stmt->Error.ReturnValue;
 }
 /* }}} */
@@ -3618,8 +3620,8 @@ SQLRETURN MADB_StmtSetPos(MADB_Stmt* Stmt, SQLSETPOSIROW RowNumber, SQLUSMALLINT
   switch (Operation) {
   case SQL_POSITION:
     {
-      Stmt->Cursor.Position += (RowNumber - 1);
-      MADB_StmtDataSeek(Stmt, Stmt->Cursor.Position);
+      //Stmt->Cursor.Position += (RowNumber - 1);
+      MADB_StmtDataSeek(Stmt, Stmt->Cursor.Position + RowNumber - 1);
     }
   case SQL_REFRESH:
       /* todo */
@@ -3887,6 +3889,7 @@ SQLRETURN MADB_StmtSetPos(MADB_Stmt* Stmt, SQLSETPOSIROW RowNumber, SQLUSMALLINT
           std::lock_guard<std::mutex> localScopeLock(Stmt->Connection->guard->getLock());
           Stmt->Connection->guard->safeRealQuery(DynamicStmt);
           Stmt->AffectedRows += mysql_affected_rows(Stmt->Connection->mariadb);
+          MADB_SETPOS_AGG_RESULT(result, SQL_SUCCESS);
         }
         ++Start;
         DynamicStmt.erase(baseStmtLen);

driver/ma_statement.cpp

@@ -2184,7 +2184,7 @@ ODBC_TEST(t_auto_reconnect)
   SQLHDBC  Hdbc;
   SQLHSTMT Hstmt;
 
-  //SKIPIF(IsMaxScale || IsSkySqlHa, "Doesn't make sense with Maxscale, as we kill connection from MaxScale to one of servers, and our connection to MaxScale persists");
+  SKIPIF(IsMaxScale || IsSkySqlHa, "Doesn't make sense with Maxscale, as we kill connection from MaxScale to one of servers, and our connection to MaxScale persists");
   /* Create a new connection with reconnect option on that we will kill to check if it still functioning */
   CHECK_ENV_RC(Env, SQLAllocConnect(Env, &Hdbc));
   Hstmt = DoConnect(Hdbc, FALSE, NULL, NULL, NULL, 0, NULL, NULL, NULL, "AUTO_RECONNECT=1");

test/basic.c

@@ -802,30 +802,36 @@ ODBC_TEST(odbc476)
 
   FAIL_IF(SQLFetchScroll(Stmt, SQL_FETCH_NEXT, 1) != SQL_NO_DATA_FOUND, "no data found expected");
 
-  // The commented does not work, but it never did it seems. So, leaving it so far
-  //CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_UNBIND));
-  //CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
-
-  ///* Now the same but on block cursor. Not null values should be deleted, SQLSetPos should return SQL_SUCCESS_WITH_INFO */
-  //OK_SIMPLE_STMT(Stmt, "SELECT string_null FROM t_odbc476 ORDER BY string_null DESC");
-  //CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 1, SQL_C_CHAR, szData, sizeof(szData[0]), nlen));
-  //CHECK_STMT_RC(Stmt, SQLFetchScroll(Stmt, SQL_FETCH_ABSOLUTE, 1));
-  //// This fails with "can't build index" if id field or all fields are not selected
-  //EXPECT_STMT(Stmt, SQLSetPos(Stmt, 0, SQL_DELETE, SQL_LOCK_NO_CHANGE), SQL_SUCCESS_WITH_INFO);
-
-  //CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
-
-  //OK_SIMPLE_STMT(Stmt, "SELECT * FROM t_odbc476 ORDER BY id");
-  //CHECK_STMT_RC(Stmt, SQLSetStmtAttr(Stmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)2, 0));
-  //CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 1, SQL_C_LONG, &nData, 0, nrow));
-  //CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 2, SQL_C_CHAR, szData, sizeof(szData[0]), nlen));
-  //CHECK_STMT_RC(Stmt, SQLFetchScroll(Stmt, SQL_FETCH_ABSOLUTE, 1));
-
-  //is_num(nData[0], 1);
-  //is_num(nlen[0], SQL_NULL_DATA);
-  //is_num(nData[1], 4);
-  //is_num(nlen[1], SQL_NULL_DATA);
-  //FAIL_IF(SQLFetchScroll(Stmt, SQL_FETCH_NEXT, 1) != SQL_NO_DATA_FOUND, "no data found expected");
+  CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_UNBIND));
+  CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
+
+  /* Now the same but on block cursor. Not null values should be deleted, SQLSetPos should return SQL_SUCCESS_WITH_INFO */
+  OK_SIMPLE_STMT(Stmt, "SELECT string_null FROM t_odbc476 ORDER BY string_null DESC"); // To make NULL go last
+  CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 1, SQL_C_CHAR, szData, sizeof(szData[0]), nlen));
+  CHECK_STMT_RC(Stmt, SQLFetchScroll(Stmt, SQL_FETCH_ABSOLUTE, 1));
+
+  // Setting position to first NULL value. From here this is also and mostly test for ODBC-482
+  CHECK_STMT_RC(Stmt, SQLSetPos(Stmt, 3, SQL_POSITION, SQL_LOCK_NO_CHANGE));
+  // Now we should be able to SQLGetData from that row. Gonna use the same buffer
+  nlen[2]= 0;
+  CHECK_STMT_RC(Stmt, SQLGetData(Stmt, 1, SQL_C_CHAR, &szData[2], sizeof(szData[2]), &nlen[2]));
+  is_num(SQL_NULL_DATA, nlen[2]);
+  // This fails with "can't build index" if id field or all fields are not selected
+  EXPECT_STMT(Stmt, SQLSetPos(Stmt, 0, SQL_DELETE, SQL_LOCK_NO_CHANGE), SQL_SUCCESS_WITH_INFO);
+
+  CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_CLOSE));
+
+  OK_SIMPLE_STMT(Stmt, "SELECT * FROM t_odbc476 ORDER BY id");
+  CHECK_STMT_RC(Stmt, SQLSetStmtAttr(Stmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)2, 0));
+  CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 1, SQL_C_LONG, &nData, 0, nrow));
+  CHECK_STMT_RC(Stmt, SQLBindCol(Stmt, 2, SQL_C_CHAR, szData, sizeof(szData[0]), nlen));
+  CHECK_STMT_RC(Stmt, SQLFetchScroll(Stmt, SQL_FETCH_ABSOLUTE, 1));
+
+  is_num(nData[0], 1);
+  is_num(nlen[0], SQL_NULL_DATA);
+  is_num(nData[1], 4);
+  is_num(nlen[1], SQL_NULL_DATA);
+  FAIL_IF(SQLFetchScroll(Stmt, SQL_FETCH_NEXT, 1) != SQL_NO_DATA_FOUND, "no data found expected");
 
   CHECK_STMT_RC(Stmt, SQLSetStmtAttr(Stmt, SQL_ATTR_ROW_ARRAY_SIZE, (SQLPOINTER)1, 0));
   CHECK_STMT_RC(Stmt, SQLFreeStmt(Stmt, SQL_UNBIND));
@@ -846,7 +852,7 @@ MA_ODBC_TESTS my_tests[]=
   {my_zero_irow_update, "my_zero_irow_update",      NORMAL },
   {my_zero_irow_delete, "my_zero_irow_delete",       NORMAL},
   {my_dynamic_cursor, "my_dynamic_cursor",           NORMAL},
-  {odbc476, "odbc476-not-enough-info",           NORMAL},
+  {odbc476, "odbc476_odbc482-not-enough-info",           NORMAL},
   {NULL, NULL}
 };