[ 
https://issues.apache.org/jira/browse/CALCITE-3956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17155008#comment-17155008
 ] 

Julian Hyde commented on CALCITE-3956:
--------------------------------------

Reviewing the [latest commit in the 
PR|https://github.com/apache/calcite/pull/1944/commits/c696c0f5b250d83006ac6a577698ba1cf9d534f5].
 I have forgotten why we are doing this. Are we aiming to simplify things? It 
doesn't seem simpler.

> Unify comparison logic for RelOptCost
> -------------------------------------
>
>                 Key: CALCITE-3956
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3956
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>            Reporter: Liya Fan
>            Assignee: Liya Fan
>            Priority: Major
>          Time Spent: 1h 50m
>  Remaining Estimate: 0h
>
> Currently, comparisons between RelOptCost objects are based on 3 methods:
> 1. {{boolean isLe(RelOptCost cost)}}
> 2. {{boolean isLt(RelOptCost cost)}}
> 3. {{boolean equals(RelOptCost cost)}}
> The 3 methods used in combination determine the relation between RelOptCost 
> objects. 
> There are some problems with this implementation:
> 1. Some logic is duplicate in the above methods, making it difficult to 
> maintain. 
> 2. To determine the relation between RelOptCost objects, we often need to 
> call more than one comparison methods, leading to performance overhead.
> 3. Since the logic is spread in multiple methods, it is easy to end up with 
> contradictive comparison logic, which will suprise the users. For example, 
> the following assertion should hold according to common sense:
> {{if a >=b, then we have a > b or a == b}}
> However, with the current implementation of {{VolcanoCost}}, we can easily 
> create instances that violate the above assertion. 
> To solve the problems, we want to make {{RelOptCost}} extends the 
> {{Comparable<RelOptCost>}}, so the comparison logic is unified in the 
> {{compareTo}} method, which solves the above problems. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to