[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: RelTraitPropagationVisitor and useless code policy

Well, an Apache tenet is “just do it”.

But you need to be confident not just that you are right, but that it will be consensus that you have done the right thing. If you remove a piece of code and then there is disagreement, it is messy and causes conflict. So I’d recommend building consensus before you act if you have any doubts.

In this case, it is true that the code’s only purpose is to throw errors. But that alone is not sufficient to say that it is useless - it may have triggered thousands of times in a developer’s sandbox since 2014, and we would not know, because the developer fixed the errors before pushing.

This code is probably useless because the rules about how the planner treats trait-sets with missing traits have evolved over time. We are stricter in requiring that nodes have the right collection of traits. However there are still issues with heterogeneous trees, e.g. https://issues.apache.org/jira/browse/CALCITE-1681 <https://issues.apache.org/jira/browse/CALCITE-1681>. We still need to solve them, but I think we’d do it in a different way.


> On Oct 5, 2018, at 6:53 AM, Vladimir Sitnikov <sitnikov.vladimir@xxxxxxxxx> wrote:
>> For instance:
> - always delete, then discuss
> :-)
> If one is confident, no discussions needed.
> The code can be just deleted provided there's proper commit message.
> Vladimir