[
https://issues.apache.org/jira/browse/CALCITE-3956?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17155178#comment-17155178
]
Liya Fan commented on CALCITE-3956:
-----------------------------------
[~julianhyde] Thanks a lot for your attention, and sorry for my delay.
Please allow me to breifly summarize what we have so far.
Currently, we compare {{RelOptCost}} objects by three methods: \{{isLt}},
\{{isLe}}, and \{{equals}} . We do not use the \{{Comparable#compareTo}}
method, because the costs form a partial order.
Problems with this approach:
# Duplicate comparison logic is spread in 3 methods, making the code more
difficult to maintain.
# To get precise relations between costs, we may need to call the comparison
methods multiple times, leading to performance overhead.
# Since the comparison logic is spread in 3 methods, it is easy to produce
contradicive logic, making the cost objects not a partial order.
Our solution:
Add an abstract class with an abstract method \{{compareCost}} that
concentrates all the comparison logic. It also implements commonly used
operations, so the client code is expected to extend the abstract class, rather
than implementing the {{RelOptCost}} interface directly.
Results:
# It solves all the problems given above, without breaking existing clident
code.
# It makes things simpler, because
## Instead of implementing 3 methods, now the client code only needs to
implement one.
## The client does not need to worry about the contradictive logic, which
causes problems that are tricky to debug and diagnose.
> 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)
