Add String functions(trim) #525
Conversation
waahhh
requested review from
pickmoment,
huisam,
cj848 and
shouwn
as code owners
|
먼저 도움을 주셔서 감사합니다. ❤️
위 질문에 대해서 작은 따옴표(')를 한번 테스트해봐주실 수 있으신가요? charLiteral에서 걸리는 부분은 작은 따옴표 외에는 없을 것 같아요. 추가로 Like도 동일한 이슈가 발생할 수 있겠네요. 만약 작은 따옴표(')로 했을 때도 정상 동작한다면 Like 쪽도 한번 테스트해봐주실 수 있으신가요? 그리고 테스트가 통과하면 Like 쪽의 Char도 charLiteral로 수정을 부탁드릴게요. |
|
그런데 input 파라미터도 가능하다고 최신 버전의 JPA 명세에 적혀 있는데, hibernate 쪽에 버그 같기도 하네요. 😢 https://jakarta.ee/specifications/persistence/3.1/jakarta-persistence-spec-3.1.html#like-expressions
|
|
추가로 trim을 JPA 포맷과 비슷하게 맞춰서 개발해주실 수 있으신가요? 저는 아래의 형태를 제공할 생각이었습니다. trim(string)
trim(char).from(string)
trimLeading(char).from(string)
trimTrailing(char).from(string)
trimBoth(char).from(string)
trim().from(string)
trimLeading().from(string)
trimTrailing().from(string)
trimBoth().from(string)이 부분은 case when을 보시면 참고하여 구현이 가능할 것으로 보이는데요. 혹시 너무 부담되시면 말씀주세요. 원래 제가 개발할 계획을 가지고 있기 때문에 작성해주신 코드 참고하여 개발하겠습니다. |
Trim Test
Spring Data Jpa - 사용하는 구현체 종류, 버전에따라 위와 동일하게 문제 발생@shouwn 우선 named parameter[Expressions.value()]를 사용하는 경우 발생하는 문제는 HHH-17435 |
Like Test
Like의 경우 위 환경에서 모두 정상 작동합니다. |
작성해주신 테스트 결과 및 JPA 스펙에 그러면 hibernate 쪽 버그로 보고 Kotlin JDSL 구현은 char input parameter인 테스트 해주셔서 정말 감사합니다. ❤️ |
waahhh
force-pushed
the
feature/add-string-functions-trim
branch
from
a55c5fa to
b3b5152
Compare
@shouwn |
waahhh
force-pushed
the
feature/add-string-functions-trim
branch
from
b3b5152 to
048e5eb
Compare
| fun trim(character: Char? = null): TrimFromStep { | ||
| return TrimDsl( | ||
| null, | ||
| character?.let { Expressions.value(it) }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Expressionable<Char>를 파라미터로 받는 메소드의 구현도 부탁드릴게요. 🙏
There was a problem hiding this comment.
Expressionable<Char>를 파라미터로 받는 메소드의 구현도 부탁드릴게요. 🙏
fun trim(character: Expressionable<Char>? = null): TrimFromStep {...}nullable 형태의 파라미터를 받는 메서드를 추가할 경우
Overload resolution ambiguity. All these functions match. 컴파일 오류가 발생하는데,
fun trim(character: Expressionable<Char>): TrimFromStep {...}non-nullable 형태의 파라미터를 받는 메서드 구현 추가를 말씀하시는게 맞을까요?
non-nullable 형태의 추가가 맞을 경우, trim(..) 메서드에 대해서만 추가하면 될까요?
아니면 trimLeading(..), trimTrailing(..), trimBoth(..) 모두 추가를 원하시는건가요?
저는 Kotlin JDSL은 Custom DSL을 지원하므로, 가장 흔하게 사용하는 파라미터 형태의 메서드만 기본 제공하고, Expressionable<Char> 형태의 파라미터가 필요시 사용자가 Custom DSL을 사용하면 되지 않을까 생각했습니다.
There was a problem hiding this comment.
필요시 사용자가 Custom DSL을 사용하면 되지 않을까 생각했습니다.
네 Kotlin JDSL이 Custom DSL에 오픈적이기 때문에 모든 사용자의 요구사항을 들어줄 필요는 없다고 생각합니다. Expressionable<Char>를 요청 드린 이유는 사용자에게 많은 API를 제공한다기 보다 일관된 API를 제공하기 위함입니다.
다른 메소드들을 확인해보면 모두 기본 자료형 및 Expressionable 2개의 파라미터를 받을 수 있게 되어 있습니다. 그러면 다른 메소드를 사용한 사용자라면 trim() 메소드도 당연히 동일한 사용 방법을 제공해줄 것이라 생각할 것입니다.
만약 이걸 제공해주지 못 하면 다른 메소드를 사용하던 사용자의 추측이 실패할 경우가 발생하고, 이는 사용자가 Kotlin JDSL이 제공하는 인터페이스를 유추할 때 겁나게 만들어 사용에 하나씩 API 마다 찾아봐야 하는 스트레스를 만들 것으로 생각합니다. 그래서 사용자가 유추 가능한 인터페이스를 유지하기 위해 다른 메소드가 제공한 시그니처도 동일하게 trim에서 제공하고자 했습니다.
There was a problem hiding this comment.
nullable 형태의 파라미터를 생각했습니다. 말씀해주신 것처럼 기본 자료형 및 Expressionable 모두 nullable이기 때문에 일반적으로 모호하다는 에러가 발생합니다.
이는 @LowPriorityInOverloadResolution를 통해 해소가 가능하며, 기본 자료형에 LowPriorityInOverloadResolution을 붙여주시면 될 것으로 보입니다!
| @SinceJdsl("3.1.0") | ||
| fun from(value: Expressionable<String>): Expression<String> | ||
|
|
||
| @SinceJdsl("3.1.0") | ||
| fun from(value: String): Expression<String> |
There was a problem hiding this comment.
메소드의 순서를 Expressionable이 아래로 오게 수정 부탁드려요!
@SinceJdsl("3.1.0")
fun from(value: String): Expression<String>
@SinceJdsl("3.1.0")
fun from(value: Expressionable<String>): Expression<String>| import com.linecorp.kotlinjdsl.querymodel.jpql.expression.Expression | ||
| import com.linecorp.kotlinjdsl.querymodel.jpql.expression.Expressionable | ||
| import com.linecorp.kotlinjdsl.querymodel.jpql.expression.Expressions | ||
| import com.linecorp.kotlinjdsl.querymodel.jpql.expression.impl.JpqlTrim |
There was a problem hiding this comment.
querymodel은 DSL에 있어서 외부 라이브러리라고 생각해주세요.
querymodel의 public API가 아닌 impl 패키지 안의 internal API를 사용하면 안 됩니다. 따라서 JpqlTrim을 직접 참조할 수 없게 됩니다.
이 경우에는 Trim을 구분할 때 Enum class의 참조 방식이 아닌 JpqlCaseValue, JpqlCaseWhen과 같이 JpqlTrimLeading, JpqlTrimTrailing, JpqlTrimBoth 클래스들을 만들고 Factory 메소드로 각각 만드는 방식으로 작업 부탁드려요. 🙏
도와주시는 것인데 계속 리뷰를 드려 죄송하네요. 😢
There was a problem hiding this comment.
Enum class를 만들면 코드의 중복을 줄일 수 있지만,
외부에 Enum class를 노출해야 하기 때문에 Expression의 추상화 유지보수 비용이 발생해 이런 방식을 선택하게 되었습니다.
There was a problem hiding this comment.
DSL의 구현 방식은 TrimFromStep 인터페이스는 1개로 유지하고, TrimDsl의 구현을 3개를 만드는 방식이 되어, 사용자에 노출되는 인터페이스 클래스는 TrimFromStep만 됩니다.
There was a problem hiding this comment.
피드백 감사드립니다. 👍
저에겐 익숙하지 않은 구조이지만, 다양한 설계 관점에서 한번 더 생각해 볼 수 있어서 좋습니다.
편하게 피드백 해주세요.😍
fun trim(value: String): Expression<String> {
return Expressions.trim(value = Expressions.value(value))
}
fun trim(value: Expressionable<String>): Expression<String> {
return Expressions.trim(value = value.toExpression())
}trim(string) 형태에 대한 메서드를 만들때, trim().from() 구현할때 처럼 TrimDsl, TrimBuilder를 구현하는게 아닌 위 코드처럼 팩토리 메서드를 바로 호출해도 되나요? 아니면 저 경우에도 별도의 추가 Dsl, Builder를 만들어야하나요? |
interface TrimFromStep {
@SinceJdsl("3.1.0")
fun from(value: String): Expression<String>
@SinceJdsl("3.1.0")
fun from(value: Expressionable<String>): Expression<String>
}에서 from의 반환 타입은 |
바로 팩토리 메소드를 호출해주셔도 됩니다! Dsl 및 Builder 클래스는 DSL를 개발하는 데 있어 편의를 위해 만든 클래스이기 때문에 구현상 필요가 없으면 안 만드셔도 괜찮습니다! |
앗 이 부분을 제가 예전에 개발하면서 잊고 있었네요. 찾아주셔서 감사합니다. ㅎㅎ Interface인 Step을 통한 생성에는
Step의 경우 이후 다른 Step이 추가될 수도 있기 때문에 확장을 위해 마지막 결과물에도 |
waahhh
force-pushed
the
feature/add-string-functions-trim
branch
from
048e5eb to
787893b
Compare
앗 머지를 해주셨네요. 수정하신 코드를 보니 제가 신경쓰지 못한 부분이 많았네요. 🙏 |
function 버그 수정을 위해 빠르게 배포하게 되었습니다. 😢
다른 메소드도 도와주신다니 정말 감사드립니다! 원래 3.1.1을 생각하고 있었지만 develop 브랜치를 가져가게 되면서 3.2.0가 더 어울리겠네요. 넵 3.2.0로 부탁드립니다! |
Motivation
Modifications
Add JpqlTrim, JpqlTrimSerializer, dsl for trim function with tests
현재 아래와 같이 trim을 구현하였는데, character 파라미터를 Expression으로 변환시
기존처럼 Expressions.value(it) 사용이 아닌 Expressions.charLiteral(it)을 사용하여 구현했습니다.
기존처럼 Expressions.value(it)를 사용할 경우,
select( trim(path(Book::title), character = 'B'), ).from( entity(Book::class), )로 생성되는 JPQL은
SELECT TRIM(:param1 FROM Book.title) FROM Book AS Book형태로 trim character 부분이 파라미터 바인딩 형태인데, 이 경우 hibernate 6.x 이상 혹은 이를 사용하는 Spring Data JPA(jakarta) 환경에서는 SemanticException이 발생하는 문제가 있습니다.동일 jpql을 eclipselink, hibernate 5.6.x 이하 혹은 이를 사용하는 Spring Data JPA(javax) 환경에서 수행할 경우 정상 동작합니다.반면에 Expressions.charLiteral(it)를 사용하여 생성되는
SELECT TRIM('B' FROM Book.title) FROM Book AS Book형태의 JPQL은모든 환경에서 정상 동작합니다.
따라서 JPA 구현체, 버전별 호환성을 위해 Expressions.charLiteral(it)를 사용하였는데,
이렇게 charLiteral() 을 사용할 경우 문제될만한 부분이 있을까요?
Commit Convention Rule
commit typeplease describe it on the Pull RequestResult
Closes