[GitHub] [calcite] liyafan82 edited a comment on pull request #1944: [CALCITE-3956] Unify comparison logic for RelOptCost

[GitBox <gi...@apache.org>]

liyafan82 edited a comment on pull request #1944:
URL: https://github.com/apache/calcite/pull/1944#issuecomment-652837339


   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.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org