calcite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Hyde <jh...@apache.org>
Subject Re: Unclear hard-coding in planner and other parts of source code
Date Fri, 24 Jul 2015 16:26:55 GMT
In most cases, the ‘if (true)’ and ‘if (false)’ are the equivalent of commented-out
code. A bit embarrassing to us developers, but almost inevitable because Calcite and in particular
its planner is a complex beast and difficult to get working for case X without breaking existing
cases A, B, C, … Consider these the scars of software that is in production in several different
systems.

One of the dirty secrets of cost-based optimizers is that you use the simplest cost model
that does the job. A more complex cost model will have more bugs, and need stats that you
don’t have yet.

Can you share more about the case you are trying to optimize, and we will help you get it
working.

Julian


> On Jul 24, 2015, at 8:36 AM, Alexander Reshetov <alexander.v.reshetov@gmail.com>
wrote:
> 
> 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
View raw message