Skip to content

[CALCITE-5640] Add SAFE_ADD function (enabled in BigQuery library) - #3370

Merged
tanclary merged 1 commit into
apache:mainfrom
tanclary:safe-add
Aug 18, 2023
Merged

[CALCITE-5640] Add SAFE_ADD function (enabled in BigQuery library)#3370
tanclary merged 1 commit into
apache:mainfrom
tanclary:safe-add

Conversation

@tanclary

@tanclary tanclary commented Aug 16, 2023

Copy link
Copy Markdown
Contributor

This PR continues working towards the goal of implementing all BigQuery safe arithmetic functions, captured in CALCITE-5591. SAFE_MULTIPLY was merged recently in 1b8dc6 and this PR follows a very similar structure, except of course using addition rather than multiplication.

Any questions or suggestions would be appreciated as always!

@tanclary
tanclary force-pushed the safe-add branch 3 times, most recently from ca2bb36 to 9946240 Compare August 16, 2023 16:34

/** SQL <code>SAFE_ADD</code> function applied to BigDecimal and long values. */
public static @Nullable BigDecimal safeAdd(BigDecimal b0, long b1) {
BigDecimal ans = b0.add(BigDecimal.valueOf(b1));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe using return safeAdd(b1, b0) would be better.
Same comment for
Double safeAdd(long b0, double b1)
Double safeAdd(BigDecimal b0, double b1)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a great suggestion. I have updated the methods for safeAdd and safeMultiply as well. Let me know if you have any other thoughts, thanks!

@sonarqubecloud

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

98.9% 98.9% Coverage
40.0% 40.0% Duplication

@macroguo-ghy macroguo-ghy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tanclary
tanclary merged commit a651ea6 into apache:main Aug 18, 2023
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.

2 participants