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:43 UTC
[2/3] incubator-trafodion git commit: Rework for [TRAFODION-2661]
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(¤tPred_)) &&
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)
{