List:Commits« Previous MessageNext Message »
From:Ole John Aske Date:May 5 2011 7:43am
Subject:bzr commit into mysql-5.1-telco-7.0 branch (ole.john.aske:4357)
View as plain text  
#At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1-telco-7.0/ based on revid:jonas@stripped

 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-04 11:45:33 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryBuilder.cpp	2011-05-05 07:43:12 +0000
@@ -857,13 +857,9 @@ NdbQueryBuilder::readTuple(const NdbDict
                                        ident,
                                        m_impl.m_operations.size(),
                                        error);
-  returnErrIf(op==0, Err_MemoryAlloc);
-  if (unlikely(error != 0))
-  {
-    m_impl.setErrorCode(error);
-    delete op;
-    return NULL;
-  }
+
+  returnErrIf(m_impl.takeOwnership(op)!=0, Err_MemoryAlloc);
+  returnErrIf(error!=0, error); // C'tor returned error, bailout
 
   Uint32 keyindex = 0;
   for (i=0; i<colcount; ++i)
@@ -873,11 +869,7 @@ NdbQueryBuilder::readTuple(const NdbDict
     {
       assert (keyindex==col->m_keyInfoPos);
       int error = op->m_keys[col->m_keyInfoPos]->bindOperand(*col,*op);
-      if (unlikely(error))
-      { m_impl.setErrorCode(error);
-        delete op;
-        return NULL;
-      }
+      returnErrIf(error!=0, error);
 
       keyindex++;
       if (keyindex >= static_cast<Uint32>(keyfields))
@@ -885,17 +877,7 @@ NdbQueryBuilder::readTuple(const NdbDict
     }
   }
   
-  if (likely(m_impl.m_operations.push_back(op) == 0))
-  {
-    return &op->m_interface;
-  }
-  else
-  {
-    assert(errno == ENOMEM);
-    delete op;
-    m_impl.setErrorCode(Err_MemoryAlloc);
-    return NULL;
-  }
+  return &op->m_interface;
 }
 
 
@@ -944,13 +926,9 @@ NdbQueryBuilder::readTuple(const NdbDict
                                        ident,
                                        m_impl.m_operations.size(),
                                        error);
-  returnErrIf(op==0, Err_MemoryAlloc);
-  if (unlikely(error != 0))
-  {
-    m_impl.setErrorCode(error);
-    delete op;
-    return NULL;
-  }
+
+  returnErrIf(m_impl.takeOwnership(op)!=0, Err_MemoryAlloc);
+  returnErrIf(error!=0, error); // C'tor returned error, bailout
 
   // Bind to Column and check type compatibility
   for (i=0; i<inxfields; ++i)
@@ -959,24 +937,10 @@ NdbQueryBuilder::readTuple(const NdbDict
     assert (col.getColumnNo() == i);
 
     error = keys[i]->getImpl().bindOperand(col,*op);
-    if (unlikely(error))
-    { m_impl.setErrorCode(error);
-      delete op;
-      return NULL;
-    }
+    returnErrIf(error!=0, error);
   }
 
-  if (likely(m_impl.m_operations.push_back(op) == 0))
-  {
-    return &op->m_interface;
-  }
-  else
-  {
-    assert(errno == ENOMEM);
-    delete op;
-    m_impl.setErrorCode(Err_MemoryAlloc);
-    return NULL;
-  }
+  return &op->m_interface;
 }
 
 
@@ -996,21 +960,9 @@ NdbQueryBuilder::scanTable(const NdbDict
                                           ident,
                                           m_impl.m_operations.size(),
                                           error);
-  returnErrIf(op==0, Err_MemoryAlloc);
-  if (unlikely(error != 0))
-  {
-    m_impl.setErrorCode(error);
-    delete op;
-    return NULL;
-  }
 
-  if (unlikely(m_impl.m_operations.push_back(op) != 0))
-  {
-    assert(errno == ENOMEM);
-    delete op;
-    m_impl.setErrorCode(Err_MemoryAlloc);
-    return NULL;
-  }
+  returnErrIf(m_impl.takeOwnership(op)!=0, Err_MemoryAlloc);
+  returnErrIf(error!=0, error); // C'tor returned error, bailout
 
   error = op->markScanAncestors();
   returnErrIf(error!=0, error);
@@ -1053,20 +1005,13 @@ NdbQueryBuilder::scanIndex(const NdbDict
                                           ident,
                                           m_impl.m_operations.size(),
                                           error);
-  returnErrIf(op==0, Err_MemoryAlloc);
-  if (unlikely(error != 0))
-  {
-    m_impl.setErrorCode(error);
-    delete op;
-    return NULL;
-  }
 
-  if (unlikely(op->m_bound.lowKeys  > indexImpl.getNoOfColumns() ||
-               op->m_bound.highKeys > indexImpl.getNoOfColumns()))
-  { m_impl.setErrorCode(QRY_TOO_MANY_KEY_VALUES);
-    delete op;
-    return NULL;
-  }
+  returnErrIf(m_impl.takeOwnership(op)!=0, Err_MemoryAlloc);
+  returnErrIf(error!=0, error); // C'tor returned error, bailout
+
+  returnErrIf(op->m_bound.lowKeys  > indexImpl.getNoOfColumns() ||
+              op->m_bound.highKeys > indexImpl.getNoOfColumns(),
+              QRY_TOO_MANY_KEY_VALUES);
 
   // Bind lowKeys, and if applicable, highKeys to the column being refered
   Uint32 i;
@@ -1078,11 +1023,7 @@ NdbQueryBuilder::scanIndex(const NdbDict
        ? op->m_bound.low[i]->bindOperand(col,*op) || op->m_bound.high[i]->bindOperand(col,*op)
        : op->m_bound.low[i]->bindOperand(col,*op);
 
-    if (unlikely(error))
-    { m_impl.setErrorCode(error);
-      delete op;
-      return NULL;
-    }
+    returnErrIf(error!=0, error);
   }
 
   // Bind any remaining highKeys past '#lowKeys'
@@ -1090,31 +1031,13 @@ NdbQueryBuilder::scanIndex(const NdbDict
   {
     const NdbColumnImpl& col = NdbColumnImpl::getImpl(*indexImpl.getColumn(i));
     error = op->m_bound.high[i]->bindOperand(col,*op);
-    if (unlikely(error))
-    { m_impl.setErrorCode(error);
-      delete op;
-      return NULL;
-    }
+    returnErrIf(error!=0, error);
   }
 
   error = op->markScanAncestors();
-  if (unlikely(error))
-  { m_impl.setErrorCode(error);
-    delete op;
-    return NULL;
-  }
+  returnErrIf(error!=0, error);
 
-  if (likely(m_impl.m_operations.push_back(op) == 0))
-  {
-    return &op->m_interface;
-  }
-  else
-  {
-    assert(errno == ENOMEM);
-    delete op;
-    m_impl.setErrorCode(Err_MemoryAlloc);
-    return NULL;
-  }
+  return &op->m_interface;
 }
 
 const NdbQueryDef*
@@ -1220,27 +1143,43 @@ NdbQueryBuilderImpl::prepare()
   return def;
 }
 
-
-NdbQueryOperand* 
-NdbQueryBuilderImpl::addOperand(NdbQueryOperandImpl* operand)
+inline int 
+NdbQueryBuilderImpl::takeOwnership(NdbQueryOperandImpl* operand)
 {
   if (unlikely(operand == NULL))
   {
-    setErrorCode(Err_MemoryAlloc);
-    return NULL;
+    return Err_MemoryAlloc;
   }
+  else if (unlikely(m_operands.push_back(operand) != 0))
+  {
+    assert(errno == ENOMEM);
+    delete operand;
+    return Err_MemoryAlloc;
+  }
+  return 0;
+}
 
-  if (likely(m_operands.push_back(operand) == 0))
+inline int 
+NdbQueryBuilderImpl::takeOwnership(NdbQueryOperationDefImpl* operation)
+{
+  if (unlikely(operation == NULL))
   {
-    return &operand->getInterface();
+    return Err_MemoryAlloc;
   }
-  else
+  else if (unlikely(m_operations.push_back(operation) != 0))
   {
     assert(errno == ENOMEM);
-    delete operand;
-    setErrorCode(Err_MemoryAlloc);
-    return NULL;
+    delete operation;
+    return Err_MemoryAlloc;
   }
+  return 0;
+}
+
+NdbQueryOperand* 
+NdbQueryBuilderImpl::addOperand(NdbQueryOperandImpl* operand)
+{
+  returnErrIf(takeOwnership(operand)!=0, Err_MemoryAlloc);
+  return &operand->getInterface();
 }
 
 ///////////////////////////////////

=== modified file 'storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp'
--- a/storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp	2011-05-04 11:45:33 +0000
+++ b/storage/ndb/src/ndbapi/NdbQueryBuilderImpl.hpp	2011-05-05 07:43:12 +0000
@@ -663,6 +663,17 @@ private:
    */
   NdbQueryOperand* addOperand(NdbQueryOperandImpl* operand);
 
+  /**
+   * Take ownership of specified object: From now on it is the
+   * responsibility of this NdbQueryBuilderImpl to manage the
+   * lifetime of the object. If takeOwnership() fails, the 
+   * specified object is deleted before it returns.
+   * @param[in] operand to take ownership for (may be NULL).
+   * @return 0 if ok, else there has been an 'Err_MemoryAlloc'
+   */
+  int takeOwnership(NdbQueryOperandImpl*);
+  int takeOwnership(NdbQueryOperationDefImpl*);
+
   bool contains(const NdbQueryOperationDefImpl*);
 
   NdbQueryBuilder m_interface;


Attachment: [text/bzr-bundle] bzr/ole.john.aske@oracle.com-20110505074312-sr143qdhqjyc26fp.bundle
Thread
bzr commit into mysql-5.1-telco-7.0 branch (ole.john.aske:4357) Ole John Aske5 May