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 > <[email protected]> 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
