kgyrtkirk commented on code in PR #17039:
URL: https://github.com/apache/druid/pull/17039#discussion_r1759364314
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/querygen/DruidQueryGenerator.java:
##########
@@ -58,32 +60,64 @@ public DruidQueryGenerator(PlannerContext plannerContext,
DruidLogicalNode relRo
this.vertexFactory = new PDQVertexFactory(plannerContext, rexBuilder);
}
+ static class DruidNodeStack extends Stack<DruidLogicalNode>
Review Comment:
I've tried different approaches; `DruidLogicalNode` -s are immutable - and
supposed to describe a subtree - the index of that node in its parent is an
external detail - so I don't feel it right to add it.
I've covered 2 stacks with `DruidNodeStack` - which was better as it made
possible to introduce methods which could made the logic more readable.
And as alternate approach to attach the data more naturally I've added an
`Entry` class which contains the `index` and the `DruidLogicalNode` - I've
retained the custom methods...but it did disturbed some existing places a bit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
