calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <jh...@apache.org>
Subject Re: RelTraitPropagationVisitor and useless code policy
Date Fri, 05 Oct 2018 16:44:55 GMT
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.

Julian





> On Oct 5, 2018, at 6:53 AM, Vladimir Sitnikov <sitnikov.vladimir@gmail.com> 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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message