Github user zellerh commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1585#discussion_r191880649
--- Diff: core/sql/optimizer/ItemExpr.cpp ---
@@ -3886,23 +3886,7 @@ ItemExpr * Parameter::copyTopNode(ItemExpr
*derivedNode, CollHeap* outHeap)
ItemExpr * DynamicParam::copyTopNode(ItemExpr *derivedNode, CollHeap*
outHeap)
{
- ItemExpr *result;
-
- if (derivedNode == NULL) {
- result = new (outHeap) DynamicParam(paramName_, indicatorName_,
outHeap);
- ((DynamicParam *) result)->setRowsetSize(rowsetSize_);
- ((DynamicParam *) result)->setRowsetInfo(rowsetInfo_);
- ((DynamicParam *) result)->setParamHeading(heading_);
- ((DynamicParam *) result)->setParamTablename(tablename_);
- // we remember our original dynamic parameter because we
- // must use their valueid at dynamicparam::codegen time
- ((DynamicParam *) result)->setOriginal(this);
- }
-
- else
- result = derivedNode;
-
- return Parameter::copyTopNode(result, outHeap);
+ return this;
--- End diff --
Good question, and I'm not sure I know the answer to that. It probably is
not possible to take trees that contain things like parameters, bound aggregate
nodes and BaseColumn nodes and make a copy that is valid.
Another thing I should have mentioned: I tried to remember what my original
design goal was when implementing copyTopNode. Now I think I remember - I said
it will just make a copy of an object with all data members, regardless of
whether that is meaningful or not. It would be the responsibility of the caller
to fix up things that are invalid in a copy. I don't like that design anymore,
given that we use copyTopNode() and copyTree() in so many places.
Something to be aware of is that these methods aren't generally usable, one
needs to be careful when calling them. Here are some cases that should
generally work:
- Copying the before pattern in an optimizer rule (note that this does not
copy the "cut" nodes, nor does it copy any ItemExpr nodes)
- Copying an unbound RelExpr and ItemExpr tree in the parser (e.g. for WITH
clause processing)
- Copying ItemExprs that can be evaluated at compile time, consisting only
of constants and ItemExpr operators, with no aggregates, parameters or other
special ItemExprs in them like current_time, sequence functions, etc.
- I'm sure there are more we could add to this list
---
