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/10/05 15:25:12 UTC
[3/5] incubator-trafodion git commit: Reworks based on Hans review
comments
Reworks based on Hans review comments
Project: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/commit/1877b7d5
Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/tree/1877b7d5
Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/diff/1877b7d5
Branch: refs/heads/master
Commit: 1877b7d5be8337777d16f7603b55dd6bfc6e638a
Parents: 6355b69
Author: Dave Birdsall <db...@apache.org>
Authored: Wed Oct 4 20:55:15 2017 +0000
Committer: Dave Birdsall <db...@apache.org>
Committed: Wed Oct 4 20:55:15 2017 +0000
----------------------------------------------------------------------
core/sql/generator/GenRelUpdate.cpp | 101 +++++++++++++++++--------------
core/sql/optimizer/ValueDesc.h | 2 +
core/sql/sqlcomp/DefaultConstants.h | 2 -
core/sql/sqlcomp/nadefaults.cpp | 1 -
4 files changed, 58 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/1877b7d5/core/sql/generator/GenRelUpdate.cpp
----------------------------------------------------------------------
diff --git a/core/sql/generator/GenRelUpdate.cpp b/core/sql/generator/GenRelUpdate.cpp
index acfe7f9..9cda158 100644
--- a/core/sql/generator/GenRelUpdate.cpp
+++ b/core/sql/generator/GenRelUpdate.cpp
@@ -301,22 +301,21 @@ static short genUpdConstraintExpr(Generator * generator,
{
ExpGenerator * expGen = generator->getExpGenerator();
- // The Attributes for the table columns refer to the old values of the column.
- // The constraints must operate on the new values, though. So we must do a
- // switcheroo on the Attributes for the update expression. The target value IDs
- // come from targetRecExprArray.
-
- ValueIdList savedSourceVIDlist;
- NAList<Attributes*> savedSourceAttrsList(generator->wHeap());
+ // The ValueIds in the constrTree refer to the source values of the columns.
+ // Construct a ValueIdMap so we can rewrite the constrTree to refer to the
+ // target value of the columns.
+ ValueIdMap sourceToTarget; // top values will be source, bottom will be target
+
for (ValueId sourceValId = constraintColumns.init();
constraintColumns.next(sourceValId);
constraintColumns.advance(sourceValId))
{
+ GenAssert(sourceValId.getItemExpr()->getOperatorType() == ITM_INDEXCOLUMN,
+ "unexpected type of constraint expression column");
NAColumn * sourceCol = ((IndexColumn*)sourceValId.getItemExpr())->getNAColumn();
ValueId targetValId;
- NABoolean found = FALSE;
- for (CollIndex ni = 0; (!found) && (ni < targetRecExprArray.entries()); ni++)
+ for (CollIndex ni = 0; (ni < targetRecExprArray.entries()); ni++)
{
const ItemExpr *assignExpr = targetRecExprArray[ni].getItemExpr();
targetValId = assignExpr->child(0)->castToItemExpr()->getValueId();
@@ -325,50 +324,62 @@ static short genUpdConstraintExpr(Generator * generator,
targetCol = ((BaseColumn*)targetValId.getItemExpr())->getNAColumn();
else if (targetValId.getItemExpr()->getOperatorType() == ITM_INDEXCOLUMN)
targetCol = ((IndexColumn*)targetValId.getItemExpr())->getNAColumn();
-
- if ((targetCol) &&
- (targetCol->getColName() == sourceCol->getColName()) &&
- (targetCol->getHbaseColFam() == sourceCol->getHbaseColFam()) &&
- (targetCol->getHbaseColQual() == sourceCol->getHbaseColQual()) &&
- (targetCol->getNATable()->getTableName().getQualifiedNameAsAnsiString() ==
- sourceCol->getNATable()->getTableName().getQualifiedNameAsAnsiString()))
+
+ if (targetCol && sourceCol->getPosition() == targetCol->getPosition())
{
- found = TRUE;
- break;
+ GenAssert(sourceCol->getNATable() == targetCol->getNATable(),
+ "expecting same NATable for constraint source and target");
+
+ // We found the target column matching the source column in the
+ // targetRecExprArray. Now, an optimization: If the assignment
+ // merely moves the old column value to the new, there is no need
+ // to map it.
+
+ ValueId rhsValId = assignExpr->child(1)->castToItemExpr()->getValueId();
+ NAColumn *rhsCol = NULL;
+ if (rhsValId.getItemExpr()->getOperatorType() == ITM_BASECOLUMN)
+ rhsCol = ((BaseColumn*)rhsValId.getItemExpr())->getNAColumn();
+ else if (rhsValId.getItemExpr()->getOperatorType() == ITM_INDEXCOLUMN)
+ rhsCol = ((IndexColumn*)rhsValId.getItemExpr())->getNAColumn();
+
+ if (rhsCol && rhsCol->getPosition() == targetCol->getPosition())
+ {
+ // assignment copies old column value to target without change;
+ // no need to map
+ GenAssert(rhsCol->getNATable() == targetCol->getNATable(),
+ "expecting same NATable for assignment source and target");
+ }
+ else
+ {
+ // the column value is changing (or maybe this is an insert),
+ // so map it
+ sourceToTarget.addMapEntry(sourceValId, targetValId);
+ }
+ ni = targetRecExprArray.entries(); // found it, no need to search further
}
}
+ }
- if (found)
- {
- Attributes * sourceValAttr = (generator->addMapInfo(sourceValId, 0))->getAttr();
- Attributes * targetValAttr = (generator->getMapInfo(targetValId, 0))->getAttr();
+ // If there is anything to map, rewrite the constraint expression
+ // and generate it. If there is nothing to map, that means none of
+ // the constraint expression columns is changed (which implies this
+ // is an update expr and not an insert, by the way). In that case, we
+ // don't need to generate the constraint expression as the constraint
+ // should already be satisfied by the old values.
- // Save original location attributes so we can change them back after
- // generating the update constraint expression
-
- Attributes * savedValAttr = new(generator->wHeap()) Attributes();
- savedValAttr->copyLocationAttrs(sourceValAttr);
- savedSourceAttrsList.insert(savedValAttr);
- savedSourceVIDlist.insert(sourceValId);
-
- sourceValAttr->copyLocationAttrs(targetValAttr);
- }
+ if (sourceToTarget.entries() > 0)
+ {
+ // map the ValueIds in the constraint tree to target values
+ ValueId mappedConstrTree;
+ sourceToTarget.rewriteValueIdDown(constrTree->getValueId(),mappedConstrTree /* out */);
+ // generate the expression
+ expGen->generateExpr(mappedConstrTree, ex_expr::exp_SCAN_PRED,
+ targetExpr);
}
-
- // Now that we have remapped the Attributes for the columns to their values
- // in the new record, we can generate the update constraint expression.
-
- expGen->generateExpr(constrTree->getValueId(), ex_expr::exp_SCAN_PRED,
- targetExpr);
-
- // Now put the Attributes back the way they were.
-
- for (Lng32 i = 0; i < savedSourceVIDlist.entries(); i++)
+ else
{
- ValueId sourceValId = savedSourceVIDlist[i];
- Attributes * sourceValAttr = (generator->getMapInfo(sourceValId, 0))->getAttr();
- sourceValAttr->copyLocationAttrs(savedSourceAttrsList[i]);
+ targetExpr = NULL;
}
return 0;
http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/1877b7d5/core/sql/optimizer/ValueDesc.h
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/ValueDesc.h b/core/sql/optimizer/ValueDesc.h
index 11b1be1..1074749 100644
--- a/core/sql/optimizer/ValueDesc.h
+++ b/core/sql/optimizer/ValueDesc.h
@@ -1533,6 +1533,8 @@ public:
NABoolean operator != (const ValueIdMap &other) const
{ return NOT operator==(other); }
+ CollIndex entries() { return topValues_.entries(); }
+
// accessor functions
const ValueIdList & getTopValues() const { return topValues_; }
const ValueIdList & getBottomValues() const { return bottomValues_; }
http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/1877b7d5/core/sql/sqlcomp/DefaultConstants.h
----------------------------------------------------------------------
diff --git a/core/sql/sqlcomp/DefaultConstants.h b/core/sql/sqlcomp/DefaultConstants.h
index 3852e70..e077ce7 100644
--- a/core/sql/sqlcomp/DefaultConstants.h
+++ b/core/sql/sqlcomp/DefaultConstants.h
@@ -3350,8 +3350,6 @@ enum DefaultConstants
HBASE_MAX_NUM_SEARCH_KEYS,
CALL_EMBEDDED_ARKCMP,
- HBASE_TRANSFORM_UPDATE_TO_DELETE_INSERT,
-
// ON, if blocks should be cached for this scan. This includes subset scans used by Update and Delete
HBASE_CACHE_BLOCKS,
http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/1877b7d5/core/sql/sqlcomp/nadefaults.cpp
----------------------------------------------------------------------
diff --git a/core/sql/sqlcomp/nadefaults.cpp b/core/sql/sqlcomp/nadefaults.cpp
index 382d430..1d3cb76 100644
--- a/core/sql/sqlcomp/nadefaults.cpp
+++ b/core/sql/sqlcomp/nadefaults.cpp
@@ -1782,7 +1782,6 @@ SDDkwd__(EXE_DIAGNOSTIC_EVENTS, "OFF"),
DDkwd__(HBASE_SMALL_SCANNER, "OFF"),
DDkwd__(HBASE_SQL_IUD_SEMANTICS, "ON"),
DDkwd__(HBASE_STATS_PARTITIONING, "ON"),
- DDkwd__(HBASE_TRANSFORM_UPDATE_TO_DELETE_INSERT, "OFF"),
// If set to 'OFF' we get a stub cost of 1 for update operations.
// We can remove this once the delete costing code has broader