fix to address review comments.

Project: http://git-wip-us.apache.org/repos/asf/trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafodion/commit/0a1bb2a1
Tree: http://git-wip-us.apache.org/repos/asf/trafodion/tree/0a1bb2a1
Diff: http://git-wip-us.apache.org/repos/asf/trafodion/diff/0a1bb2a1

Branch: refs/heads/master
Commit: 0a1bb2a1a038d60fdeeb5405ca0d30f4705307f7
Parents: 5c51fa3
Author: Prashant Vasudev <prashanth.vasu...@esgyn.com>
Authored: Mon Jul 30 22:19:45 2018 +0000
Committer: Prashant Vasudev <prashanth.vasu...@esgyn.com>
Committed: Mon Jul 30 22:19:45 2018 +0000

----------------------------------------------------------------------
 core/sql/executor/ExSequence.cpp | 99 ++++++++++++++++-------------------
 core/sql/executor/ExSequence.h   |  1 +
 core/sql/executor/cluster.cpp    | 87 ++++++++++++++++++++++++++++++
 core/sql/executor/cluster.h      |  8 +++
 4 files changed, 141 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafodion/blob/0a1bb2a1/core/sql/executor/ExSequence.cpp
----------------------------------------------------------------------
diff --git a/core/sql/executor/ExSequence.cpp b/core/sql/executor/ExSequence.cpp
index aa2d905..7ba414a 100644
--- a/core/sql/executor/ExSequence.cpp
+++ b/core/sql/executor/ExSequence.cpp
@@ -992,23 +992,24 @@ short ExSequenceTcb::work()
             break;
           }
           ex_assert(isUnboundedFollowing(),"");
- 
-         if ( ! cluster_->flush(&rc_) ) {  // flush the buffers
+
+          ComDiagsArea *myDiags = NULL;
+          if (!cluster_->flush(myDiags, heap_)) {  // flush the buffers
             // if no errors this code path is not visited
-           if ( rc_ ) 
+            if (myDiags) 
             { // some error
-              updateDiagsArea( rc_);
+              updateDiagsArea(myDiags);
               pstate->step_ = ExSeq_ERROR;
-             break;
-           }
-           // not all the buffers are completely flushed. An I/O is pending
+              break;
+            }
+            // not all the buffers are completely flushed. An I/O is pending
             // maybe we cane remove in the future
-           return WORK_OK; 
-         }
+            return WORK_OK; 
+          }
 
-         // At this point -- all the buffers were completely flushed
+          // At this point -- all the buffers were completely flushed
 
-         OLAPBuffersFlushed_ = TRUE;
+          OLAPBuffersFlushed_ = TRUE;
 
           if (getPartitionEnd())
           {
@@ -1041,24 +1042,24 @@ short ExSequenceTcb::work()
         // 2. ExSeq_ERROR - If an error occurs
          case ExSeq_OVERFLOW_READ:
         {
+          assert(firstOLAPBufferFromOF_ &&
+                  isUnboundedFollowing() );
 
-            assert(firstOLAPBufferFromOF_ &&
-                    isUnboundedFollowing() );
-
-           if ( ! cluster_->read(&rc_) ) {
-             if ( rc_ ) { // some error
-                updateDiagsArea( rc_);
-               pstate->step_ = ExSeq_ERROR;
-               break;
-             }
-             // not all the buffers are completely read. An I/O is pending
-             return WORK_OK;
-           }
+          ComDiagsArea *myDiags = NULL;
+          if (!cluster_->read(myDiags, heap_)) {
+            if (myDiags) { // some error
+              updateDiagsArea(myDiags);
+              pstate->step_ = ExSeq_ERROR;
+              break;
+            }
+            // not all the buffers are completely read. An I/O is pending
+            return WORK_OK;
+          }
 
             numberOfRowsReturnedBeforeReadOF_ = 0;
             pstate->step_ = ExSeq_WORKING_RETURN;
-       }
-       break;
+        }
+        break;
 
         // ExSeq_DONE
         //
@@ -1477,6 +1478,23 @@ void ExSequenceTcb::updateDiagsArea(ex_queue_entry * 
centry)
       }
     }
 }
+
+void ExSequenceTcb::updateDiagsArea(ComDiagsArea *da)
+{
+    if (da) 
+    {
+      if (workAtp_->getDiagsArea())
+      {     
+        workAtp_->getDiagsArea()->mergeAfter(*da);
+      }
+      else
+      {
+        workAtp_->setDiagsArea(da);
+        da->incrRefCount();
+      }
+    }
+}
+
 void ExSequenceTcb::updateDiagsArea(  ExeErrorCode rc_)
 {                   
     ComDiagsArea *da = workAtp_->getDiagsArea();
@@ -1487,35 +1505,8 @@ void ExSequenceTcb::updateDiagsArea(  ExeErrorCode rc_)
     }
     if (!da->contains((Lng32) -rc_))
     {
-      char msg[512];
-      if(rc_ == EXE_SORT_ERROR)
-      {
-        char errorMsg[100];
-        Lng32 scratchError = 0;
-        Lng32 scratchSysError = 0;
-        Lng32 scratchSysErrorDetail = 0;
-
-        if(clusterDb_)
-        {
-          clusterDb_->getScratchErrorDetail(scratchError,
-                                 scratchSysError,
-                                 scratchSysErrorDetail,
-                                 errorMsg);
-
-          str_sprintf(msg, "Sequence Scratch IO Error occurred. Scratch Error: 
%d, System Error: %d, System Error Detail: %d, Details: %s",
-              scratchError, scratchSysError, scratchSysErrorDetail, errorMsg);
-        }
-        else
-        {
-          str_sprintf(msg, "Sequence Scratch IO Error occurred." );
-        }
-      }
-      else
-      {
-        str_sprintf(msg, "Sequence Operator Error occurred."); 
-      }
-      
-      *da << DgString0(msg);
+      *da << DgSqlCode(-rc_);
+      *da << DgString0("Sequence Operator Error occurred.");
     }
 }
 //

http://git-wip-us.apache.org/repos/asf/trafodion/blob/0a1bb2a1/core/sql/executor/ExSequence.h
----------------------------------------------------------------------
diff --git a/core/sql/executor/ExSequence.h b/core/sql/executor/ExSequence.h
index d198f4a..daf324f 100644
--- a/core/sql/executor/ExSequence.h
+++ b/core/sql/executor/ExSequence.h
@@ -190,6 +190,7 @@ public:
 
   void updateDiagsArea(ex_queue_entry * centry);
   void updateDiagsArea( ExeErrorCode rc_);
+  void updateDiagsArea(ComDiagsArea *da);
 
   NABoolean getPartitionEnd() const
   {

http://git-wip-us.apache.org/repos/asf/trafodion/blob/0a1bb2a1/core/sql/executor/cluster.cpp
----------------------------------------------------------------------
diff --git a/core/sql/executor/cluster.cpp b/core/sql/executor/cluster.cpp
index f9ceb86..14b150b 100644
--- a/core/sql/executor/cluster.cpp
+++ b/core/sql/executor/cluster.cpp
@@ -1611,6 +1611,49 @@ NABoolean Cluster::initScratch(ExeErrorCode * rc)
   return FALSE;
 }
 
+//Wrapper function on flush() to populate diags area.
+//Returns: FALSE - either error, or need to call again
+//         TRUE -- flush is done
+NABoolean Cluster::flush(ComDiagsArea *&da, CollHeap *heap) {
+  ExeErrorCode rc = EXE_OK;
+  
+  //Flush returning FALSE means either Error or IO_NOT_COMPLETE.
+  //if rc != EXE_OK then it is error. 
+  if(!flush(&rc)) {
+    if(rc != EXE_OK) {
+      da = ComDiagsArea::allocate(heap);
+      *da << DgSqlCode(-rc);
+      
+      char msg[512];
+      if(rc == EXE_SORT_ERROR) {
+        char errorMsg[100];
+        Lng32 scratchError = 0;
+        Lng32 scratchSysError = 0;
+        Lng32 scratchSysErrorDetail = 0;
+
+        if(clusterDb_) {
+          clusterDb_->getScratchErrorDetail(scratchError,
+                                 scratchSysError,
+                                 scratchSysErrorDetail,
+                                 errorMsg);
+
+          str_sprintf(msg, "Scratch IO Error occurred. Scratch Error: %d, 
System Error: %d, System Error Detail: %d, Details: %s",
+              scratchError, scratchSysError, scratchSysErrorDetail, errorMsg);
+        }
+        else {
+          str_sprintf(msg, "Scratch IO Error occurred. clusterDb_ is NULL" );
+        }
+      } else {
+        str_sprintf(msg, "Cluster Flush Error occurred."); 
+      }
+      
+      *da << DgString0(msg);
+    }
+    return FALSE;
+  }
+  return TRUE;
+}
+
 // Flush the in-memory buffers of this cluster
 // Returns: FALSE - either error, or need to call again
 //          TRUE -- flush is done
@@ -2325,6 +2368,50 @@ NABoolean Cluster::read(ExeErrorCode * rc) {
   } // WHILE ( TRUE )    
 };
 
+//Wrapper function on read() to populate diags area.
+//Returns: FALSE - either error, or need to call again
+//       TRUE -- flush is done
+NABoolean Cluster::read(ComDiagsArea *&da, CollHeap *heap) {
+  ExeErrorCode rc = EXE_OK;
+
+  //read returning FALSE means either Error or IO_NOT_COMPLETE.
+  //if rc != EXE_OK then it is error. 
+  if(!read(&rc)) {
+    if(rc != EXE_OK) {
+       da = ComDiagsArea::allocate(heap);
+       *da << DgSqlCode(-rc);
+      
+      char msg[512];
+      if(rc == EXE_SORT_ERROR) {
+        char errorMsg[100];
+        Lng32 scratchError = 0;
+        Lng32 scratchSysError = 0;
+        Lng32 scratchSysErrorDetail = 0;
+  
+        if(clusterDb_) {
+          clusterDb_->getScratchErrorDetail(scratchError,
+                                 scratchSysError,
+                                 scratchSysErrorDetail,
+                                 errorMsg);
+  
+          str_sprintf(msg, "Cluster::read Scratch IO Error occurred. Scratch 
Error: %d, System Error: %d, System Error Detail: %d, Details: %s",
+              scratchError, scratchSysError, scratchSysErrorDetail, errorMsg);
+        }
+        else {
+          str_sprintf(msg, "Cluster::read Scratch IO Error occurred. 
clusterDb_ is NULL" );
+        }
+      } else {
+        str_sprintf(msg, "Cluster::read Error occurred."); 
+      }
+      
+      *da << DgString0(msg);
+    }
+    return FALSE;
+  }
+  return TRUE;
+}
+
+
 /////////////////////////////////////////////////////////////////////////////
 
 // Return: FALSE - No split needed, or split was done successfully

http://git-wip-us.apache.org/repos/asf/trafodion/blob/0a1bb2a1/core/sql/executor/cluster.h
----------------------------------------------------------------------
diff --git a/core/sql/executor/cluster.h b/core/sql/executor/cluster.h
index c987002..4234db1 100644
--- a/core/sql/executor/cluster.h
+++ b/core/sql/executor/cluster.h
@@ -933,6 +933,10 @@ public:
   // flush the Cluster. If the flushing is not complete yet (I/Os pending)
   // flush() returns FALSE. Otherwise TRUE.
   NABoolean flush(ExeErrorCode * rc);
+  
+  // encapsulate error handling within flush. it is wrapper 
+  // around flush(ExeErrorCode * rc) call. Used by ExSequence.
+  NABoolean flush(ComDiagsArea *&da, CollHeap *heap);
 
   // spill a cluster. Spill is the same as flush. The only difference is,
   // that a spilled cluster still has a hash table (this is only used for
@@ -945,6 +949,10 @@ public:
   // Cluster read() reads as many buffers as possible. If it is an outer
   // Cluster, read() reads just one bufffer.
   NABoolean read(ExeErrorCode * rc);
+  
+  // encapsulate error handling within read. it is wrapper 
+  // around read(ExeErrorCode * rc) call. Used by ExSequence.
+  NABoolean read(ComDiagsArea *&da, CollHeap *heap);
  
   // create a hash table for this Cluster and chain all rows of the Cluster
   // into this hash table.

Reply via email to