From: Ole John Aske
Date: August 22 2011 12:57pm
Subject: bzr push into mysql-5.1-telco-7.0 branch (ole.john.aske:4474 to 4475)
List-Archive: http://lists.mysql.com/commits/140771
Message-Id: <20110822125742.D06C5218@fimafeng09.norway.sun.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

 4475 Ole John Aske	2011-08-22
      Fix for SPJ regression introduced by revno4456 to branch 'mysql-5.1-telco-7.0'
      
      My previous 'correlation.patch' stores a TupleCorrelation (Uint32) together
      with each received row directly in the NdbReceiver buffer.
      
      This causes problem when there are RecAttr's in the received data.
      NdbReceiver has an assumption that everything not being a NdbRecord
      are NdbRecAttr in this buffer. It therefore try to parse this TupleCorrelation
      as a NdbRecAttr which will fail.
      
      Instead of storing the TupleCorrelations together with the row, we now
      allocate a separate array of TupleCorrelations as part of each NdbResultSet
      where the TupleCorrelations are stored.

    modified:
      storage/ndb/src/ndbapi/NdbQueryOperation.cpp
 4474 Ole John Aske	2011-08-22
      Fixed SPJ regression:
      
      After push of 'revno: 4448' to branch 'mysql-5.1-telco-7.0', the ndbapi test 
      'testSpj' started failing.
      
      This is a fix for the first of these problems ('testSpj -n scanJoin' still fails)
      which reverts part of the changes in revno 4448.
      
      The problem fixed - or rather compensated for - is that retrieving ATTR values
      from the receive buffer has the underlying assumption that the NdbReceiver
      refer the next row (or end of the current).
      
      Therefor this fix increment the 'current' row by using ::get_row instead of ::peek_row
      and remove a verifySortOrder from a place where the current row has been advanced, and
      before the fragment has been reorganized(). 

    modified:
      storage/ndb/src/ndbapi/NdbQueryOperation.cpp
=== modified file 'storage/ndb/src/ndbapi/NdbQueryOperation.cpp'
--- a/storage/ndb/src/ndbapi/NdbQueryOperation.cpp	2011-08-22 12:20:15 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryOperation.cpp	2011-08-22 12:56:56 +0000
@@ -374,9 +374,6 @@ public:
   Uint32 getRowCount() const
   { return m_rowCount; }
 
-  char* getRow(Uint32 tupleNo) const
-  { return (m_buffer + (tupleNo*m_rowSize)); }
-
 private:
   /** No copying.*/
   NdbResultSet(const NdbResultSet&);
@@ -388,6 +385,9 @@ private:
   /** Used for checking if buffer overrun occurred. */
   Uint32* m_batchOverflowCheck;
 
+  /** Array of TupleCorrelations for all rows in m_buffer */
+  TupleCorrelation* m_correlations;
+
   Uint32 m_rowSize;
 
   /** The current #rows in 'm_buffer'.*/
@@ -669,6 +669,7 @@ void* NdbBulkAllocator::allocObjMem(Uint
 NdbResultSet::NdbResultSet() :
   m_buffer(NULL),
   m_batchOverflowCheck(NULL),
+  m_correlations(NULL),
   m_rowSize(0),
   m_rowCount(0)
 {}
@@ -688,6 +689,12 @@ NdbResultSet::init(NdbQueryImpl& query,
     m_batchOverflowCheck = 
       reinterpret_cast<Uint32*>(bufferAlloc.allocObjMem(sizeof(Uint32)));
     *m_batchOverflowCheck = 0xacbd1234;
+
+    if (query.getQueryDef().isScanQuery())
+    {
+      m_correlations = reinterpret_cast<TupleCorrelation*>
+                         (bufferAlloc.allocObjMem(maxRows*sizeof(TupleCorrelation)));
+    }
   }
 }
 
@@ -872,19 +879,17 @@ void
 NdbResultStream::execTRANSID_AI(const Uint32 *ptr, Uint32 len,
                                 TupleCorrelation correlation)
 {
-  m_receiver.execTRANSID_AI(ptr, len);
-  m_resultSets[m_recv].m_rowCount++;
-
+  NdbResultSet& receiveSet = m_resultSets[m_recv];
   if (isScanQuery())
   {
     /**
-     * Store TupleCorrelation as hidden value imm. after received row
-     * (NdbQueryOperationImpl::getRowSize() has reserved space for it)
+     * Store TupleCorrelation.
      */
-    Uint32* row_recv = reinterpret_cast<Uint32*>
-                         (m_receiver.m_record.m_row_recv);
-    row_recv[-1] = correlation.toUint32();
+    receiveSet.m_correlations[receiveSet.m_rowCount] = correlation;
   }
+
+  m_receiver.execTRANSID_AI(ptr, len);
+  receiveSet.m_rowCount++;
 } // NdbResultStream::execTRANSID_AI()
 
 /**
@@ -1010,8 +1015,8 @@ NdbResultStream::prepareResultSet(Uint32
 
 /**
  * Fill m_tupleSet[] with correlation data between parent 
- * and child tuples. The 'TupleCorrelation' is stored as
- * and extra Uint32 after each row in the NdbResultSet
+ * and child tuples. The 'TupleCorrelation' is stored
+ * in an array of TupleCorrelations in each ResultSet
  * by execTRANSID_AI().
  *
  * NOTE: In order to reduce work done when holding the 
@@ -1040,12 +1045,9 @@ NdbResultStream::buildResultCorrelations
     /* Rebuild correlation & hashmap from 'readResult' */
     for (Uint32 tupleNo=0; tupleNo<readResult.m_rowCount; tupleNo++)
     {
-      const Uint32* row = (Uint32*)readResult.getRow(tupleNo+1);
-      const TupleCorrelation correlation(row[-1]);
-
-      const Uint16 tupleId  = correlation.getTupleId();
+      const Uint16 tupleId  = readResult.m_correlations[tupleNo].getTupleId();
       const Uint16 parentId = (m_parent!=NULL) 
-                                ? correlation.getParentTupleId()
+                                ? readResult.m_correlations[tupleNo].getParentTupleId()
                                 : tupleNotFound;
 
       m_tupleSet[tupleNo].m_skip     = false;
@@ -2707,14 +2709,13 @@ NdbQueryImpl::prepareSend()
   for (Uint32 opNo = 0; opNo < getNoOfOperations(); opNo++)
   {
     const NdbQueryOperationImpl& op = getQueryOperation(opNo);
+    // Add space for m_correlations, m_buffer & m_batchOverflowCheck
+    totalBuffSize += (sizeof(TupleCorrelation) * op.getMaxBatchRows());
     totalBuffSize += (op.getRowSize() * op.getMaxBatchRows());
+    totalBuffSize += sizeof(Uint32); // Overflow check
   }
-  /**
-   * Add one word per ResultStream for buffer overrun check. We add a word
-   * rather than a byte to avoid possible alignment problems.
-   */
-  m_rowBufferAlloc.init(m_rootFragCount * 
-                       (totalBuffSize + (sizeof(Uint32) * getNoOfOperations())) );
+
+  m_rowBufferAlloc.init(m_rootFragCount * totalBuffSize);
   if (getQueryDef().isScanQuery())
   {
     Uint32 totalRows = 0;
@@ -4816,7 +4817,7 @@ NdbQueryOperationImpl::execTRANSID_AI(co
   if (getQueryDef().isScanQuery())
   {
     const CorrelationData correlData(ptr, len);
-    const Uint32 receiverId = CorrelationData(ptr, len).getRootReceiverId();
+    const Uint32 receiverId = correlData.getRootReceiverId();
     
     /** receiverId holds the Id of the receiver of the corresponding stream
      * of the root operation. We can thus find the correct root fragment 
@@ -5184,12 +5185,6 @@ Uint32 NdbQueryOperationImpl::getRowSize
   {
     m_rowSize = 
       NdbReceiver::ndbrecord_rowsize(m_ndbRecord, m_firstRecAttr, 0, false);
-
-    const bool withCorrelation = getRoot().getQueryDef().isScanQuery();
-    if (withCorrelation)
-    {
-      m_rowSize += TupleCorrelation::wordCount*sizeof(Uint32);
-    }
   }
   return m_rowSize;
 }

No bundle (reason: useless for push emails).