[ https://issues.apache.org/jira/browse/CALCITE-3723?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17013334#comment-17013334 ]
Danny Chen commented on CALCITE-3723: ------------------------------------- Well, i can do that if you think the change would not breaks too much. I had thought about that, if we change the constructor of RelNodes, we should also change the RelBuilder factories according to the change. But overall, i also think that one constructor for a RelNode is more easy to maintain and less error-prone. Would fire a fix soon ~ > Following the change to add hints to RelNode, deprecate the old constructors > ---------------------------------------------------------------------------- > > Key: CALCITE-3723 > URL: https://issues.apache.org/jira/browse/CALCITE-3723 > Project: Calcite > Issue Type: Bug > Reporter: Julian Hyde > Priority: Major > > In CALCITE-482 and CALCITE-3590 we added constructors for various sub-classes > RelNode ({{LogicalProject}}, {{LogicalAggregate}}, and others) that take a > list of hints. But now those classes have two constructors. Our practice has > been to have only one (public, non-deprecated) constructor in each RelNode > class. (Otherwise we would have dozens.) So, please deprecate the old > constructors and change code that uses them. > Can we do this before 1.22? > Also note that the new and old constructors have exactly the same comment. > You should avoid that. But in this case, just remove the comment of the > deprecated constructor. > The non-hints constructor does {{new ArrayList<>()}}. Please change to use > {{ImmutableList.of()}}, which saves a malloc. > cc [~danny0405] and [~icshuo]. -- This message was sent by Atlassian Jira (v8.3.4#803005)