NDB wl-2151 Fix bounds setting table handler vs TUX

mysql-test/ndb/ndb_range_bounds.pl:
  wl-2151 Fix bounds setting table handler vs TUX
ndb/include/kernel/signaldata/TuxBound.hpp:
  wl-2151 Fix bounds setting table handler vs TUX
ndb/include/ndbapi/NdbIndexScanOperation.hpp:
  wl-2151 Fix bounds setting table handler vs TUX
ndb/src/kernel/blocks/dblqh/DblqhMain.cpp:
  wl-2151 Fix bounds setting table handler vs TUX
ndb/src/kernel/blocks/dbtux/DbtuxCmp.cpp:
  wl-2151 Fix bounds setting table handler vs TUX
ndb/src/kernel/blocks/dbtux/DbtuxScan.cpp:
  wl-2151 Fix bounds setting table handler vs TUX
ndb/test/ndbapi/testOIBasic.cpp:
  wl-2151 Fix bounds setting table handler vs TUX
sql/ha_ndbcluster.cc:
  wl-2151 Fix bounds setting table handler vs TUX
sql/ha_ndbcluster.h:
  wl-2151 Fix bounds setting table handler vs TUX

6 files changed, 105 insertions, 134 deletions

diff --git a/ndb/include/kernel/signaldata/TuxBound.hpp b/ndb/include/kernel/signaldata/TuxBound.hpp
index 1f256150573..87ce3c3c098 100644
--- a/ndb/include/kernel/signaldata/TuxBound.hpp
+++ b/ndb/include/kernel/signaldata/TuxBound.hpp
@@ -48,7 +48,6 @@ private:
   Uint32 tuxScanPtrI;
   /*
    * Number of words of bound info included after fixed signal data.
-   * Starts with 5 unused words (word 0 is length used by LQH).
    */
   Uint32 boundAiLength;
 };
diff --git a/ndb/include/ndbapi/NdbIndexScanOperation.hpp b/ndb/include/ndbapi/NdbIndexScanOperation.hpp
index a854cb58665..66b3fc9d43b 100644
--- a/ndb/include/ndbapi/NdbIndexScanOperation.hpp
+++ b/ndb/include/ndbapi/NdbIndexScanOperation.hpp
@@ -56,27 +56,11 @@ public:
   }
 
   /**
-   * @name Define Range Scan
-   *
-   * A range scan is a scan on an ordered index.  The operation is on
-   * the index table but tuples are returned from the primary table.
-   * The index contains all tuples where at least one index key has not
-   * null value.
-   *
-   * A range scan is currently opened via a normal open scan method.
-   * Bounds can be defined for each index key.  After setting bounds,
-   * usual scan methods can be used (get value, interpreter, take over).
-   * These operate on the primary table.
-   *
-   * @{
-   */
-
-  /**
    * Type of ordered index key bound.  The values (0-4) will not change
    * and can be used explicitly (e.g. they could be computed).
    */
   enum BoundType {
-    BoundLE = 0,        ///< lower bound,
+    BoundLE = 0,        ///< lower bound
     BoundLT = 1,        ///< lower bound, strict
     BoundGE = 2,        ///< upper bound
     BoundGT = 3,        ///< upper bound, strict
@@ -86,20 +70,28 @@ public:
   /**
    * Define bound on index key in range scan.
    *
-   * Each index key can have lower and/or upper bound, or can be set
-   * equal to a value.  The bounds can be defined in any order but
-   * a duplicate definition is an error.
+   * Each index key can have lower and/or upper bound.  Setting the key
+   * equal to a value defines both upper and lower bounds.  The bounds
+   * can be defined in any order.  Conflicting definitions is an error.
+   *
+   * For equality, it is better to use BoundEQ instead of the equivalent
+   * pair of BoundLE and BoundGE.  This is especially true when table
+   * distribution key is an initial part of the index key.
    *
-   * The bounds must specify a single range i.e. they are on an initial
-   * sequence of index keys and the condition is equality for all but
-   * (at most) the last key which has a lower and/or upper bound.
+   * The sets of lower and upper bounds must be on initial sequences of
+   * index keys.  All but possibly the last bound must be non-strict.
+   * So "a >= 2 and b > 3" is ok but "a > 2 and b >= 3" is not.
+   *
+   * The scan may currently return tuples for which the bounds are not
+   * satisfied.  For example, "a <= 2 and b <= 3" scans the index up to
+   * (a=2, b=3) but also returns any (a=1, b=4).
    *
    * NULL is treated like a normal value which is less than any not-NULL
-   * value and equal to another NULL value.  To search for NULL use
+   * value and equal to another NULL value.  To compare against NULL use
    * setBound with null pointer (0).
    *
-   * An index stores also all-NULL keys (this may become optional).
-   * Doing index scan with empty bound set returns all table tuples.
+   * An index stores also all-NULL keys.  Doing index scan with empty
+   * bound set returns all table tuples.
    *
    * @param attrName    Attribute name, alternatively:
    * @param anAttrId    Index column id (starting from 0)
@@ -117,8 +109,6 @@ public:
    */
   int setBound(Uint32 anAttrId, int type, const void* aValue, Uint32 len = 0);
 
-  /** @} *********************************************************************/
-
   /**
    * Reset bounds and put operation in list that will be
    *   sent on next execute
diff --git a/ndb/src/kernel/blocks/dblqh/DblqhMain.cpp b/ndb/src/kernel/blocks/dblqh/DblqhMain.cpp
index c5e2bd900e9..ed81f0735cb 100644
--- a/ndb/src/kernel/blocks/dblqh/DblqhMain.cpp
+++ b/ndb/src/kernel/blocks/dblqh/DblqhMain.cpp
@@ -7683,7 +7683,6 @@ void Dblqh::accScanConfScanLab(Signal* signal)
   Uint32 boundAiLength = tcConnectptr.p->primKeyLen - 4;
   if (scanptr.p->rangeScan) {
     jam();
-    // bound info length is in first of the 5 header words
     TuxBoundInfo* const req = (TuxBoundInfo*)signal->getDataPtrSend();
     req->errorCode = RNIL;
     req->tuxScanPtrI = scanptr.p->scanAccPtr;
diff --git a/ndb/src/kernel/blocks/dbtux/DbtuxCmp.cpp b/ndb/src/kernel/blocks/dbtux/DbtuxCmp.cpp
index 549720cc17c..ddab77b97b5 100644
--- a/ndb/src/kernel/blocks/dbtux/DbtuxCmp.cpp
+++ b/ndb/src/kernel/blocks/dbtux/DbtuxCmp.cpp
@@ -87,21 +87,23 @@ Dbtux::cmpSearchKey(const Frag& frag, unsigned& start, ConstData searchKey, Cons
 /*
  * Scan bound vs node prefix or entry.
  *
- * Compare lower or upper bound and index attribute data.  The attribute
- * data may be partial in which case CmpUnknown may be returned.
- * Returns -1 if the boundary is to the left of the compared key and +1
- * if the boundary is to the right of the compared key.
+ * Compare lower or upper bound and index entry data.  The entry data
+ * may be partial in which case CmpUnknown may be returned.  Otherwise
+ * returns -1 if the bound is to the left of the entry and +1 if the
+ * bound is to the right of the entry.
  *
- * To get this behaviour we treat equality a little bit special.  If the
- * boundary is a lower bound then the boundary is to the left of all
- * equal keys and if it is an upper bound then the boundary is to the
- * right of all equal keys.
+ * The routine is similar to cmpSearchKey, but 0 is never returned.
+ * Suppose all attributes compare equal.  Recall that all bounds except
+ * possibly the last one are non-strict.  Use the given bound direction
+ * (0-lower 1-upper) and strictness of last bound to return -1 or +1.
  *
- * When searching for the first key we are using the lower bound to try
- * to find the first key that is to the right of the boundary.  Then we
- * start scanning from this tuple (including the tuple itself) until we
- * find the first key which is to the right of the boundary.  Then we
- * stop and do not include that key in the scan result.
+ * Following example illustrates this.  We are at (a=2, b=3).
+ *
+ * dir  bounds                  strict          return
+ * 0    a >= 2 and b >= 3       no              -1
+ * 0    a >= 2 and b >  3       yes             +1
+ * 1    a <= 2 and b <= 3       no              +1
+ * 1    a <= 2 and b <  3       yes             -1
  */
 int
 Dbtux::cmpScanBound(const Frag& frag, unsigned dir, ConstData boundInfo, unsigned boundCount, ConstData entryData, unsigned maxlen)
@@ -111,12 +113,7 @@ Dbtux::cmpScanBound(const Frag& frag, unsigned dir, ConstData boundInfo, unsigne
   ndbrequire(dir <= 1);
   // number of words of data left
   unsigned len2 = maxlen;
-  /*
-   * No boundary means full scan, low boundary is to the right of all
-   * keys.  Thus we should always return -1.  For upper bound we are to
-   * the right of all keys, thus we should always return +1.  We achieve
-   * this behaviour by initializing type to 4.
-   */
+  // in case of no bounds, init last type to something non-strict
   unsigned type = 4;
   while (boundCount != 0) {
     if (len2 <= AttributeHeaderSize) {
@@ -124,7 +121,7 @@ Dbtux::cmpScanBound(const Frag& frag, unsigned dir, ConstData boundInfo, unsigne
       return NdbSqlUtil::CmpUnknown;
     }
     len2 -= AttributeHeaderSize;
-    // get and skip bound type
+    // get and skip bound type (it is used after the loop)
     type = boundInfo[0];
     boundInfo += 1;
     if (! boundInfo.ah().isNULL()) {
@@ -166,30 +163,7 @@ Dbtux::cmpScanBound(const Frag& frag, unsigned dir, ConstData boundInfo, unsigne
     entryData += AttributeHeaderSize + entryData.ah().getDataSize();
     boundCount -= 1;
   }
-  if (dir == 0) {
-    jam();
-    /*
-     * Looking for the lower bound.  If strict lower bound then the
-     * boundary is to the right of the compared key and otherwise (equal
-     * included in range) then the boundary is to the left of the key.
-     */
-    if (type == 1) {
-      jam();
-      return +1;
-    }
-    return -1;
-  } else {
-    jam();
-    /*
-     * Looking for the upper bound.  If strict upper bound then the
-     * boundary is to the left of all equal keys and otherwise (equal
-     * included in the range) then the boundary is to the right of all
-     * equal keys.
-     */
-    if (type == 3) {
-      jam();
-      return -1;
-    }
-    return +1;
-  }
+  // all attributes were equal
+  const int strict = (type & 0x1);
+  return (dir == 0 ? (strict == 0 ? -1 : +1) : (strict == 0 ? +1 : -1));
 }
diff --git a/ndb/src/kernel/blocks/dbtux/DbtuxScan.cpp b/ndb/src/kernel/blocks/dbtux/DbtuxScan.cpp
index 7414691ab78..cf39185e403 100644
--- a/ndb/src/kernel/blocks/dbtux/DbtuxScan.cpp
+++ b/ndb/src/kernel/blocks/dbtux/DbtuxScan.cpp
@@ -108,15 +108,23 @@ Dbtux::execACC_SCANREQ(Signal* signal)
 /*
  * Receive bounds for scan in single direct call.  The bounds can arrive
  * in any order.  Attribute ids are those of index table.
+ *
+ * Replace EQ by equivalent LE + GE.  Check for conflicting bounds.
+ * Check that sets of lower and upper bounds are on initial sequences of
+ * keys and that all but possibly last bound is non-strict.
+ *
+ * Finally save the sets of lower and upper bounds (i.e. start key and
+ * end key).  Full bound type (< 4) is included but only the strict bit
+ * is used since lower and upper have now been separated.
  */
 void
 Dbtux::execTUX_BOUND_INFO(Signal* signal)
 {
   jamEntry();
   struct BoundInfo {
+    int type;
     unsigned offset;
     unsigned size;
-    int type;
   };
   TuxBoundInfo* const sig = (TuxBoundInfo*)signal->getDataPtrSend();
   const TuxBoundInfo reqCopy = *(const TuxBoundInfo*)sig;
@@ -124,18 +132,11 @@ Dbtux::execTUX_BOUND_INFO(Signal* signal)
   // get records
   ScanOp& scan = *c_scanOpPool.getPtr(req->tuxScanPtrI);
   Index& index = *c_indexPool.getPtr(scan.m_indexId);
-  // collect bound info for each index attribute
-  BoundInfo boundInfo[MaxIndexAttributes][2];
+  // collect lower and upper bounds
+  BoundInfo boundInfo[2][MaxIndexAttributes];
   // largest attrId seen plus one
-  Uint32 maxAttrId = 0;
-  // skip 5 words
+  Uint32 maxAttrId[2] = { 0, 0 };
   unsigned offset = 0;
-  if (req->boundAiLength < offset) {
-    jam();
-    scan.m_state = ScanOp::Invalid;
-    sig->errorCode = TuxBoundInfo::InvalidAttrInfo;
-    return;
-  }
   const Uint32* const data = (Uint32*)sig + TuxBoundInfo::SignalLength;
   // walk through entries
   while (offset + 2 <= req->boundAiLength) {
@@ -156,32 +157,35 @@ Dbtux::execTUX_BOUND_INFO(Signal* signal)
       sig->errorCode = TuxBoundInfo::InvalidAttrInfo;
       return;
     }
-    while (maxAttrId <= attrId) {
-      BoundInfo* b = boundInfo[maxAttrId++];
-      b[0].type = b[1].type = -1;
-    }
-    BoundInfo* b = boundInfo[attrId];
-    if (type == 0 || type == 1  || type == 4) {
-      if (b[0].type != -1) {
-        jam();
-        scan.m_state = ScanOp::Invalid;
-        sig->errorCode = TuxBoundInfo::InvalidBounds;
-        return;
+    for (unsigned j = 0; j <= 1; j++) {
+      // check if lower/upper bit matches
+      const unsigned luBit = (j << 1);
+      if ((type & 0x2) != luBit && type != 4)
+        continue;
+      // EQ -> LE, GE
+      const unsigned type2 = (type & 0x1) | luBit;
+      // fill in any gap
+      while (maxAttrId[j] <= attrId) {
+        BoundInfo& b = boundInfo[j][maxAttrId[j]++];
+        b.type = -1;
       }
-      b[0].offset = offset;
-      b[0].size = 2 + dataSize;
-      b[0].type = type;
-    }
-    if (type == 2 || type == 3 || type == 4) {
-      if (b[1].type != -1) {
-        jam();
-        scan.m_state = ScanOp::Invalid;
-        sig->errorCode = TuxBoundInfo::InvalidBounds;
-        return;
+      BoundInfo& b = boundInfo[j][attrId];
+      if (b.type != -1) {
+        // compare with previous bound
+        if (b.type != type2 ||
+            b.size != 2 + dataSize ||
+            memcmp(&data[b.offset + 2], &data[offset + 2], dataSize << 2) != 0) {
+          jam();
+          scan.m_state = ScanOp::Invalid;
+          sig->errorCode = TuxBoundInfo::InvalidBounds;
+          return;
+        }
+      } else {
+        // enter new bound
+        b.type = type2;
+        b.offset = offset;
+        b.size = 2 + dataSize;
       }
-      b[1].offset = offset;
-      b[1].size = 2 + dataSize;
-      b[1].type = type;
     }
     // jump to next
     offset += 2 + dataSize;
@@ -192,34 +196,27 @@ Dbtux::execTUX_BOUND_INFO(Signal* signal)
     sig->errorCode = TuxBoundInfo::InvalidAttrInfo;
     return;
   }
-  // save the bounds in index attribute id order
-  scan.m_boundCnt[0] = 0;
-  scan.m_boundCnt[1] = 0;
-  for (unsigned i = 0; i < maxAttrId; i++) {
-    jam();
-    const BoundInfo* b = boundInfo[i];
-    // current limitation - check all but last is equality
-    if (i + 1 < maxAttrId) {
-      if (b[0].type != 4 || b[1].type != 4) {
+  for (unsigned j = 0; j <= 1; j++) {
+    // save lower/upper bound in index attribute id order
+    for (unsigned i = 0; i < maxAttrId[j]; i++) {
+      jam();
+      const BoundInfo& b = boundInfo[j][i];
+      // check for gap or strict bound before last
+      if (b.type == -1 || (i + 1 < maxAttrId[j] && (b.type & 0x1))) {
         jam();
         scan.m_state = ScanOp::Invalid;
         sig->errorCode = TuxBoundInfo::InvalidBounds;
         return;
       }
-    }
-    for (unsigned j = 0; j <= 1; j++) {
-      if (b[j].type != -1) {
+      bool ok = scan.m_bound[j]->append(&data[b.offset], b.size);
+      if (! ok) {
         jam();
-        bool ok = scan.m_bound[j]->append(&data[b[j].offset], b[j].size);
-        if (! ok) {
-          jam();
-          scan.m_state = ScanOp::Invalid;
-          sig->errorCode = TuxBoundInfo::OutOfBuffers;
-          return;
-        }
-        scan.m_boundCnt[j]++;
+        scan.m_state = ScanOp::Invalid;
+        sig->errorCode = TuxBoundInfo::OutOfBuffers;
+        return;
       }
     }
+    scan.m_boundCnt[j] = maxAttrId[j];
   }
   // no error
   sig->errorCode = 0;
diff --git a/ndb/test/ndbapi/testOIBasic.cpp b/ndb/test/ndbapi/testOIBasic.cpp
index f9eb3514926..10994fd7847 100644
--- a/ndb/test/ndbapi/testOIBasic.cpp
+++ b/ndb/test/ndbapi/testOIBasic.cpp
@@ -1965,9 +1965,21 @@ BSet::calcpk(Par par, unsigned i)
 int
 BSet::setbnd(Par par) const
 {
-  for (unsigned j = 0; j < m_bvals; j++) {
-    const BVal& bval = *m_bval[j];
-    CHK(bval.setbnd(par) == 0);
+  if (m_bvals != 0) {
+    unsigned p1 = urandom(m_bvals);
+    unsigned p2 = 10009;        // prime
+    // random order
+    for (unsigned j = 0; j < m_bvals; j++) {
+      unsigned k = p1 + p2 * j;
+      const BVal& bval = *m_bval[k % m_bvals];
+      CHK(bval.setbnd(par) == 0);
+    }
+    // duplicate
+    if (urandom(5) == 0) {
+      unsigned k = urandom(m_bvals);
+      const BVal& bval = *m_bval[k];
+      CHK(bval.setbnd(par) == 0);
+    }
   }
   return 0;
 }