[jira] [Commented] (CALCITE-3723) Following the change to add hints to RelNode, deprecate the old constructors

["Danny Chen (Jira)" <ji...@apache.org>]

    [ 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)