-
Notifications
You must be signed in to change notification settings - Fork 92
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
Implement power operator #651
Changes from 8 commits
67a4be2
d7744f1
2384029
9822e0b
bfb8d48
8b80467
69109b2
5369802
1bfb2cf
5d231b1
c4a8a6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -646,6 +646,70 @@ open class Jpql : JpqlDsl { | |||||
return Expressions.ln(value.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Any> mod(expr: KProperty1<T, Int>, value: Int): Expression<Int> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It appears that the mod has not been implemented, but the commit appears to have been distorted. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
return Expressions.mod(Paths.path(expr), Expressions.value(value)) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun mod(value1: Expressionable<Int>, value2: Int): Expression<Int> { | ||||||
return Expressions.mod(value1.toExpression(), Expressions.value(value2)) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Any> mod(expr: KProperty1<T, Int>, value: Expression<Int>): Expression<Int> { | ||||||
return Expressions.mod(Paths.path(expr), value.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun mod(value1: Expressionable<Int>, value2: Expressionable<Int>): Expression<Int> { | ||||||
return Expressions.mod(value1.toExpression(), value2.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Create expression that calculates the powering of a numeric [base] to a specified [exponent]. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about writing something like this?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thank you. The sentence below looks easier to read. |
||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Any, V : Number> power(base: KProperty1<T, @Exact V>, exponent: Int): Expression<V> { | ||||||
return Expressions.power(Paths.path(base), Expressions.value(exponent)) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Create expression that calculates the powering of a numeric [base] to a specified [exponent]. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Number> power(base: Expressionable<T>, exponent: Int): Expression<T> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please write a method based on the specification in jakarta specification? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, I love doing reviews based on such solid knowledge. lol Thank you :) I feel like this allows me to get up close and personal with the Hibernate spec. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would recommend following the jakarta spec unless you have a special use case. This is because if Hibernate and EclipseLink have different specifications, you may not be able to decide which one to follow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh... Thank you. References |
||||||
return Expressions.power(base.toExpression(), Expressions.value(exponent)) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Create expression that calculates the powering of a numeric [base] to a specified [exponent]. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Any, V : Number> power(base: KProperty1<T, @Exact V>, exponent: Expression<Int>): Expression<V> { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When used as a parameter to a dsl method, it is more appropriate to write it with the Expressionable<> type instead of the Expression<> type e.g.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it to accept a wider range of material types?? reflected ! :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also see this in other code that I haven't checked. I'll fix it in one shot later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for checking it out. |
||||||
return Expressions.power(Paths.path(base), exponent.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Create expression that calculates the powering of a numeric [base] to a specified [exponent]. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Number> power(base: Expressionable<T>, exponent: Expressionable<Int>): Expression<T> { | ||||||
return Expressions.power(base.toExpression(), exponent.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the sign of value. | ||||||
* | ||||||
|
@@ -718,38 +782,6 @@ open class Jpql : JpqlDsl { | |||||
return Expressions.round(value.toExpression(), scale.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Any> mod(expr: KProperty1<T, Int>, value: Int): Expression<Int> { | ||||||
return Expressions.mod(Paths.path(expr), Expressions.value(value)) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun mod(value1: Expressionable<Int>, value2: Int): Expression<Int> { | ||||||
return Expressions.mod(value1.toExpression(), Expressions.value(value2)) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun <T : Any> mod(expr: KProperty1<T, Int>, value: Expression<Int>): Expression<Int> { | ||||||
return Expressions.mod(Paths.path(expr), value.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that represents the mod of values. | ||||||
*/ | ||||||
@SinceJdsl("3.4.0") | ||||||
fun mod(value1: Expressionable<Int>, value2: Expressionable<Int>): Expression<Int> { | ||||||
return Expressions.mod(value1.toExpression(), value2.toExpression()) | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates an expression that the number of elements of the collection. | ||||||
*/ | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,73 @@ | ||||||||||||||||
package com.linecorp.kotlinjdsl.dsl.jpql.expression | ||||||||||||||||
|
||||||||||||||||
import com.linecorp.kotlinjdsl.dsl.jpql.entity.employee.Employee | ||||||||||||||||
import com.linecorp.kotlinjdsl.dsl.jpql.queryPart | ||||||||||||||||
import com.linecorp.kotlinjdsl.querymodel.jpql.expression.Expression | ||||||||||||||||
import com.linecorp.kotlinjdsl.querymodel.jpql.expression.Expressions | ||||||||||||||||
import com.linecorp.kotlinjdsl.querymodel.jpql.path.Paths | ||||||||||||||||
import org.assertj.core.api.WithAssertions | ||||||||||||||||
import org.junit.jupiter.api.Test | ||||||||||||||||
|
||||||||||||||||
class PowerDslTest : WithAssertions { | ||||||||||||||||
private val baseExpression = Paths.path(Employee::age) | ||||||||||||||||
private val exponentExpression = Expressions.value(2) | ||||||||||||||||
private val exponentPrimitive = 2 | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be done to make the semantics of the variables clearer, but it would be nice to have a consistent style with other existing test code (e.g. ModDslTest.kt)
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consistency rather than semantics, I see :) |
||||||||||||||||
|
||||||||||||||||
@Test | ||||||||||||||||
fun `power() with a property`() { | ||||||||||||||||
// when | ||||||||||||||||
val expression1 = queryPart { | ||||||||||||||||
power(Employee::age, exponentPrimitive) | ||||||||||||||||
}.toExpression() | ||||||||||||||||
|
||||||||||||||||
val expression2 = queryPart { | ||||||||||||||||
power(Employee::age, exponentExpression) | ||||||||||||||||
}.toExpression() | ||||||||||||||||
|
||||||||||||||||
val actual1: Expression<Int> = expression1 // for type check | ||||||||||||||||
val actual2: Expression<Int> = expression2 // for type check | ||||||||||||||||
|
||||||||||||||||
// then | ||||||||||||||||
val expected1 = Expressions.power( | ||||||||||||||||
base = Paths.path(Employee::age), | ||||||||||||||||
exponent = Expressions.value(exponentPrimitive), | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
val expected2 = Expressions.power( | ||||||||||||||||
base = Paths.path(Employee::age), | ||||||||||||||||
exponent = exponentExpression, | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
assertThat(actual1).isEqualTo(expected1) | ||||||||||||||||
assertThat(actual2).isEqualTo(expected2) | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
@Test | ||||||||||||||||
fun `power() with a expression`() { | ||||||||||||||||
// when | ||||||||||||||||
val expression1 = queryPart { | ||||||||||||||||
power(baseExpression, exponentPrimitive) | ||||||||||||||||
}.toExpression() | ||||||||||||||||
|
||||||||||||||||
val expression2 = queryPart { | ||||||||||||||||
power(baseExpression, exponentExpression) | ||||||||||||||||
}.toExpression() | ||||||||||||||||
|
||||||||||||||||
val actual1: Expression<Int> = expression1 // for type check | ||||||||||||||||
val actual2: Expression<Int> = expression2 // for type check | ||||||||||||||||
|
||||||||||||||||
// then | ||||||||||||||||
val expected1 = Expressions.power( | ||||||||||||||||
base = baseExpression, | ||||||||||||||||
exponent = Expressions.value(exponentPrimitive), | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
val expected2 = Expressions.power( | ||||||||||||||||
base = baseExpression, | ||||||||||||||||
exponent = exponentExpression, | ||||||||||||||||
) | ||||||||||||||||
|
||||||||||||||||
assertThat(actual1).isEqualTo(expected1) | ||||||||||||||||
assertThat(actual2).isEqualTo(expected2) | ||||||||||||||||
} | ||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package com.linecorp.kotlinjdsl.querymodel.jpql.expression.impl | ||
|
||
import com.linecorp.kotlinjdsl.Internal | ||
import com.linecorp.kotlinjdsl.querymodel.jpql.expression.Expression | ||
|
||
/** | ||
* Expression that calculates the powering of a numeric [base] to a specified [exponent]. | ||
*/ | ||
@Internal | ||
data class JpqlPower<T : Number> internal constructor( | ||
val base: Expression<T>, | ||
val exponent: Expression<Int>, | ||
) : Expression<T> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,6 +105,7 @@ import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlPathPropertySeria | |
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlPathTreatSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlPathTypeSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlPlusSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlPowerSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlPredicateParenthesesSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlRoundSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.impl.JpqlSelectQuerySerializer | ||
|
@@ -370,6 +371,7 @@ private class DefaultModule : JpqlRenderModule { | |
JpqlPredicateParenthesesSerializer(), | ||
JpqlRoundSerializer(), | ||
JpqlSelectQuerySerializer(), | ||
JpqlPowerSerializer(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please move the position of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean alphabetical order! Got it :) |
||
JpqlSignSerializer(), | ||
JpqlSizeSerializer(), | ||
JpqlSortSerializer(), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package com.linecorp.kotlinjdsl.render.jpql.serializer.impl | ||
|
||
import com.linecorp.kotlinjdsl.Internal | ||
import com.linecorp.kotlinjdsl.querymodel.jpql.expression.impl.JpqlPower | ||
import com.linecorp.kotlinjdsl.render.RenderContext | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.JpqlRenderSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.serializer.JpqlSerializer | ||
import com.linecorp.kotlinjdsl.render.jpql.writer.JpqlWriter | ||
import kotlin.reflect.KClass | ||
|
||
@Internal | ||
class JpqlPowerSerializer : JpqlSerializer<JpqlPower<*>> { | ||
override fun handledType(): KClass<JpqlPower<*>> { | ||
return JpqlPower::class | ||
} | ||
|
||
override fun serialize(part: JpqlPower<*>, writer: JpqlWriter, context: RenderContext) { | ||
val delegate = context.getValue(JpqlRenderSerializer) | ||
|
||
writer.write("POWER") | ||
|
||
writer.writeParentheses { | ||
delegate.serialize(part.base, writer, context) | ||
|
||
writer.write(",") | ||
writer.write(" ") | ||
|
||
delegate.serialize(part.exponent, writer, context) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the
Book::verticalLength
property new?It would be better to use an existing property (e.g. Employee::age)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right, I thought about adding that field, but it's not currently there.
I know someone added an age field recently haha I'll replace it with that.