Fix of how parameter codec process insdicator values.

Now it's processed by single coded set for the whole row as it is not
individual for different column/codec types. It checks and sets
inddicator values, e.g. in case of NULL value indicator.
This row-level codec is now set always.
Slightly optimized how descriptor values are read in codecs.

Author: lawrinn

driver/class/ServerSidePreparedStatement.cpp

@@ -326,22 +326,31 @@ namespace mariadb
 
   my_bool* defaultParamCallback(void* data, MYSQL_BIND* bind, uint32_t row_nr)
   {
+    // Assuming paramset skip is set in the indicator of 1st column
+    if (bind->u.indicator && *bind->u.indicator == STMT_INDICATOR_IGNORE_ROW) {
+      return '\0';
+    }
     // We can't let the callback to throw - we have to intercept, as otherwise we are guaranteed to have
     // the protocol broken
     try
     {
       ServerSidePreparedStatement *stmt= reinterpret_cast<ServerSidePreparedStatement*>(data);
 
       for (uint32_t i= 0; i < stmt->getParamCount(); ++i) {
+        auto current= bind + i;
+        if (current->u.indicator && *current->u.indicator == STMT_INDICATOR_NULL) {
+          continue;
+        }
         const auto& it= stmt->parColCodec.find(i);
-        /* Let's assume for now, that each column has to have a codec. But still we don't use vector and don't throw exception if not all of them have
-       * For such columns we could assume, that we have normal arrays that we need to pass, and this function could do that simple job instead of callback.
-       * But indicator arrow would still be needed. Thus maybe callback is still better. Another option could be - using default value(indicator for that)
-       * But here there is another problem - the common "row" callback could take care of such columns
-       * More likely that app will need to use array
-       */
+        /* Let's assume for now, that each column has to have a codec. But still we don't use vector and don't throw exception
+         * if not all of them have. For such columns we could assume, that we have normal arrays that we need to pass, and this
+         * function could do that simple job instead of callback. But indicator array would still be needed. Thus maybe callback
+         * is still better. Another option could be - using default value(indicator for that)
+         * But here there is another problem - the common "row" callback could take care of such columns
+         * More likely that app will need to use array
+         */
         if (it != stmt->parColCodec.end()) {
-          if ((*it->second)(stmt->callbackData, bind + i, i, row_nr)) {
+          if ((*it->second)(stmt->callbackData, current, i, row_nr)) {
             return (my_bool*)&error;
           }
         }
@@ -351,7 +360,7 @@ namespace mariadb
     {
       return (my_bool*)&error;
     }
-    return NULL;
+    return '\0';
   }
 
 

driver/interface/PreparedStatement.cpp

@@ -26,8 +26,12 @@
 
 namespace mariadb
 {
-  /*const*/ char paramIndicatorNone= '\0', paramIndicatorNull= STMT_INDICATOR_NULL, paramIndicatorNts= STMT_INDICATOR_NTS, paramIndicatorIgnore= STMT_INDICATOR_IGNORE,
-    paramIndicatorIgnoreRow= STMT_INDICATOR_IGNORE_ROW;
+  /*const*/ char paramIndicatorNone=      '\0',
+                 paramIndicatorNull=      STMT_INDICATOR_NULL, 
+                 paramIndicatorDef=       STMT_INDICATOR_DEFAULT,
+                 paramIndicatorNts=       STMT_INDICATOR_NTS,
+                 paramIndicatorIgnore=    STMT_INDICATOR_IGNORE,
+                 paramIndicatorIgnoreRow= STMT_INDICATOR_IGNORE_ROW;
   /**
    * Constructor. Base class that permits setting parameters for client and server PrepareStatement.
    *

driver/interface/PreparedStatement.h

@@ -28,7 +28,7 @@
 #include "class/ResultSetMetaData.h"
 namespace mariadb
 {
-  extern char paramIndicatorNone, paramIndicatorNull, paramIndicatorNts, paramIndicatorIgnore,
+  extern char paramIndicatorNone, paramIndicatorNull, paramIndicatorDef, paramIndicatorNts, paramIndicatorIgnore,
     paramIndicatorNull, paramIndicatorIgnoreRow;
 // The functor returns bool only to fit the need to check if to skip paramset, and I don't really need to introduce 2 diff callbacks for that, so one "universal"
 class ParamCodec
@@ -43,7 +43,10 @@ class PCodecCallable : public ParamCodec
 {
 public:
   virtual ~PCodecCallable() {}
-  virtual bool operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr) { return T(data, bind, col_nr, row_nr); }
+  virtual bool operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
+  {
+    return T(data, bind, col_nr, row_nr);
+  }
 };
 
 

driver/ma_bulk.cpp

@@ -173,13 +173,13 @@ void MADB_SetBulkOperLengthArr(MADB_Stmt *Stmt, MADB_DescRecord *CRec, SQLLEN *O
     }
 
     if ((OctetLengthPtr != nullptr && OctetLengthPtr[row] == SQL_NULL_DATA)
-      || (IndicatorPtr != nullptr && IndicatorPtr[row] != SQL_NULL_DATA))
+      || (IndicatorPtr != nullptr && IndicatorPtr[row] == SQL_NULL_DATA))
     {
       MADB_SetIndicatorValue(Stmt, MaBind, row, SQL_NULL_DATA);
       continue;
     }
     if ((OctetLengthPtr != nullptr && OctetLengthPtr[row] == SQL_COLUMN_IGNORE)
-      || (IndicatorPtr != nullptr && IndicatorPtr[row] != SQL_COLUMN_IGNORE))
+      || (IndicatorPtr != nullptr && IndicatorPtr[row] == SQL_COLUMN_IGNORE))
     {
       MADB_SetIndicatorValue(Stmt, MaBind, row, SQL_COLUMN_IGNORE);
       continue;
@@ -270,9 +270,8 @@ void MADB_SetIndicatorValue(MADB_Stmt *Stmt, MYSQL_BIND *MaBind, unsigned int ro
 /* }}} */
 
 /* {{{  */
-bool MADB_Stmt::setParamRowCallback(ParamCodec * callback)
+bool MADB_Stmt::setParamRowCallback(ParamCodec* callback)
 {
-  //TODO should do all param codecs. and the operation below should be done once when 1st param codec is added, and not "row callback"
   if (paramCodec.capacity() < stmt->getParamCount()) {
     paramCodec.reserve(stmt->getParamCount());
   }
@@ -368,14 +367,7 @@ void MADB_Stmt::setupBulkCallbacks(uint32_t parNr, MADB_DescRecord* CRec, MADB_D
   auto parIt= paramCodec.begin() + parNr;
   paramCodec.insert(parIt, Unique::ParamCodec(nullptr));
   std::size_t dataArrStep= getArrayStep(Apd->Header, CRec->OctetLength);
-  /*if (Apd->Header.ArrayStatusPtr != nullptr && Apd->Header.ArrayStatusPtr[row] == SQL_PARAM_IGNORE)
-  {
-    continue;
-  }
-  if (MaBind->u.indicator && MaBind->u.indicator[row] > STMT_INDICATOR_NONE)
-  {
-    continue;
-  }*/
+
   if (MADB_AppBufferCanBeUsed(CRec->ConciseType, SqlRec->ConciseType)) {
     paramCodec[parNr].reset(new FixedSizeCopyCodec(it));
   }
@@ -519,15 +511,14 @@ SQLRETURN MADB_ExecuteBulk(MADB_Stmt *Stmt, unsigned int ParamOffset)
       }
     }
   }
-
+  if (useCallbacks)
+  {
+    Stmt->stmt->setParamCallback(new IndicatorMapper(Stmt));
+  }
   /* just to do this once, and to use already allocated indicator array */
   if (Stmt->Bulk.HasRowsToSkip)
   {
-    if (useCallbacks)
-    {
-      Stmt->stmt->setParamCallback(new IgnoreRow(Stmt->Apd->Header.ArrayStatusPtr, Stmt->ArrayOffset));
-    }
-    else
+    if (!useCallbacks)
     {
       SQLULEN row, Start= Stmt->ArrayOffset;
       if (IndIdx == (unsigned int)-1)

driver/ma_codec.cpp

@@ -34,6 +34,44 @@ namespace mariadb
     }
   }
 
+  IndicatorMapper::IndicatorMapper(MADB_Stmt *Stmt) :
+    statusArr(Stmt->Apd->Header.ArrayStatusPtr + Stmt->ArrayOffset),
+    indPtr(Stmt->Ipd->Header.Count, nullptr), // For all statement parameters, not only for bound by the application
+                                             // (Ipd vs Apd)
+    arrStep(getArrayStep(Stmt->Apd->Header, sizeof(SQLLEN)))
+  {
+    for (auto i= 0; i < Stmt->Ipd->Header.Count; ++i) {
+      auto CRec= MADB_DescGetInternalRecord(Stmt->Apd, i, MADB_DESC_READ);
+      if (CRec->inUse) {
+        indPtr[i]= static_cast<SQLLEN*>(GetBindOffset(Stmt->Ipd->Header, CRec->IndicatorPtr, 0, sizeof(SQLLEN)));
+      }
+    }
+  }
+
+  bool IndicatorMapper::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr) {
+    if (statusArr && statusArr[row_nr] == SQL_PARAM_IGNORE) {
+      bind->u.indicator= &paramIndicatorIgnoreRow;
+      return false;
+    }
+    else {
+      bind->u.indicator= &paramIndicatorNone;
+    }
+    MADB_Stmt *stmt= static_cast<MADB_Stmt*>(data);
+    for (auto i= 0; i < MADB_STMT_PARAM_COUNT(stmt); ++i) {
+      auto indicator= indPtr[i];
+      if (indicator == nullptr) {
+        //(bind + i)->u.indicator= &paramIndicatorDef;
+      }
+      else {
+        if (*indicator == SQL_NULL_DATA) {
+          (bind + i)->u.indicator= &paramIndicatorNull;
+        }
+        indPtr[i]= indicator + arrStep;
+      }
+    }
+    return false;
+  }
+
 
   bool FixedSizeCopyCodec::operator()(void * data, MYSQL_BIND * bind, uint32_t col_nr, uint32_t row_nr)
   {
@@ -393,12 +431,7 @@ namespace mariadb
     SQLLEN buffCharLen= it.getDescRec()->OctetLength/sizeof(SQLWCHAR), charLen;
 
     if (length == NULL_LENGTH) {
-      if (it.indicator() != nullptr) {
-        *it.indicator()= SQL_NULL_DATA;
-      }  
-      else {
-        *it.length()= SQL_NULL_DATA;
-      }
+      *it.indicator()= SQL_NULL_DATA;
       return;
     }
     if (MADB_ConvertAnsi2Unicode(&Stmt->Connection->Charset, (char *)row, (SQLLEN)length, (SQLWCHAR *)it.value(),
@@ -438,12 +471,7 @@ namespace mariadb
     MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
 
     if (length == NULL_LENGTH) {
-      if (it.indicator() != nullptr) {
-        *it.indicator()= SQL_NULL_DATA;
-      }
-      else {
-        *it.length()= SQL_NULL_DATA;
-      }
+      *it.indicator()= SQL_NULL_DATA;
       return;
     }
 

driver/ma_codec.h

@@ -29,21 +29,14 @@ namespace mariadb
 
 // {'\0', STMT_INDICATOR_NULL, STMT_INDICATOR_NTS, STMT_INDICATOR_IGNORE, STMT_INDICATOR_NULL, STMT_INDICATOR_IGNORE_ROW};
 
-class IgnoreRow : public ParamCodec
+class IndicatorMapper : public ParamCodec
 {
   SQLUSMALLINT *statusArr;
+  std::vector<SQLLEN*> indPtr;
+  std::size_t arrStep;
 public:
-  IgnoreRow(SQLUSMALLINT* ArrStatusPtr, std::size_t initialOffset= 0) : statusArr(ArrStatusPtr + initialOffset) {}
-  bool operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr) override {
-    if (statusArr[row_nr] == SQL_PARAM_IGNORE) {
-      bind->u.indicator= &paramIndicatorIgnoreRow;
-      return false;
-    }
-    else {
-      bind->u.indicator= &paramIndicatorNone;
-    }
-    return true;
-  }
+  IndicatorMapper(MADB_Stmt *Stmt);
+  bool operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr) override;
 };
 
 

driver/ma_desc.cpp

@@ -1085,10 +1085,6 @@ DescArrayIterator::DescArrayIterator(MADB_Header& header, MADB_DescRecord& rec,
   , indicatorPtr(reinterpret_cast<SQLLEN*>(GetBindOffset(header, rec.IndicatorPtr, 0, sizeof(SQLLEN))))
   , lengthOffset(getArrayStep(header, sizeof(SQLLEN)))
 {
-  // Somewhere code depends on that. Maybe makes sense - less "noise"
-  if (indicatorPtr == octetLengthPtr) {
-    indicatorPtr= nullptr;
-  }
 }
 
 
@@ -1098,7 +1094,7 @@ DescArrayIterator::DescArrayIterator(MADB_DescRecord& rec, void * val, std::size
   , valueOffset(valOffset)
   , endPtr(reinterpret_cast<void*>(reinterpret_cast<char*>(valuePtr) + valueOffset*arrSize))
   , octetLengthPtr(len)
-  , indicatorPtr(ind != len ? ind : nullptr)
+  , indicatorPtr(ind)
   , lengthOffset(lenOffset)
 {
 }

driver/ma_desc.h

@@ -78,10 +78,11 @@ class DescArrayIterator
   DescArrayIterator(MADB_Header& header, MADB_DescRecord& rec, SQLSMALLINT i);
   DescArrayIterator(MADB_DescRecord& rec, void* val, std::size_t valOffset, SQLLEN* len, SQLLEN* ind, std::size_t lenOffset, std::size_t arrSize);
   inline void* next() {
-    octetLengthPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(octetLengthPtr) + lengthOffset);
-    if (indicatorPtr) {
+    // The following condition has to be checked before octetLengthPtr is moved to the next value
+    if (indicatorPtr != octetLengthPtr) {
       indicatorPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(indicatorPtr) + lengthOffset);
     }
+    octetLengthPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(octetLengthPtr) + lengthOffset);
     return (valuePtr= (void*)((char*)valuePtr + valueOffset));
   }
   inline void*   value()    { return valuePtr;       }