Skip to content

[CALCITE-6543] Change the RelOptCostImpl toString method to be consistent with VolcanoCost#3930

Merged
mihaibudiu merged 1 commit intoapache:mainfrom
ChengJie1053:main-RelOptCostImpl
Aug 27, 2024
Merged

[CALCITE-6543] Change the RelOptCostImpl toString method to be consistent with VolcanoCost#3930
mihaibudiu merged 1 commit intoapache:mainfrom
ChengJie1053:main-RelOptCostImpl

Conversation

@ChengJie1053
Copy link
Member

@ChengJie1053 ChengJie1053 commented Aug 24, 2024

image

@caicancai
Copy link
Member

I think this PR deserves a jira case

@ChengJie1053
Copy link
Member Author

I think this PR deserves a jira case

Ok, thanks for reviewing the code for me

@ChengJie1053 ChengJie1053 changed the title RelOptCostImpl toString is consistent with VolcanoCost Change the RelOptCostImpl toString method to be consistent with VolcanoCost Aug 24, 2024
@ChengJie1053 ChengJie1053 changed the title Change the RelOptCostImpl toString method to be consistent with VolcanoCost [CALCITE-6543]Change the RelOptCostImpl toString method to be consistent with VolcanoCost Aug 24, 2024
@ChengJie1053
Copy link
Member Author

I think this PR deserves a jira case

Hello, jira has been created
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6543?filter=allissues

@ChengJie1053 ChengJie1053 changed the title [CALCITE-6543]Change the RelOptCostImpl toString method to be consistent with VolcanoCost [CALCITE-6543] Change the RelOptCostImpl toString method to be consistent with VolcanoCost Aug 24, 2024
@Override public String toString() {
if (value == Double.MAX_VALUE) {
return "huge";
} else if (value == Double.POSITIVE_INFINITY) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be best to have a test example to illustrate

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for reviewing the code for me

@mihaibudiu
Copy link
Contributor

Ideally you should avoid code duplication, especially when the control-flow is so complicated.
In this case you should probably create a static method that takes a double and returns a string, and call it from both places.

@ChengJie1053
Copy link
Member Author

Ideally you should avoid code duplication, especially when the control-flow is so complicated. In this case you should probably create a static method that takes a double and returns a string, and call it from both places.

Ok, thank you for helping me review the code, I will modify this place

@ChengJie1053
Copy link
Member Author

Ideally you should avoid code duplication, especially when the control-flow is so complicated. In this case you should probably create a static method that takes a double and returns a string, and call it from both places.

Hello, the modified code has been submitted. If there is any problem, I will modify it again

@sonarqubecloud
Copy link

@caicancai
Copy link
Member

@ChengJie1053 You should not squash. Wait until no one else has any comments, then squash. We need to know what you changed in each commit, so that it is better to review.

@ChengJie1053
Copy link
Member Author

squash

Ok, thank you. I mistakenly thought that every commit requires a squash commit. I will pay attention to that in the future

@mihaibudiu mihaibudiu merged commit 787dfdb into apache:main Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants