trafodion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dbirds...@apache.org
Subject [1/3] trafodion git commit: [TRAFODION-2840] Make [first n] with ORDER BY views non-updatable
Date Fri, 26 Jan 2018 23:19:31 GMT
Repository: trafodion
Updated Branches:
  refs/heads/master c83471122 -> 2cdcdad51


[TRAFODION-2840] Make [first n] with ORDER BY views non-updatable


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

Branch: refs/heads/master
Commit: ebf7283de994729f898fa5a9f5e476fa03b40a4f
Parents: 63e1083
Author: Dave Birdsall <dbirdsall@apache.org>
Authored: Thu Jan 25 00:16:08 2018 +0000
Committer: Dave Birdsall <dbirdsall@apache.org>
Committed: Thu Jan 25 00:16:08 2018 +0000

----------------------------------------------------------------------
 core/sql/generator/GenPreCode.cpp     | 18 +++++-
 core/sql/optimizer/BindRelExpr.cpp    | 35 +++++++++---
 core/sql/optimizer/Inlining.cpp       |  6 +-
 core/sql/optimizer/NormRelExpr.cpp    | 36 +++++++++++-
 core/sql/optimizer/OptPhysRelExpr.cpp | 89 ++++++++++++++++++++++++++++++
 core/sql/optimizer/RelExpr.cpp        |  4 +-
 core/sql/optimizer/RelMisc.h          | 19 ++++++-
 7 files changed, 192 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/generator/GenPreCode.cpp
----------------------------------------------------------------------
diff --git a/core/sql/generator/GenPreCode.cpp b/core/sql/generator/GenPreCode.cpp
index cc17512..208ca0b 100644
--- a/core/sql/generator/GenPreCode.cpp
+++ b/core/sql/generator/GenPreCode.cpp
@@ -1604,8 +1604,20 @@ RelExpr * RelRoot::preCodeGen(Generator * generator,
   // can teach optimizer to do this.
   if ((getFirstNRows() != -1) || (getFirstNRowsParam()))
     {
+      // As of JIRA TRAFODION-2840, the binder always adds the FirstN,
+      // except in the case of output rowsets. So, the only time we 
+      // should now be going through this code is a SELECT query using
+      // output rowsets + FirstN + ORDER BY. A follow-on JIRA,
+      // TRAFODION-2924, will take care of that case and delete this code.
+      // (As a matter of design, it is highly undesireable to sometimes
+      // create the FirstN in the Binder and sometimes in the Generator;
+      // that means that any FirstN-related semantic checks in the 
+      // intervening passes will need two completely separate 
+      // implementations.)
+
       RelExpr * firstn = new(generator->wHeap()) FirstN(child(0),
 							getFirstNRows(),
+							needFirstSortedRows(),   
                                                         getFirstNRowsParam());
 
       // move my child's attributes to the firstN node.
@@ -5872,7 +5884,8 @@ RelExpr * GroupByAgg::preCodeGen(Generator * generator,
       && (getFirstNRows() == 1))
     {
       RelExpr * firstnNode = new(generator->wHeap()) FirstN(child(0),
-							    getFirstNRows());
+							    getFirstNRows(),
+							    FALSE /* [any n] is good enough */);
       firstnNode->setEstRowsUsed(getEstRowsUsed());
       firstnNode->setMaxCardEst(getMaxCardEst());
       firstnNode->setInputCardinality(child(0)->getInputCardinality());
@@ -10565,7 +10578,8 @@ RelExpr* PhyPack::preCodeGen(Generator* generator,
   if (getFirstNRows() != -1)
     {
       RelExpr * firstn = new(generator->wHeap()) FirstN(child(0),
-                                                        getFirstNRows());
+                                                        getFirstNRows(),
+                                                        FALSE /* [any n] is good enough */);
 
       // move my child's attributes to the firstN node.
       // Estimated rows will be mine.

http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/optimizer/BindRelExpr.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/BindRelExpr.cpp b/core/sql/optimizer/BindRelExpr.cpp
index 53abc71..7beaf9e 100644
--- a/core/sql/optimizer/BindRelExpr.cpp
+++ b/core/sql/optimizer/BindRelExpr.cpp
@@ -6662,17 +6662,38 @@ RelExpr *RelRoot::bindNode(BindWA *bindWA)
         }
     }
 
-  if ((NOT hasOrderBy()) &&
-      ((getFirstNRows() != -1) ||
-       (getFirstNRowsParam())))
+  if ((getFirstNRows() != -1) ||
+       (getFirstNRowsParam()))
     {
       // create a firstN node to retrieve firstN rows.
       FirstN * firstn = new(bindWA->wHeap())
-        FirstN(child(0), getFirstNRows(), getFirstNRowsParam());
+        FirstN(child(0), getFirstNRows(), needFirstSortedRows(), getFirstNRowsParam()); 
 
+
       firstn->bindNode(bindWA);
       if (bindWA->errStatus())
         return NULL;
 
+      // Note: For ORDER BY + [first n], we want to sort the rows before 
+      // picking just n of them. (We don't do this for [any n].) We might
+      // be tempted to copy the orderByTree into the FirstN node at this
+      // point, but this doesn't work. Instead, we copy the bound ValueIds
+      // at normalize time. We have to do this in case there are expressions
+      // involved in the ORDER BY and there is a DESC. The presence of the
+      // Inverse node at the top of the expression tree seems to cause the
+      // expressions underneath to be bound to different ValueIds, which 
+      // causes coverage tests in FirstN::createContextForAChild requirements
+      // generation to fail. An example of where this occurs is:
+      //
+      // prepare s1 from
+      //   select [first 2] y, x from
+      //    (select a,b + 26 from t1) as t(x,y)
+      //   order by y desc;
+      //
+      // If we copy the ORDER BY ItemExpr tree and rebind, we get a different
+      // ValueId for the expression b + 26 in the child characteristic outputs
+      // than what we get for the child of Inverse in Inverse(B + 26). The
+      // trick of copying the already-bound ORDER BY clause later avoids this.
+
       setChild(0, firstn);
 
       // reset firstN indication in the root node.
@@ -11545,7 +11566,7 @@ RelExpr *Update::bindNode(BindWA *bindWA)
       if (scanNode->getFirstNRows() >= 0)
         {
           FirstN * firstn = new(bindWA->wHeap())
-            FirstN(scanNode, scanNode->getFirstNRows(), NULL);
+            FirstN(scanNode, scanNode->getFirstNRows(), FALSE /* there's no ORDER BY on
an UPDATE */, NULL);
           firstn->bindNode(bindWA);
           if (bindWA->errStatus())
             return NULL;
@@ -11901,7 +11922,7 @@ RelExpr *Delete::bindNode(BindWA *bindWA)
 
       RelExpr * childNode = child(0)->castToRelExpr();
       FirstN * firstn = new(bindWA->wHeap())
-        FirstN(childNode, getFirstNRows(), NULL);
+        FirstN(childNode, getFirstNRows(), FALSE /* There's no ORDER BY on a DELETE */, NULL);
       firstn->bindNode(bindWA);
       if (bindWA->errStatus())
         return NULL;
@@ -11988,7 +12009,7 @@ RelExpr *Delete::bindNode(BindWA *bindWA)
       // during handleInlining. Occurs when DELETE FIRST N is used on table with no
       // dependent objects. 
       FirstN * firstn = new(bindWA->wHeap())
-        FirstN(boundExpr, getFirstNRows());
+        FirstN(boundExpr, getFirstNRows(), FALSE /* There's no ORDER BY on a DELETE */ );
       if (NOT(scanNode && scanNode->getSelectionPred().containsSubquery()))
         firstn->setCanExecuteInDp2(TRUE);
       firstn->bindNode(bindWA);

http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/optimizer/Inlining.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/Inlining.cpp b/core/sql/optimizer/Inlining.cpp
index a3c096a..f8d7b66 100644
--- a/core/sql/optimizer/Inlining.cpp
+++ b/core/sql/optimizer/Inlining.cpp
@@ -3132,7 +3132,7 @@ RelExpr *GenericUpdate::inlineOnlyRIandIMandMVLogging(BindWA *bindWA,
 	{
 	  // create a firstN node to delete N rows.
 	  FirstN * firstn = new(bindWA->wHeap())
-	    FirstN(topNode, topNode->getFirstNRows());
+	    FirstN(topNode, topNode->getFirstNRows(), FALSE /* No ordering requirement */);
 	  firstn->bindNode(bindWA);
 	  if (bindWA->errStatus())
 	    return NULL;
@@ -3314,7 +3314,7 @@ RelExpr *GenericUpdate::inlineAfterOnlyBackbone(BindWA *bindWA,
   {
     // create a firstN node to delete N rows.
     FirstN * firstn = new(bindWA->wHeap())
-      FirstN(topNode, topNode->getFirstNRows());
+      FirstN(topNode, topNode->getFirstNRows(), FALSE /* No ordering requirement */);
     firstn->bindNode(bindWA);
     if (bindWA->errStatus())
       return NULL;
@@ -3429,7 +3429,7 @@ RelExpr *GenericUpdate::inlineAfterOnlyBackboneForUndo(BindWA *bindWA,
     {
       // create a firstN node to delete N rows.
       FirstN * firstn = new(bindWA->wHeap())
-	FirstN(topNode, topNode->getFirstNRows());
+	FirstN(topNode, topNode->getFirstNRows(), FALSE /* No ordering requirement */);
       firstn->bindNode(bindWA);
       if (bindWA->errStatus())
 	return NULL;

http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/optimizer/NormRelExpr.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/NormRelExpr.cpp b/core/sql/optimizer/NormRelExpr.cpp
index 781ecc9..95c526b 100644
--- a/core/sql/optimizer/NormRelExpr.cpp
+++ b/core/sql/optimizer/NormRelExpr.cpp
@@ -6925,6 +6925,22 @@ RelExpr * Insert::normalizeNode(NormWA & normWARef)
   if (normalizedThis->getOperatorType() == REL_LEAF_INSERT)
     return normalizedThis;
 
+  // If there is an ORDER BY + a [first n], copy the ORDER BY ValueIds
+  // down to the FirstN node so we order the rows before taking the first n.
+  // If it is ORDER BY + [any n] we don't do this, as it is sufficient
+  // and more efficient to sort the rows after taking just n of them.
+  // Note: We do this at normalize time instead of bind time because if
+  // there are complex expressions in the ORDER BY, the binder will get
+  // different ValueIds for the non-leaf nodes which screws up coverage
+  // tests. Doing it here the ValueIds have already been uniquely computed.
+  if ((reqdOrder().entries() > 0) && 
+      (child(0)->getOperatorType() == REL_FIRST_N))
+    {
+      FirstN * firstn = (FirstN *)child(0)->castToRelExpr();
+      if (firstn->isFirstN())  // that is, [first n], not [any n] or [last n]
+        firstn->reqdOrder().insert(reqdOrder());
+    }
+
   // If the child is not a Tuple node - nothing to do here.
   CMPASSERT(normalizedThis->getArity() > 0);
   if (normalizedThis->child(0)->getOperatorType() != REL_TUPLE)
@@ -7597,6 +7613,24 @@ RelExpr * RelRoot::normalizeNode(NormWA & normWARef)
   }
 
   // ---------------------------------------------------------------------
+  // If there is an ORDER BY + a [first n], copy the ORDER BY ValueIds
+  // down to the FirstN node so we order the rows before taking the first n.
+  // If it is ORDER BY + [any n] we don't do this, as it is sufficient
+  // and more efficient to sort the rows after taking just n of them.
+  // Note: We do this at normalize time instead of bind time because if
+  // there are complex expressions in the ORDER BY, the binder will get
+  // different ValueIds for the non-leaf nodes which screws up coverage
+  // tests. Doing it here the ValueIds have already been uniquely computed.
+  // ---------------------------------------------------------------------
+  if ((reqdOrder().entries() > 0) && 
+      (child(0)->getOperatorType() == REL_FIRST_N))
+    {
+      FirstN * firstn = (FirstN *)child(0)->castToRelExpr();
+      if (firstn->isFirstN())  // that is, [first n], not [any n] or [last n]
+        firstn->reqdOrder().insert(reqdOrder());
+    }
+
+  // ---------------------------------------------------------------------
   // Normalize the child.
   // ---------------------------------------------------------------------
   child(0) = child(0)->normalizeNode(normWARef);
@@ -10908,5 +10942,5 @@ NABoolean CqsWA::isMPTable(const NAString &tableName)
     return FALSE;
   }
 } // CqsWA::isMPTable()
- 
+
  

http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/optimizer/OptPhysRelExpr.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/OptPhysRelExpr.cpp b/core/sql/optimizer/OptPhysRelExpr.cpp
index 838eae4..eff9944 100644
--- a/core/sql/optimizer/OptPhysRelExpr.cpp
+++ b/core/sql/optimizer/OptPhysRelExpr.cpp
@@ -15499,6 +15499,95 @@ GenericUtilExpr::synthPhysicalProperty(const Context* myContext,
   return sppForMe;
 } //  GenericUtilExpr::synthPhysicalProperty()
 
+// -----------------------------------------------------------------------
+// FirstN::createContextForAChild()
+// The FirstN node may have an order by requirement that it needs to
+// pass to its child context. Other than that, this method is quite
+// similar to the default implementation, RelExpr::createContextForAChild.
+// The arity of FirstN is always 1, so some logic from the default
+// implementation that deals with childIndex > 0 is unnecessary and has
+// been removed.
+// -----------------------------------------------------------------------
+Context * FirstN::createContextForAChild(Context* myContext,
+                                 PlanWorkSpace* pws,
+                                 Lng32& childIndex)
+{
+  const ReqdPhysicalProperty* rppForMe =
+                                    myContext->getReqdPhysicalProperty();
+
+  CMPASSERT(getArity() == 1);
+
+  childIndex = getArity() - pws->getCountOfChildContexts() - 1;
+
+  // return if we are done
+  if (childIndex < 0)
+    return NULL;
+
+  RequirementGenerator rg(child(childIndex), rppForMe);
+
+  if (reqdOrder().entries() > 0)
+    {
+      // replace our sort requirement with that implied by our ORDER BY clause
+
+      rg.removeSortKey();
+
+      ValueIdList sortKey;
+      sortKey.insert(reqdOrder());
+
+      // Shouldn't/Can't add a sort order type requirement
+      // if we are in DP2
+      if (rppForMe->executeInDP2())
+        rg.addSortKey(sortKey,NO_SOT);
+      else
+        rg.addSortKey(sortKey,ESP_SOT);
+    }
+
+  if (NOT pws->isEmpty())
+  {
+    const Context* childContext = pws->getLatestChildContext();
+
+    // ------------------------------------------------------------------
+    // Cost limit exceeded or got no solution? Give up since we only
+    // try one plan.
+    // ------------------------------------------------------------------
+    if(NOT (childContext AND childContext->hasOptimalSolution()))
+      return NULL;
+
+    if (NOT pws->isLatestContextWithinCostLimit())
+      return NULL;
+
+  }
+
+  if (NOT rg.checkFeasibility())
+    return NULL;
+
+  Lng32 planNumber = 0;
+
+  // ---------------------------------------------------------------------
+  // Compute the cost limit to be applied to the child.
+  // ---------------------------------------------------------------------
+  CostLimit* costLimit = computeCostLimit(myContext, pws);
+
+  // ---------------------------------------------------------------------
+  // Get a Context for optimizing the child.
+  // Search for an existing Context in the CascadesGroup to which the
+  // child belongs that requires the same properties as myContext.
+  // Reuse it, if found. Otherwise, create a new Context that contains
+  // the same rpp and input log prop as in myContext.
+  // ---------------------------------------------------------------------
+  Context* result = shareContext(childIndex, rg.produceRequirement(),
+                                 myContext->getInputPhysicalProperty(),
+                                 costLimit, myContext,
+                                 myContext->getInputLogProp());
+
+  // ---------------------------------------------------------------------
+  // Store the Context for the child in the PlanWorkSpace.
+  // ---------------------------------------------------------------------
+  pws->storeChildContext(childIndex, planNumber, result);
+
+  return result;
+
+} // FirstN::createContextForAChild()
 
 //<pb>
 //==============================================================================

http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/optimizer/RelExpr.cpp
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/RelExpr.cpp b/core/sql/optimizer/RelExpr.cpp
index c9d3f83..5c0af91 100644
--- a/core/sql/optimizer/RelExpr.cpp
+++ b/core/sql/optimizer/RelExpr.cpp
@@ -11651,13 +11651,15 @@ RelExpr * FirstN::copyTopNode(RelExpr *derivedNode,
   FirstN *result;
 
   if (derivedNode == NULL) {
-    result = new (outHeap) FirstN(NULL, getFirstNRows(), getFirstNRowsParam(),
+    result = new (outHeap) FirstN(NULL, getFirstNRows(), isFirstN(), getFirstNRowsParam(),
                                   outHeap);
     result->setCanExecuteInDp2(canExecuteInDp2());
   }
   else
     result = (FirstN *) derivedNode;
 
+  result->reqdOrder().insert(reqdOrder());
+
   return RelExpr::copyTopNode(result, outHeap);
 }
 

http://git-wip-us.apache.org/repos/asf/trafodion/blob/ebf7283d/core/sql/optimizer/RelMisc.h
----------------------------------------------------------------------
diff --git a/core/sql/optimizer/RelMisc.h b/core/sql/optimizer/RelMisc.h
index d25d21a..e6920ca 100644
--- a/core/sql/optimizer/RelMisc.h
+++ b/core/sql/optimizer/RelMisc.h
@@ -1600,12 +1600,14 @@ class FirstN : public RelExpr
 public:
  FirstN(RelExpr * child,
         Int64 firstNRows,
+        NABoolean isFirstN,
         ItemExpr * firstNRowsParam = NULL,
         CollHeap *oHeap = CmpCommon::statementHeap())
    : RelExpr(REL_FIRST_N, child, NULL, oHeap),
     firstNRows_(firstNRows),
     firstNRowsParam_(firstNRowsParam),
-    canExecuteInDp2_(FALSE)
+    canExecuteInDp2_(FALSE),
+    isFirstN_(isFirstN)
     {
       setNonCacheable();
     };
@@ -1613,6 +1615,12 @@ public:
   // sets the canExecuteInDp2 flag for the [LAST 1] operator
   // of an MTS delete and calls the base class implementation of bindNode.
   virtual RelExpr* bindNode(BindWA* bindWA);
+
+  // takes care of any ordering requirement on the child
+  virtual Context* createContextForAChild(Context* myContext,
+                     PlanWorkSpace* pws,
+                     Lng32& childIndex);
+
   //
   // Physical properties implemented in OptPhysRelExpr.cpp
   //
@@ -1650,11 +1658,20 @@ public:
   NABoolean canExecuteInDp2() const             { return canExecuteInDp2_; }
   virtual NABoolean computeRowsAffected()   const ;
 
+  NABoolean isFirstN()                          { return isFirstN_; }
+
+  ValueIdList & reqdOrder()                     { return reqdOrder_; }
+
 private:
   // Otherwise, return firstNRows_ at runtime.
   Int64 firstNRows_;
   ItemExpr * firstNRowsParam_;
   NABoolean canExecuteInDp2_;
+  NABoolean isFirstN_;  // TRUE if [first n], FALSE if [any n] or [last n]
+
+  // Optional ORDER BY to force ordering before applying First N; populated
+  // at normalizeNode time.
+  ValueIdList reqdOrder_;
 
 }; // class FirstN
 


Mime
View raw message