[
https://issues.apache.org/jira/browse/CALCITE-3956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17150021#comment-17150021
]
Liya Fan commented on CALCITE-3956:
-----------------------------------
After more thoughts and discussions, we have changed the design in another way:
1. The {{RelOptCost}} interface is left unchanged.
2. Add an abstract class that implements the {{RelOptCost}} interface. Concrete
cost classes are supposed to extend this class.
3. The new method with concentrated comparison logic ({{compareCost}}) is
declared in the abstract class as an abstract method.
4. The concrete comparison methods {{isLt}}, {{isLe}} and {{equals}} methods
are implemented in the abstract class as final methods.
Benefits of this implementation:
1. It solves all the problems in the description, namely, duplicate logic,
contradicting logic, and performance overhead.
2. It preserves the partial order between cost objects.
3. Since it does not modify the RelOptCost interface, existing client code is
not affected.
Could you please given your valuable feedback? Thank you in advance.
> 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 10m
> 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)
