-
Notifications
You must be signed in to change notification settings - Fork 694
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
WIP: PostgreSQL DSL #1456
base: main
Are you sure you want to change the base?
WIP: PostgreSQL DSL #1456
Conversation
# Conflicts: # exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt # exposed-postgresql/src/main/kotlin/org/jetbrains/exposed/postgresql/sql/insert-DSL-impl.kt # exposed-postgresql/src/main/kotlin/org/jetbrains/exposed/postgresql/sql/returning.kt # exposed-postgresql/src/test/kotlin/org/jetbrains/exposed/postgresql/sql/BasePostgresTest.kt # exposed-postgresql/src/test/kotlin/org/jetbrains/exposed/postgresql/sql/PostgresqlInsertDSLTest.kt # exposed-postgresql/src/test/kotlin/org/jetbrains/exposed/postgresql/sql/test-table.kt
…ack from list to object
Hello @naftalmm , I did not found way to contact you on github so i will write it here. You seems active on this repo. If you could look at this PR and leave some thoughts would be appreciated. I don`t mean code review for not I will tune this MR some more, but more like about idea - DB specific DSL. Thx, |
Hello! I've also thought about DB-specific DSLs recently. Let me share my opinion about suggested ideas since you're explicitly asking for it) Dialect-specific submodules The whole idea of ORM is to create a unified abstraction, independent of specific SQL dialects. This is achieved by the following design cornerstones:
Some non-standard SQL-features are not exclusive for only one DBMS, so this approach allows to not reimplement them in each dialect submodule supporting it, but write code like this:
DB-specific DSLs However, this "common API" approach has its downsides:
Probably it could be mitigated with some additional DSL, defining dialect-specific context, which extends basic "common API": transaction(forDB = /*...*/) {
//...
} But I believe the "common API" approach is enough for
Actually,
For DBMS not supporting the Suggested DSL is rather complicated, looking almost like plain SQL. fun <T : Table> T.insert(body: T.(InsertReturningStatement) -> FieldSet): List<ResultRow> which will be chosen if the last expression of lambda passed into Table.insert {
it[column] = value
it.returning(...) //or it.returningAll(), both methods should to be defined in InsertReturningStatement class and return `FieldSet`
} Not sure if it's possible to create such an overload in Kotlin currently. Alternatively, it could be: fun <T : Table> T.insert(returning: List<Expression<*>>, body: T.(InsertReturningStatement) -> Unit): List<ResultRow>
But its usage requires boiler-plate Table.insert(returning = listOf(...)) {
it[column] = value
} Another alternative: fun <T : Table> T.insert(returning: ReturningDSL.() -> List<Expression<*>>, body: T.(InsertReturningStatement) -> Unit): List<ResultRow>
object ReturningDSL {
fun returning(column: Expression<*>, vararg columns: Expression<*>) = listOf(column) + columns
fun returningAll() = emptyList<Expression<*>>() // special value, needs to be treated specifically
} It forces user to explicitly type Table.insert({ returning(...) }) {
it[column] = value
} |
Thanks for answer, Till today I did not consider Exposed as ORM framework (now i see it is in readme :) ). For me this lib was Dynamic SQL written in Kotlin with lambda DSL. I was looking for one written in Kotlin for some time. But yes, Exposed claims itself as ORM. But imho in the end it depend what parts of Exposed are used in project. I usually go for DSL only, because I don't like DAO module approach. For me is enought Dynamic SQL which allow easier mapping of data to Kotlin classes via Even if we will consider Exposed as ORM framework only, unified approach for all databases is nice to have, but not necessary. ORM is mapping between Database results and class instance in Kotlin I does not denote how DB API should look. And this is where problem lies. As you mention Exposed address DB specific features with common API, which is imho misleading, because some databases may not support that feature and exception may occur. Like using Point 2 and 3, Yes I have read and little modified how SQL is rendered for my needs. Basically I did create interface with callbacks, which may or may not be invoked for SQL Dialect. It allows to implement any DB specific features. You can see it for example SQLRenderCallbacks, specific SQL render then in dialect those callbacks are invoked. I did not find other solution for now, because rendering parameters has to be same for all dialects. Maybe checkout code and see changes as whole. Next you are mentioning specifying DB specific DSL via Function naming for sure is open for discussion and further changes no problem.
in this order I would write SQL itself.
Works same, but it look less like produced SQL to me. Mainly statement order. Returning before values are set. Yes my example does not force any order at all it is just my flow. What I want to know and don't see here is opinion whether this changes makes sense and would be welcome or this lib/framework will stick only to common DSL. |
Actually, for PostgreSQL (and, probably other DBMS not supporting limit in update) it could be emulated with an additional subquery (see https://stackoverflow.com/questions/11432155/limiting-postgresql-update-command). The obvious advantage of common API for users is the ease of DBMS migration - "write once, run everywhere". DB-specific DSLs as a set of extensions methods for common API could be a compromise between portability and feature-fullness - "write once, rewrite only DB-specific parts, run everywhere".
Kotlin allows to define extentions not only globally, but also in the context of another class/interface. To keep object PostgresExtraDSL {
fun <T : Table> T.somePostgresSpecificStaff() {
// ...
}
} So this will be compiled: transaction {
with(PostgresExtraDSL) {
MyTable.somePostgresSpecificStaff()
}
} while this not: transaction {
MyTable.somePostgresSpecificStaff()
} Later, when KEEP-259 will be implemented, additional functions could be defined: fun <T> postgresTransaction(db: Database? = null, statement: context(PostgresExtraDSL) Transaction.() -> T): T =
with(PostgresExtraDSL) { transaction(db, statement) } for even better API: postgresTransaction {
MyTable.somePostgresSpecificStaff()
}
There is an experimental @OverloadResolutionByLambdaReturnType annotation for this, but it will result in dozens of "Candidate was chosen only by @OverloadResolutionByLambdaReturnType annotation" warnings in users code, so it's not perfectly backward-compatible change. Probably, when this feature will be stabilized, these warnings won't be issued by the compiler as well.
API, strictly following SQL syntax, could be more natural for users with respectful background, but the resulting code is not as concise and readable, as it could be. Table.insert({ returning(...) }) {
it[column] = value
} is not convenient, returning before values are set. Ideally, it should be methods chaining: Table
.insert{
it[column] = value
}
.returning(...) But this API will make So, maybe, arguments order swap will be enought? Table.insert({
it[column] = value
}) {
returning(...)
}
In my opinion, it's better to implement as many features as possible via common DSL (emulating absent ones in specific dialects by their means). Dialect-specific extra DSL (with aforementioned syntax) makes sense too, but only for non-emulable features, also allowing to gradually implement emulation, moving originally dialect-specific features into common API when all emulations are done. @Tapac, what do you think? |
I don't follow. My intention was DSL for query/update data not for DB DDL meaning migrations (in my case we don't use Exposed for migrations at all) it is used strictly for data queries/update/delete and migrations are done before app even start by init container by manually written scripts with liquibase/flyway. Also DB specific DSL is intended for projects, which has set database type strictly (in our case PostgreSQL). Otherwise go for common SQL. About
seems nice. What need more work how to propagate it to DSL. Write
This is not good idea how to propagate I would rather go for database objects bound DSL. Meaning About DSL API I like most
it does not have to be breaking change if it is make like version 2 or so. Also this allow to use context extensions more fluent. |
Yes, data migration is a completely separate task. I mean that apart from data migration, the SQL code to work with new DBMS should be tweaked too (and ORM seamlessly does it for you, unless you're using DB-specific DSL).
Business decisions may change - architectural decisions should expect this to minimize possible code modifications.
You may declare queries separately from their calls in transactions.
It still would be a breaking change since it would require users to somehow migrate their code to keep its semantics. |
Hey, as we never got any response on topic: how Exposed will behave in future regarding DB specific dialects (either won`t support them -> use only comm SQL or will somehow). Then this PR is sleeping. |
PostgreSQL DSL
Still Work in progress: more tests, docs in README, kotlin docs etc, test
insert with ON CONFLICT DO UPDATE ...
.Rather big Pull request I know, but changes in core were needed like a lot of them. If needed I can split it to smaller ones.
I would like to introduce into framework also DB specific DSL not only common one. Rough idea: Is to add gradle modules for each supported database with DB specific DSL. So developers can import DSL for DB they use. DB specific DSL will be in gradle module with its own package in postgresql case it is module
exposed-postgresql
and package ispostgresql
. Insert, update and delete statements got reworked a bit and support postgresql features likeRETURNING
andON CONFLICT
. Common DSL would be still available in another module so developer can choose Exposed flavour needed for his project. But would not be part of core so won't be available by default. Selects are just reimported from core.Insert examples
Update examples
Delete examples
Open for discussion. Changes in DSL or so.
For testing I went for test-container. Usually when I run tests on Exposed project at least one docker with testing DB is running even after tests are finished. Usually change setting for my locally running docker. So in this module testing is kinda reworked.
V