Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1414#discussion_r163930312
--- Diff: 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();
--- End diff --
Hmmmm... Suppose the parent is different at normalize time than it is here.
(I don't think that ever happens but let's suppose in the future some Optimizer
rule might make that happen.) Suppose further that the order requirement for
the parent is different. We can generate a valid plan by doing a sort before
doing first n (satisfying the first n + ORDER BY semantic) and then doing
another sort with different criteria afterward (satisfying the different sort
order of the parent). I think that's what this code achieves. If I remove this
line, then we won't generate a plan (which might be OK because there might be
other plan alternatives, but it might not in which case we'll get an error
2235). So my take is that leaving this line here is slightly more robust. What
do you think?
---
