You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafodion.apache.org by db...@apache.org on 2017/07/05 21:48:42 UTC

[1/3] incubator-trafodion git commit: [TRAFODION-2661] Improve MDAM costing for RangeSpecs

Repository: incubator-trafodion
Updated Branches:
  refs/heads/master b0392f452 -> 05d14d47f


[TRAFODION-2661] Improve MDAM costing for RangeSpecs


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

Branch: refs/heads/master
Commit: 78fab67cddae6496f75c5b60e8947d95126b2332
Parents: 498b89f
Author: Dave Birdsall <db...@apache.org>
Authored: Mon Jun 26 21:38:53 2017 +0000
Committer: Dave Birdsall <db...@apache.org>
Committed: Mon Jun 26 21:38:53 2017 +0000

----------------------------------------------------------------------
 core/sql/optimizer/ScanOptimizer.cpp | 52 ++-----------------------------
 1 file changed, 2 insertions(+), 50 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/78fab67c/core/sql/optimizer/ScanOptimizer.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/ScanOptimizer.cpp b/core/sql/optimizer/ScanOptimizer.cpp
index 8edc8b3..6528913 100644
--- a/core/sql/optimizer/ScanOptimizer.cpp
+++ b/core/sql/optimizer/ScanOptimizer.cpp
@@ -9788,45 +9788,8 @@ void MDAMOptimalDisjunctPrefixWA::updateMinPrefix()
       if (CmpCommon::getDefault(SIMPLE_COST_MODEL) == DF_ON)
       {
 	DCMPASSERT(scmCost != NULL);
-        // When RangeSpec feture transforms old mdam disjuncts in to "Range in"
-        // types, we apply all disjuncts at one shot. The old way of taking Min(prefixCost) does
-        // not make sense and leads to gross underestimation of MDAM plan.
-        // For example, consider a predicate like this "A in (10, 20) and B in (100, 200)"
-        // without RangeSpec: we have 4 disjuncts Like this:
-        //     dis0 : "A = 10 and B = 100", cost C1 = MIN (CostA, CostB) for values (10,100)
-        //     dis1 : "A = 10 and B = 200", cost C2 = MIN (CostA, CostB) for values (10,200)
-        //     dis2 : "A = 20 and B = 100", cost C3 = MIN (CostA, CostB) for values (20,100)
-        //     dis3 : "A = 20 and B = 200", cost C4 = MIN (CostA, CostB) for values (20,200)
-        //     Total Mdam Cost = C1 + C2 + C3 + C4
-        // with RangeSpec: we have 1 disjunct like this:
-        //     dis0 : "A = 10 or A = 20" and "B = 100 or B = 200"
-        // we should not take MIN (CostA, CostB) for the second case, it should be the cost to apply 
-        // last key predicate. 
-        //     Total Mdam Cost = CostB
-        //
-        //  This is a heuristics in that we unconditionally include the last key column 
-        //  with IN list (OR preds) predicate without going through the cost comparison 
-        //  step.
-        //
-        // Updated comments: The commentary above is incorrect but I don't know quite
-        // what to do with it yet. MDAM at run time is a recursive algorithm. In the 
-        // example above, it will materialize values in the A column, and for each one,
-        // do a subset access on the second column. So the cost is a sum of the
-        // materialization cost on the first column and the subset access on the second.
-        // If there is a third key column C with no predicates on it, it would be 
-        // inefficient to go MDAM to the last column position; rather it would be better
-        // to use B as the stop column. That is, do subsets on each distinct value of (A,B),
-        // rather than do subsets on each distinct (A,B,C). The larger the UEC of C, the
-        // more gross the inefficiency. Unfortunately, the code below will cause us to 
-        // go MDAM to column C. In reference to the comments above, we need to devise
-        // a better way to estimate cost in the presence of RangeSpecs.
-        if ( (CmpCommon::getDefault(RANGESPEC_TRANSFORMATION) == DF_ON ) &&
-              optimizer_.getDisjuncts().containsOrPredsInRanges() &&
-              prefixColumnPosition_ == (lastColumnPosition_ - 1) 
-           ) {
-          newMinimumFound = TRUE;
-        } else
-          newMinimumFound = (pMinCost_ == NULL) ? TRUE :
+   
+        newMinimumFound = (pMinCost_ == NULL) ? TRUE :
             (scmCost->scmCompareCosts(*pMinCost_) == LESS);
       }
       else
@@ -9868,17 +9831,6 @@ void MDAMOptimalDisjunctPrefixWA::updateMinPrefix()
       optSeqKBRead_ = prefixKBRead_;
       optKeyPreds_.insert(prefixKeyPreds_); // is a copy more efficient?
 
-      // Note: Formerly there was code here that would set stopColumn_
-      // to the last column position if the CQD RANGESPEC_TRANSFORMATION
-      // was on. This is incorrect; it would cause us to use MDAM to 
-      // traverse through all columns always, even though it may be
-      // grossly inefficient to do so. (See commentary earlier in this
-      // method.) As it stands now, so long as there are no RangeSpec
-      // key predicates, this code will correctly pick the stop column.
-      // If there are RangeSpec predicates, code earlier in this method
-      // may cause us to only consider MDAM traversing on all columns.
-      // We can improve this later by improving how RangeSpec predicates
-      // are costed for MDAM.
       stopColumn_ = prefixColumnPosition_;
 
       prevColChosen_ = TRUE;


[2/3] incubator-trafodion git commit: Rework for [TRAFODION-2661]

Posted by db...@apache.org.
Rework for [TRAFODION-2661]

This rework fixes a bug in the pre-code-gen time code. The bug was
that in the presence of partitioning key predicates, logic that
communicates stop column information to the generator was being
bypassed. This would result in traversal to the last key predicate
at run time, with poor performance if the stop column was prior to
the last key predicate.

Also included in this rework are some tweaks to the run-time debug
code for MDAM (the code had become non-functional from not being used
for a long time), and a fix to a run-time memory leak bug that shows
up as an assert when that debug code is present.


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

Branch: refs/heads/master
Commit: e3c7edce74fc260e0ebc9f2d7cb6ad8c2d02ccee
Parents: 78fab67
Author: Dave Birdsall <db...@apache.org>
Authored: Mon Jul 3 21:21:13 2017 +0000
Committer: Dave Birdsall <db...@apache.org>
Committed: Mon Jul 3 21:21:13 2017 +0000

----------------------------------------------------------------------
 core/sql/common/str.cpp           | 10 ++++++----
 core/sql/common/str.h             |  6 ++++--
 core/sql/executor/MdamPoint.cpp   |  2 +-
 core/sql/executor/MdamPoint.h     |  3 ---
 core/sql/executor/ex_mdam.cpp     |  2 ++
 core/sql/nskgmake/Makerules.linux |  3 +++
 core/sql/optimizer/mdam.cpp       | 22 +++++++++++++++-------
 7 files changed, 31 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/common/str.cpp
----------------------------------------------------------------------
diff --git a/core/sql/common/str.cpp b/core/sql/common/str.cpp
index 4b30104..8e1195b 100644
--- a/core/sql/common/str.cpp
+++ b/core/sql/common/str.cpp
@@ -1620,9 +1620,11 @@ Int32 str_convertToHexAscii(const char * src,               // in
 //    char * dataPointer = getDataPointer();
 //    Lng32 len = tupp_.getAllocatedSize();
 //
-//    printBrief(dataPointer, len) 
+//    printBrief(dataPointer, len) if you want an end of line
+//
+//    printBrief(dataPointer, len, FALSE) if you don't 
 //    
-void printBrief(char* dataPointer, Lng32 len) 
+void printBrief(char* dataPointer, Lng32 len, NABoolean endLine) 
 {
    // We don't know what the data type is, but we do know how
    // long the field is. So we will guess the data type.
@@ -1771,7 +1773,7 @@ void printBrief(char* dataPointer, Lng32 len)
        }
      }    
    cout << local;
-   // cout << *(Lng32 *)dataPointer;
-   // End test change
+   if (endLine)
+     cout << endl;
  }
 

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/common/str.h
----------------------------------------------------------------------
diff --git a/core/sql/common/str.h b/core/sql/common/str.h
index 12e039c..eb60ae0 100644
--- a/core/sql/common/str.h
+++ b/core/sql/common/str.h
@@ -461,8 +461,10 @@ Int32 str_convertToHexAscii(const char * src,                   // in
 //    char * dataPointer = getDataPointer();
 //    Lng32 len = tupp_.getAllocatedSize();
 //
-//    printBrief(dataPointer, len) 
+//    printBrief(dataPointer, len) if you want an end of line
+//
+//    printBrief(dataPointer, len, FALSE) if you don't 
 //    
 NA_EIDPROC
-void printBrief(char* dataPointer, Lng32 keyLen); 
+void printBrief(char* dataPointer, Lng32 keyLen, NABoolean endLine = TRUE); 
 #endif // STR_H

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/executor/MdamPoint.cpp
----------------------------------------------------------------------
diff --git a/core/sql/executor/MdamPoint.cpp b/core/sql/executor/MdamPoint.cpp
index 29a4867..1ecc00c 100644
--- a/core/sql/executor/MdamPoint.cpp
+++ b/core/sql/executor/MdamPoint.cpp
@@ -138,6 +138,6 @@ void MdamPoint::printBrief() const
   char * dataPointer = getDataPointer();
   Lng32 keyLen = tupp_.getAllocatedSize();
 
-  printBrief(dataPointer, keyLen);
+  ::printBrief(dataPointer, keyLen, FALSE /* no end of line wanted */);
 }
 #endif

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/executor/MdamPoint.h
----------------------------------------------------------------------
diff --git a/core/sql/executor/MdamPoint.h b/core/sql/executor/MdamPoint.h
index 7eb4742..2dcdf0e 100644
--- a/core/sql/executor/MdamPoint.h
+++ b/core/sql/executor/MdamPoint.h
@@ -103,9 +103,6 @@ public:
   NA_EIDPROC void printBrief() const;
   #endif /* NA_MDAM_EXECUTOR_DEBUG */
   
-  #ifdef _DEBUG
-  NA_EIDPROC static void printBrief(char* ptr, Lng32 len) ;
-  #endif 
 private:
 
   // The point's value.

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/executor/ex_mdam.cpp
----------------------------------------------------------------------
diff --git a/core/sql/executor/ex_mdam.cpp b/core/sql/executor/ex_mdam.cpp
index f238911..9f6e35a 100644
--- a/core/sql/executor/ex_mdam.cpp
+++ b/core/sql/executor/ex_mdam.cpp
@@ -762,6 +762,8 @@ while ((predIterator.positionToNextOr(&currentPred_)) &&
                                             columnGenInfo_->getLength(),
                                             mdamIntervalHeap,
                                             mdamRefListEntryHeap);
+                or_temp1.deleteAllIntervals(mdamIntervalHeap,
+                                            mdamRefListEntryHeap);
               }
           }
       }

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/nskgmake/Makerules.linux
----------------------------------------------------------------------
diff --git a/core/sql/nskgmake/Makerules.linux b/core/sql/nskgmake/Makerules.linux
index 802f670..9d90ebe 100755
--- a/core/sql/nskgmake/Makerules.linux
+++ b/core/sql/nskgmake/Makerules.linux
@@ -216,6 +216,9 @@ ifeq ($(FLAVOR),debug)
    SYS_CXXFLAGS += -O0
    SYS_DEFS += -D_DEBUG
 
+   # uncomment the next line if you want the MDAM run-time debugging code
+   #SYS_CXXFLAGS += -DNA_MDAM_EXECUTOR_DEBUG
+
    # for coverage checking support
    ifeq ($(SQ_COVERAGE),1)
       SYS_CXXFLAGS += --coverage

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/e3c7edce/core/sql/optimizer/mdam.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/mdam.cpp b/core/sql/optimizer/mdam.cpp
index 3a0a82c..01d99f4 100644
--- a/core/sql/optimizer/mdam.cpp
+++ b/core/sql/optimizer/mdam.cpp
@@ -1680,20 +1680,28 @@ void MdamKey::preCodeGen(ValueIdSet& executorPredicates,
   // might have wanted).  This code *does not* print any
   // *official* warnings (but it prints a warning to the console).
   // --------------------------------------------------------------------
-  // if the mdamkey has more than one disjunct and if one
-  // disjunct does not have key  predicates:
 
-  // Now, tell each element of the columnOrderListArray up to
-  // which column they contain valid key predicates:
+  // Note: The "for" loop below does two very important things:
+  // 1. Detects empty disjuncts (as discussed above) 
+  //
+  // and, while it is doing that,
+  //
+  // 2. Sets the stop column for each disjunct.
+  //
+  // Note that if we don't set the stop column, then MDAM will traverse
+  // *all* of the key columns, whether they have predicates or not, and
+  // performance will be terrible.
+
   NABoolean isEmpty = FALSE;
-  if (!partKeyPredsAdded)
+
     for (i=0; i < curDisjuncts->entries(); i++)
       {
         (*columnOrderListPtrArrayPtr_)[i]->setStopColumn(getStopColumn(i));
 
         // Find out if the disjunct has key preds:
         const ColumnOrderList& coList= *((*columnOrderListPtrArrayPtr_)[i]);
-        isEmpty = TRUE; // assume it is empty
+        if (!partKeyPredsAdded)
+          isEmpty = TRUE; // assume it is empty
         for (CollIndex j=0; j < coList.entries(); j++)
           {
             if (coList[j] AND (NOT coList[j]->isEmpty()))
@@ -1707,7 +1715,7 @@ void MdamKey::preCodeGen(ValueIdSet& executorPredicates,
             FSOWARNING("Empty disjunct");
             break;
           }
-      } // find out whether there is an empty disjunct
+      } // set stop columns and also find out whether there is an empty disjunct
 
   if (isEmpty)
     {


[3/3] incubator-trafodion git commit: Merge [TRAFODION-2661] PR 1146 Fix three MDAM bugs

Posted by db...@apache.org.
Merge [TRAFODION-2661] PR 1146 Fix three MDAM bugs


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

Branch: refs/heads/master
Commit: 05d14d47fec6f8469cf8da42f09462690af5bbbd
Parents: b0392f4 e3c7edc
Author: Dave Birdsall <db...@apache.org>
Authored: Wed Jul 5 21:47:07 2017 +0000
Committer: Dave Birdsall <db...@apache.org>
Committed: Wed Jul 5 21:47:07 2017 +0000

----------------------------------------------------------------------
 core/sql/common/str.cpp              | 10 +++---
 core/sql/common/str.h                |  6 ++--
 core/sql/executor/MdamPoint.cpp      |  2 +-
 core/sql/executor/MdamPoint.h        |  3 --
 core/sql/executor/ex_mdam.cpp        |  2 ++
 core/sql/nskgmake/Makerules.linux    |  3 ++
 core/sql/optimizer/ScanOptimizer.cpp | 52 ++-----------------------------
 core/sql/optimizer/mdam.cpp          | 22 ++++++++-----
 8 files changed, 33 insertions(+), 67 deletions(-)
----------------------------------------------------------------------