List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:May 5 2011 11:06am
Subject:bzr push into mysql-5.1-telco-7.0 branch (ole.john.aske:4357 to 4358)
View as plain text  
 4358 Ole John Aske	2011-05-05
      Refactoring / cleanup of SPJ API after http://lists.mysql.com/commits/136639
      (Which allows pushed scan childs to be 'bushy')
      
      The NdbQueryBuilder used to keep track of whether a pushed operation 
      already had a scan descendant - Mainly in order to be able to throw
      an error of a bushy scan join was constructed.
      
      The only usage of ::hasScanDescendant() besides this was within
      NdbQueryImpl::sendFetchMore(). It used it for calculating
      which ResultStreams to ::reset() as they would receive new rows.
      It turned out that this code was overly complicated, and could be 
      simplified - which removed the need for the 'scan descendant' knowledge.
      
      ::markScanAncestors(), ::hasScanDescendant() has then become 
      obsolete and is therefore removed by this patch.

    modified:
      storage/ndb/src/ndbapi/NdbQueryBuilder.cpp
      storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp
      storage/ndb/src/ndbapi/NdbQueryOperation.cpp
 4357 Ole John Aske	2011-05-05
      Small refactoring of NdbQueryBuilder's construction of pushed operations (SPJ API)
      Partly a followup to: mysql.com/commits/135407
      
      To simplify error handling code when building NdbQueryOperationDef's, the allocated
      operation is added to NdbQueryBuilders'Vector<NdbQueryOperationDefImpl*> m_operations'
      earlier. In case of a failure during query build, the operation will then be 
      released together with all other resources allocated so far through this NdbQueryBuilder.
      
      This removed the need for manually delete the allocated operation in case of failures.

    modified:
      storage/ndb/src/ndbapi/NdbQueryBuilder.cpp
      storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp
=== modified file 'storage/ndb/src/ndbapi/NdbQueryBuilder.cpp'
--- a/storage/ndb/src/ndbapi/NdbQueryBuilder.cpp	2011-05-05 07:43:12 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryBuilder.cpp	2011-05-05 11:06:08 +0000
@@ -964,9 +964,6 @@ NdbQueryBuilder::scanTable(const NdbDict
   returnErrIf(m_impl.takeOwnership(op)!=0, Err_MemoryAlloc);
   returnErrIf(error!=0, error); // C'tor returned error, bailout
 
-  error = op->markScanAncestors();
-  returnErrIf(error!=0, error);
-
   return &op->m_interface;
 }
 
@@ -1034,9 +1031,6 @@ NdbQueryBuilder::scanIndex(const NdbDict
     returnErrIf(error!=0, error);
   }
 
-  error = op->markScanAncestors();
-  returnErrIf(error!=0, error);
-
   return &op->m_interface;
 }
 
@@ -1837,7 +1831,6 @@ NdbQueryOperationDefImpl::NdbQueryOperat
                                      int& error)
   :m_isPrepared(false), 
    m_diskInChildProjection(false), 
-   m_hasScanDescendant(false),
    m_table(table), 
    m_ident(ident), 
    m_ix(ix), m_id(ix),
@@ -1990,25 +1983,6 @@ int NdbQueryOperationDefImpl::addParamRe
   return 0;
 }
 
-
-int NdbQueryOperationDefImpl::markScanAncestors()
-{
-  // Verify that parent links have been established.
-  assert(m_ix == 0 || m_parent != NULL);
-  assert(isScanOperation());
-  NdbQueryOperationDefImpl* operation = getParentOperation();
-  while (operation != NULL)
-  {
-    operation->m_hasScanDescendant = true;
-    if (operation->isScanOperation())
-    {
-      break;
-    }
-    operation = operation->getParentOperation();
-  }
-  return 0;
-}
-
 /** This class is used for serializing sequences of 16 bit integers,
  * where the first 16 bit integer specifies the length of the sequence.
  */

=== modified file 'storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp'
--- a/storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp	2011-05-05 07:43:12 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp	2011-05-05 11:06:08 +0000
@@ -388,15 +388,6 @@ public:
   // Return 'true' is query type is a multi-row scan
   virtual bool isScanOperation() const = 0;
 
-  /** Return true if this operation or any of its descendants is a scan.*/
-  bool hasScanDescendant() const
-  { return m_hasScanDescendant; }
-
-  /** Mark lookup ancestors of this operation as having a scan decendant.
-   * @return Possible error code.
-   */
-  int markScanAncestors();
-
   virtual const NdbQueryOperationDef& getInterface() const = 0; 
 
   /** Make a serialized representation of this operation, corresponding to
@@ -467,9 +458,6 @@ protected:
    */
   bool m_diskInChildProjection;
 
-  /** True if this operation or any of its descendants is a scan.*/
-  bool m_hasScanDescendant;
-
 private:
   bool isChildOf(const NdbQueryOperationDefImpl* parentOp) const;
 

=== modified file 'storage/ndb/src/ndbapi/NdbQueryOperation.cpp'
--- a/storage/ndb/src/ndbapi/NdbQueryOperation.cpp	2011-04-27 10:48:16 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryOperation.cpp	2011-05-05 11:06:08 +0000
@@ -47,7 +47,7 @@
  */
 #define UNUSED(x) ((void)(x))
 
-//#define TEST_SCANREQ
+//#define TEST_NEXTREQ
 
 /* Various error codes that are not specific to NdbQuery. */
 static const int Err_TupleNotFound = 626;
@@ -333,32 +333,35 @@ public:
   { return m_iterState == Iter_finished; }
 
   /** 
-   * This method is only used for result streams of scan operations. It is
+   * This method is
    * used for marking a stream as holding the last batch of a sub scan. 
    * This means that it is the last batch of the scan that was instantiated 
    * from the current batch of its parent operation.
    */
-  void setSubScanComplete(bool complete)
+  void setSubScanCompletion(bool complete)
   { 
-    assert(m_operation.getQueryOperationDef().isScanOperation());
+    // Lookups should always be 'complete'
+    assert(complete || m_operation.getQueryOperationDef().isScanOperation());
     m_subScanComplete = complete; 
   }
 
   /** 
-   * This method is only relevant for result streams of scan operations. It 
+   * This method 
    * returns true if this result stream holds the last batch of a sub scan
    * This means that it is the last batch of the scan that was instantiated 
    * from the current batch of its parent operation.
    */
   bool isSubScanComplete() const
   { 
-    assert(m_operation.getQueryOperationDef().isScanOperation());
+    // Lookups should always be 'complete'
+    assert(m_subScanComplete || m_operation.getQueryOperationDef().isScanOperation());
     return m_subScanComplete; 
   }
 
   /** Variant of isSubScanComplete() above which checks that this resultstream
    * and all its descendants have consumed all batches of rows instantiated 
-   * from their parent operation(s). */
+   * from their parent operation(s).
+   */
   bool isAllSubScansComplete() const;
 
   /** For debugging.*/
@@ -530,7 +533,7 @@ NdbResultStream::NdbResultStream(NdbQuer
   m_operation(operation),
   m_iterState(Iter_notStarted),
   m_currentRow(tupleNotFound),
-  m_subScanComplete(false),
+  m_subScanComplete(true),
   m_tupleSet(NULL)
 {};
 
@@ -578,8 +581,10 @@ NdbResultStream::reset()
 
   clearTupleSet();
   m_receiver.prepareSend();
-  /* If this stream will get new rows in the next batch, then so will
-   * all of its descendants.*/
+  /**
+   * If this stream will get new rows in the next batch, then so will
+   * all of its descendants.
+   */
   for (Uint32 childNo = 0; childNo < m_operation.getNoOfChildOperations();
        childNo++)
   {
@@ -605,8 +610,10 @@ NdbResultStream::clearTupleSet()
 bool
 NdbResultStream::isAllSubScansComplete() const
 { 
-  if (m_operation.getQueryOperationDef().isScanOperation() && 
-      !m_subScanComplete)
+  // Lookups should always be 'complete'
+  assert(m_subScanComplete || m_operation.getQueryOperationDef().isScanOperation());
+
+  if (!m_subScanComplete)
     return false;
 
   for (Uint32 childNo = 0; childNo < m_operation.getNoOfChildOperations(); 
@@ -2814,31 +2821,21 @@ NdbQueryImpl::sendFetchMore(NdbRootFragm
   assert(!emptyFrag.finalBatchReceived());
   assert(m_queryDef.isScanQuery());
 
+  const Uint32 fragNo = emptyFrag.getFragNo();
   emptyFrag.reset();
 
   for (unsigned opNo=0; opNo<m_countOperations; opNo++) 
   {
-    const NdbQueryOperationImpl& op = getQueryOperation(opNo);
-    // Check if this is a leaf scan.
-    if (!op.getQueryOperationDef().hasScanDescendant() &&
-        op.getQueryOperationDef().isScanOperation())
-    {
-      // Find first scan ancestor that is not finished.
-      const NdbQueryOperationImpl* ancestor = &op;
-      while (ancestor != NULL && 
-             (!ancestor->getQueryOperationDef().isScanOperation() ||
-              ancestor->getResultStream(emptyFrag.getFragNo())
-              .isSubScanComplete())
-              )
-      {
-        ancestor = ancestor->getParentOperation();
-      }
-      if (ancestor!=NULL)
-      {
-        /* Reset ancestor and all its descendants, since all these
-         * streams will get a new set of rows in the next batch. */ 
-        ancestor->getResultStream(emptyFrag.getFragNo()).reset();
-      }
+    NdbResultStream& resultStream = 
+       getQueryOperation(opNo).getResultStream(fragNo);
+
+    if (!resultStream.isSubScanComplete())
+    {
+      /**
+       * Reset resultstream and all its descendants, since all these
+       * streams will get a new set of rows in the next batch.
+       */ 
+      resultStream.reset();
     }
   }
 
@@ -3946,7 +3943,7 @@ NdbQueryOperationImpl
   if (myClosestScan != NULL)
   {
 
-#ifdef TEST_SCANREQ
+#ifdef TEST_NEXTREQ
     // To force usage of SCAN_NEXTREQ even for small scans resultsets
     if (this == &getRoot())
     {
@@ -4703,21 +4700,18 @@ NdbQueryOperationImpl::execSCAN_TABCONF(
   for (Uint32 opNo = 0; opNo < queryDef.getNoOfOperations(); opNo++)
   {
     const NdbQueryOperationImpl& op = m_queryImpl.getQueryOperation(opNo);
-    /* Find the node number seen by the SPJ block. Since a unique index
+    /**
+     * Find the node number seen by the SPJ block. Since a unique index
      * operation will have two distincts nodes in the tree used by the
-     * SPJ block, this number may be different from 'opNo'.*/
+     * SPJ block, this number may be different from 'opNo'.
+     */
     const Uint32 internalOpNo = op.getQueryOperationDef().getQueryOperationId();
     assert(internalOpNo >= opNo);
-    const bool maskSet = ((nodeMask >> internalOpNo) & 1) == 1;
+    const bool complete = ((nodeMask >> internalOpNo) & 1) == 0;
 
-    if (op.getQueryOperationDef().isScanOperation())
-    {
-      rootFrag->getResultStream(opNo).setSubScanComplete(!maskSet);
-    }
-    else
-    {
-      assert(!maskSet);
-    }
+    // Lookups should always be 'complete'
+    assert(complete ||  op.getQueryOperationDef().isScanOperation());
+    rootFrag->getResultStream(opNo).setSubScanCompletion(complete);
   }
   // Check that nodeMask does not have more bits than we have operations. 
   assert(nodeMask >> 

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.1-telco-7.0 branch (ole.john.aske:4357 to 4358) Ole John Aske5 May