+
Skip to content

Add case when support #413

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

Merged
merged 6 commits into from
Sep 21, 2022
Merged

Add case when support #413

merged 6 commits into from
Sep 21, 2022

Conversation

zuisong
Copy link
Contributor

@zuisong zuisong commented Jul 14, 2022

No description provided.

@zuisong zuisong force-pushed the case-when branch 2 times, most recently from bda5ac8 to 5d13149 Compare July 14, 2022 11:36
@vincentlauvlwj vincentlauvlwj marked this pull request as ready for review July 15, 2022 10:02
@vincentlauvlwj
Copy link
Member

  • Ktorm 的 SQL 语法支持都会配合相应的 DSL 写法,没有让用户手动创建 expression 的道理
  • SQL case when 不仅支持 case when condition1 then statement1 when condition2 then statement2 end 的写法,还支持在 case 关键字后跟一个变量,比如 case var when value1 then statement1 when value2 then statement2 end

综上,这个功能实现不完整,暂不考虑合并

@zuisong
Copy link
Contributor Author

zuisong commented Jul 15, 2022

针对第二点, 我在最新的代码里已经提供支持了

第一点是是准备周末的时候再写工具方法让用户方便的创建的, 现在也已经更新上来了 , 可以帮忙再次看一下吗?

文档还没更新。觉得可以合并的时候, 我一起把文档也更新上, 还有一些方法名和参数名使用了kotlin的关键字, 也有待商量怎么处理, 可以讨论出一个合理的方案, 我一起改进

@zuisong
Copy link
Contributor Author

zuisong commented Jul 15, 2022

可能需要重新打开这个MR才能看到最新的代码 😄

@vincentlauvlwj
Copy link
Member

不好意思,你继续写吧

@vincentlauvlwj
Copy link
Member

推荐一种 DSL 的写法:

val query = database
    .from(Employees)
    .select((CASE(Employees.name) WHEN "vince" THEN 1 WHEN "mary" THEN 2 ELSE 3).aliased("c"))
    .where { xxx }

不带参数:

val query = database
    .from(Employees)
    .select((CASE() WHEN (Employees.name eq "vince") THEN 1 WHEN (Employees.name eq "mary") THEN 2 ELSE 3).aliased("c"))
    .where { xxx }

CASE, WHEN, THEN, ELSE 这些函数统一用大写命名,避免与 Kotlin 的关键字冲突

@zuisong
Copy link
Contributor Author

zuisong commented Jul 15, 2022

想了一下,这种中缀表达式的dsl对换行不太友好,可能要提供多种dsl的写法,这种写法很直观,可以作为其中一种写法

推荐一种 DSL 的写法:

val query = database

    .from(Employees)

    .select((CASE(Employees.name) WHEN "vince" THEN 1 WHEN "mary" THEN 2 ELSE 3).aliased("c"))

    .where { xxx }

不带参数:

val query = database

    .from(Employees)

    .select((CASE() WHEN (Employees.name eq "vince") THEN 1 WHEN (Employees.name eq "mary") THEN 2 ELSE 3).aliased("c"))

    .where { xxx }

CASE, WHEN, THEN, ELSE 这些函数统一用大写命名,避免与 Kotlin 的关键字冲突

@vincentlauvlwj
Copy link
Member

中缀函数并没有禁止用户使用传统方式调用,不存在对换行不友好的问题,写成链式就好了:

val expr = CASE(Employees.name)
    .WHEN("vince").THEN(1)
    .WHEN("mary").THEN(2)
    .ELSE(3)

不带参数:

val expr = CASE()
    .WHEN(Employees.name eq "vince").THEN(1)
    .WHEN(Employees.name eq "mary").THEN(2)
    .ELSE(3)

@zuisong
Copy link
Contributor Author

zuisong commented Jul 15, 2022

中缀函数并没有禁止用户使用传统方式调用,不存在对换行不友好的问题,写成链式就好了:

val expr = CASE(Employees.name)
    .WHEN("vince").THEN(1)
    .WHEN("mary").THEN(2)
    .ELSE(3)

不带参数:

val expr = CASE()
    .WHEN(Employees.name eq "vince").THEN(1)
    .WHEN(Employees.name eq "mary").THEN(2)
    .ELSE(3)

已经实现了第一版, 还是有一点瑕疵, 可以看一下代码里的评论

Copy link
Member

@vincentlauvlwj vincentlauvlwj left a comment

Choose a reason for hiding this comment

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

大概浏览了你的代码,有三个比较大的问题需要修改:

  • DSL 的写法保留一种就好了,太多的写法会引发混乱,建议把你之前的遗留代码删了
  • SqlExpressionVisitor.visitCaseWhen 不要留空
  • 所有 public 的类和函数都应该添加文档注释,你可以在你本地运行 ./gradlew clean build 试试,不符合规范的无法成功构建

@zuisong
Copy link
Contributor Author

zuisong commented Jul 16, 2022

大概浏览了你的代码,有三个比较大的问题需要修改:

  • DSL 的写法保留一种就好了,太多的写法会引发混乱,建议把你之前的遗留代码删了
  • SqlExpressionVisitor.visitCaseWhen 不要留空
  • 所有 public 的类和函数都应该添加文档注释,你可以在你本地运行 ./gradlew clean build 试试,不符合规范的无法成功构建

好的, dsl 我更新了一下。现在第一个then 依然要传 expression , 不能传变量进来, 暂时没有想到好办法解决这个问题

这对上面这些问题, 我今天晚些时候会修复

@zuisong zuisong force-pushed the case-when branch 2 times, most recently from 1f30cec to 3685339 Compare July 17, 2022 03:37
@zuisong
Copy link
Contributor Author

zuisong commented Jul 17, 2022

问题都改好了, 麻烦再看一下呢, 有什么问题我继续改进一下 😄

Copy link
Member

@vincentlauvlwj vincentlauvlwj left a comment

Choose a reason for hiding this comment

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

修改意见参见代码评论,另外请修改 ktorm.maven-publish.gradle.kts 加入你的 ID

internal val caseValue: ScalarExpression<T>?,
internal val condition: ScalarExpression<T>,
internal val caseWhenExpression: CaseWhenExpression<T, V>? = null,
)
Copy link
Member

Choose a reason for hiding this comment

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

不需要定义这两个类,直接使用 CaseWhenExpression 即可

* @property caseExpr case statements, optional, may be null.
* @property sqlType the argument's [SqlType].
*/
public data class CaseWhenExpression<V : Any, T : Any> internal constructor(
Copy link
Member

Choose a reason for hiding this comment

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

泛型参数只需要一个 T,表示整个表达式的返回类型,V 可以去掉,用到 V 的地方可以改成 ScalarExpression<*>

Copy link
Member

Choose a reason for hiding this comment

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

另外,作为对外开放的表达式类,构造函数不宜是 internal

val caseExpr: ScalarExpression<V>? = null,
val whenThenConditions: List<Pair<ScalarExpression<V>, ScalarExpression<T>>>,
val elseExpr: ScalarExpression<T>? = null,
internal val whenSqlType: SqlType<V>,
Copy link
Member

Choose a reason for hiding this comment

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

whenSqlType 可以去掉

@zuisong zuisong requested a review from vincentlauvlwj July 21, 2022 02:13
@vincentlauvlwj vincentlauvlwj changed the base branch from master to v3.6.x September 21, 2022 12:39
@vincentlauvlwj vincentlauvlwj merged commit 6faa83a into kotlin-orm:v3.6.x Sep 21, 2022
@vincentlauvlwj
Copy link
Member

已合并,3.6.0 发布

@zuisong zuisong deleted the case-when branch August 21, 2023 07:33
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
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载