calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Reshetov <alexander.v.reshe...@gmail.com>
Subject Unclear hard-coding in planner and other parts of source code
Date Fri, 24 Jul 2015 15:36:37 GMT
Hello all,

First of all, while this is my first post in this mailing list, I'd like to
say that Calcite is very powerful project and has great target. Thanks for
it!

While I'm working on calcite's storage adapter to custom binary format, I
found some strange parts on code. Like this one:

if (true) {
    return this == that
        || this.rowCount <= that.rowCount;
}

This part is from org.apache.calcite.plan.volcano.VolcanoCost [0].
There is also second such condition. I found (with grep) similar thing also
in:
- org.apache.calcite.plan.volcano.VolcanoPlanner [1]
- org.apache.calcite.rel.rules.ReduceDecimalsRule [2]`

Moreover there is much more of such, but now with 'if (false)' statement:
- org.apache.calcite.adapter.enumerable.RexImpTable [3]
- org.apache.calcite.plan.volcano.RelSubset [4]
- org.apache.calcite.prepare.CalcitePrepareImpl [5]
- org.apache.calcite.sql.SqlCharStringLiteral [6]
- org.apache.calcite.sql.type.SqlTypeName [7]
- org.apache.calcite.sql.validate.SqlValidatorUtil [8]
- org.apache.calcite.sql2rel.StandardConvertletTable [9]
- org.apache.calcite.util.NlsString [10]
- org.apache.calcite.util.Util [11]

So actually this checks will always execute no matter what..
Also I see many true/false passed not through variables but literally, like
someMethod(true).

Actually because of some of this hardcoded values cost optimizer does not
work, at least in some cases.

Could you please give me a hint what is this? It looks like it was created
with some code generator or so. Or such parts in your opinion is not
production ready and you hard code specific behaviour?


0 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoCost.java#L99
1 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java#L756
2 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/rel/rules/ReduceDecimalsRule.java#L153
3 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java#L1135
4 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/plan/volcano/RelSubset.java#L303
5 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/prepare/CalcitePrepareImpl.java#L505
6 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/sql/SqlCharStringLiteral.java#L74
7 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/sql/type/SqlTypeName.java#L221
8:
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java#L127
9 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java#L243
10 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/util/NlsString.java#L171
11 :
https://github.com/apache/incubator-calcite/blob/master/core/src/main/java/org/apache/calcite/util/Util.java#L190

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