Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Arithmetic function ROUND #615

Merged
merged 10 commits into from
Jan 31, 2024
Merged

Support Arithmetic function ROUND #615

merged 10 commits into from
Jan 31, 2024

Conversation

LeeJejune
Copy link
Contributor

Motivation

  • Support Arithmetic function ROUND.

Modifications

  • Create Jpql model and serializer for ROUND And Create JpqlRound, Expressions, Serializer Test

Commit Convention Rule

Commit type Description
feat New Feature
fix Fix bug
docs Documentation only changed
ci Change CI configuration
refactor Not a bug fix or add feature, just refactoring code
test Add Test case or fix wrong test case
style Only change the code style(ex. white-space, formatting)
chore It refers to minor tasks such as library version upgrade, typo correction, etc.
  • If you want to add some more commit type please describe it on the Pull Request

Result

  • The arithmetic function round is created.

Closes

@CLAassistant
Copy link

CLAassistant commented Jan 30, 2024

CLA assistant check
All committers have signed the CLA.

@@ -366,6 +366,22 @@ open class Jpql : JpqlDsl {
)
}

/**
* Creates an expression that represents the rounding of the specified property's value to a specified scale.
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be great to at least value instead of property's value, since not only the value of the property can be passed as a parameter to round, but also the literal.

* Creates an expression that represents the rounding of the specified property's value to a specified scale.
*/
@SinceJdsl("3.4.0")
fun <T : Any, V : Number> round(expr: KProperty1<T, @Exact V>, scale: Expression<Int>): Expression<Double> {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the ROUND is a primary function, so I think it would be better to have the function located below the DIV.

* Creates an expression that represents the rounding of the specified property's value to a specified scale.
*/
@SinceJdsl("3.4.0")
fun <T : Number> round(value: Expressionable<T>, scale: Expressionable<Int>): Expression<Double> {
Copy link
Member

Choose a reason for hiding this comment

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

In the jakarta specification, the return type of ROUND is the same type as the first argument: value.

Could you test this to make sure it's really correct? I think it would be okay to test with Int, Double, and BigDecimal only.

Copy link
Member

Choose a reason for hiding this comment

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

image

@shouwn
Copy link
Member

shouwn commented Jan 31, 2024

I would modify a bit from what you wrote, just the method names and such. Thanks for the help!

@shouwn shouwn merged commit b673f19 into line:develop Jan 31, 2024
3 of 4 checks passed
@shouwn
Copy link
Member

shouwn commented Jan 31, 2024

Oh, and I was expecting you to use the example module to test that Double and Int work with JPA locally.

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.

4 participants