-
Notifications
You must be signed in to change notification settings - Fork 695
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
feat: EXPOSED-77 Support entity class for table with composite primary key #1987
Conversation
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.
please check the comparison and let's discuss merging
exposed-core/src/main/kotlin/org/jetbrains/exposed/dao/id/CompositeID.kt
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/dao/id/CompositeID.kt
Outdated
Show resolved
Hide resolved
...rc/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/CompositeIdTableEntityTest.kt
Show resolved
Hide resolved
a0630fd
to
54b8d89
Compare
54b8d89
to
da0d18e
Compare
@Suppress("UNCHECKED_CAST") | ||
@JvmName("setWithNullableEntityIdValue") | ||
operator fun <T : Comparable<T>, ID : EntityID<T>> set(column: Column<ID?>, value: T?) { | ||
require(column.columnType.nullable || value != null) { | ||
"Trying to set null to not nullable column $column" | ||
} | ||
values[column] = value?.let { EntityID(value, column.table as IdTable<T>) } | ||
} |
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.
It seems odd that a key column may be nullable, but this functionality exists for EntityID
columns in other IdTable
s, so I included it.
fun <T> Expression<T>.isNotNull(): IsNotNullOp = IsNotNullOp(this) | ||
fun <T> Expression<T>.isNotNull() = if (this is Column<*> && columnType.isEntityIdentifier()) { | ||
(table as IdTable<*>).mapIdOperator(::IsNotNullOp) | ||
} else { | ||
IsNotNullOp(this) | ||
} |
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.
Both isNull()
and isNotNull()
are altered because they are potentially called by a branch in the altered eq()
/neq()
. I didn't think that EntityID
columns would ever be nullable, but setters exist for this case (as seen above), so I included this change.
exposed-dao/src/main/kotlin/org/jetbrains/exposed/dao/EntityCache.kt
Outdated
Show resolved
Hide resolved
// access individual composite values - requires unwrapping - no type erasure | ||
val publisherIdComponent1Value: Int = publisherIdValue[Publishers.pubId].value | ||
val publisherIdComponent2Value: UUID = publisherIdValue[Publishers.isbn].value | ||
val bookIdComponent1Value: Int = bookIdValue[Books.bookId].value | ||
val reviewIdComponent1Value: String = reviewIdValue[Reviews.content].value | ||
val reviewIdComponent2Value: Long = reviewIdValue[Reviews.rank].value |
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.
Something to note about using entityId()
is that, because the existing behavior hasn't been changed, each composite PK column will be of type Column<EntityID<T>>
. So while an IntIdTable
only has one column with this type, its id
column, a CompositeIdTable
will have the id
column (representative of all component references), as well as X amount more for X components.
Like with IntIdTable
, I've made it so that manually defining a new ID doesn't require wrapping with EntityID
, but accessing the original value will still require chaining .value
.
So with an IntIdTable
we could access the individual value like -> id.value
With a CompositeIdTable
this means doing -> id.value[Table.column].value
for each individual value
object Reviews : CompositeIdTable("reviews") { | ||
val content = varchar("code", 8).entityId() | ||
val rank = long("rank").entityId() | ||
|
||
// FK constraint with single column can be created as a column-level constraint | ||
val book = reference("book_id", Books.bookId) | ||
|
||
override val primaryKey = PrimaryKey(content, rank) | ||
} |
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.
A point to note:
Using the following tables are synonymous:
object Tester1 : IntIdTable() {
// more columns
}
object Tester2: IdTable<Int>() {
override val id = integer(columnName).autoIncrement().entityId()
// more columns
override val primaryKey = PrimaryKey(id)
}
Trying to do something similar will not work to replace CompositeIdTable
, mostly because of how the id
column is defined but also because of internal checks. Should including this behavior be good, do you think?
object Tester3 : IdTable<CompositeID>() {
override val id = //...
// pk columns
override val primaryKey = PrimaryKey(...)
}
exposed-core/src/main/kotlin/org/jetbrains/exposed/dao/id/EntityID.kt
Outdated
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/dao/id/IdTable.kt
Outdated
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/dao/id/IdTable.kt
Outdated
Show resolved
Hide resolved
389a651
to
3497e79
Compare
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt
Outdated
Show resolved
Hide resolved
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.
Sorry for the delay, It was huge :) Good job on this, LGTM! Could you also log an issue for docs?
fb788a8
to
5482c17
Compare
@obabichevjb @e5l Changes made in latest commit based on above comments:
|
…y key - Add base objects for new CompositeEntity and CompositeIdTable. - Use existing implementation via EntityID to wrap new comparable type, CompositeID. - Retain use of IdTable.id via a placeholder column that can deconstruct to its component columns in DSL and ResultRow accessors. - Add base unit tests.
…y key - Extract logic between regular id columns and composite id columns to lower locations. - Add KDocs and change some object locations. - Flesh out unit tests
…y key - Remove changes to reference logic. - Small change to unit test. - Rebase from main.
…y key - Update KDocs to reflect merged changes - Add CompositeID equality comparison for EntityID<*>
…y key Clean up some logic Add first example of supporting a simple referencedOn relation to a composite primary key.
…y key Rebase from main Remove composeEntityId() and replace with entityId(), so all composite column components are themselves wrapped EntityIDs.
…y key Fix detekt issue
…y key Return accidentally deleted init block.
…y key Add remaining reference functions with more tests for samples. Include case when CompositeIdTable with single key column is references.
…y key Fix detekt issues. Change visibility modifier of isEntityIdentifier function
…y key - Remove isNotInitialized() - Add validation that only single entityId() is used with non-CompositeIdTables - It is no longer possible to make an empty CompositeID { } - mapIdComparison() and mapIdOperator() logic is split into respective classes - Refactor Reference/OptionalReference logic to reduce duplication - Remove composite logic from general batchInsert() in flushInserts() and resolve type mismatch (with composite PK references) in Entity setter function - Remove type tests and MariaDB exclusion
…y key - Rebase from main - Remove unused property suppressor in test file
5482c17
to
6f5006a
Compare
Getting an error with Aliases columns when using the isNull expression following this commit. val build = BuildTable.alias("build")
val compare = BuildTable.alias("compare")
build.join(
compare,
JoinType.LEFT,
additionalConstraint = {
(build[BuildTable.sourceRepositoryId] eq compare[BuildTable.sourceRepositoryId]) and
(build[BuildTable.lastStateChange] less compare[BuildTable.lastStateChange])
}
)
.join(PackTable, JoinType.INNER, onColumn = build[BuildTable.id], otherColumn = PackTable.build)
.slice(build[BuildTable.sourceRepositoryId], PackTable.packUrl, PackTable.packHash)
.selectAll().where { compare[BuildTable.id].isNull() } // here
.associate { SourceRepositoryId(it[build[BuildTable.sourceRepositoryId]]) to it.toPack() } 01:14:25.562 [pool-2-thread-1] ERROR gg.netherite.service.api.server.inject.DefaultGrpcCustomizer - Closing call with status: Status{code=UNKNOWN, description=null, cause=java.lang.ClassCastException: class org.jetbrains.exposed.sql.Alias cannot be cast to class org.jetbrains.exposed.dao.id.IdTable (org.jetbrains.exposed.sql.Alias and org.jetbrains.exposed.dao.id.IdTable are in unnamed module of loader 'app')
at org.jetbrains.exposed.sql.ISqlExpressionBuilder$DefaultImpls.isNull(SQLExpressionBuilder.kt:507)
at org.jetbrains.exposed.sql.SqlExpressionBuilder.isNull(SQLExpressionBuilder.kt:1067)
at gg.netherite.service.resourcepack.packs.SQLPackRepository.getNewestPacks$lambda$9$lambda$7(SQLPackRepository.kt:78)
at org.jetbrains.exposed.sql.Query.where(Query.kt:259)
at gg.netherite.service.resourcepack.packs.SQLPackRepository.getNewestPacks$lambda$9(SQLPackRepository.kt:78)
at gg.netherite.service.api.server.database.DatabaseProvider$transaction$2.invokeSuspend$lambda$0(DatabaseProvider.kt:43)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.inTopLevelTransaction$run(ThreadLocalTransactionManager.kt:324)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.access$inTopLevelTransaction$run(ThreadLocalTransactionManager.kt:1)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt$inTopLevelTransaction$1.invoke(ThreadLocalTransactionManager.kt:371)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.keepAndRestoreTransactionRefAfterRun(ThreadLocalTransactionManager.kt:379)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.inTopLevelTransaction(ThreadLocalTransactionManager.kt:370)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt$transaction$1.invoke(ThreadLocalTransactionManager.kt:279)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.keepAndRestoreTransactionRefAfterRun(ThreadLocalTransactionManager.kt:379)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.transaction(ThreadLocalTransactionManager.kt:249)
at org.jetbrains.exposed.sql.transactions.ThreadLocalTransactionManagerKt.transaction$default(ThreadLocalTransactionManager.kt:244)
at gg.netherite.service.api.server.database.DatabaseProvider$transaction$2.invokeSuspend(DatabaseProvider.kt:35)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:111)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:99)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:811)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:715)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:702)
} |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [org.jetbrains.exposed:exposed-jdbc](https://github.com/JetBrains/Exposed) | `0.52.0` -> `0.53.0` | [![age](https://developer.mend.io/api/mc/badges/age/maven/org.jetbrains.exposed:exposed-jdbc/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/org.jetbrains.exposed:exposed-jdbc/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/org.jetbrains.exposed:exposed-jdbc/0.52.0/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/org.jetbrains.exposed:exposed-jdbc/0.52.0/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [org.jetbrains.exposed:exposed-core](https://github.com/JetBrains/Exposed) | `0.52.0` -> `0.53.0` | [![age](https://developer.mend.io/api/mc/badges/age/maven/org.jetbrains.exposed:exposed-core/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/maven/org.jetbrains.exposed:exposed-core/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/maven/org.jetbrains.exposed:exposed-core/0.52.0/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/org.jetbrains.exposed:exposed-core/0.52.0/0.53.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>JetBrains/Exposed (org.jetbrains.exposed:exposed-jdbc)</summary> ### [`v0.53.0`](https://github.com/JetBrains/Exposed/blob/HEAD/CHANGELOG.md#0530) [Compare Source](https://github.com/JetBrains/Exposed/compare/0.52.0...0.53.0) Infrastructure: - SQLite driver 3.46.0.1 - Spring Framework 6.1.11 - Spring Boot 3.3.2 - junit-bom 5.10.3 Features: - feat: Add time extension function for temporal expressions in Kotlin and Java by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2121](https://github.com/JetBrains/Exposed/pull/2121) - feat: EXPOSED-435 Allow insertReturning() to set isIgnore = true by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2148](https://github.com/JetBrains/Exposed/pull/2148) - feat: EXPOSED-77 Support entity class for table with composite primary key by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/1987](https://github.com/JetBrains/Exposed/pull/1987) - feat: EXPOSED-446 Support N-column inList equality comparisons by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2157](https://github.com/JetBrains/Exposed/pull/2157) - feat: EXPOSED-450 Merge command: PostgreSQL improvements by [@​obabichevjb](https://github.com/obabichevjb) in [https://github.com/JetBrains/Exposed/pull/2161](https://github.com/JetBrains/Exposed/pull/2161) - feat: EXPOSED-388 Support for column type converters by [@​obabichevjb](https://github.com/obabichevjb) in [https://github.com/JetBrains/Exposed/pull/2143](https://github.com/JetBrains/Exposed/pull/2143) - Adding comment text for a query SQL by [@​xJoeWoo](https://github.com/xJoeWoo) in [https://github.com/JetBrains/Exposed/pull/2088](https://github.com/JetBrains/Exposed/pull/2088) - feat: EXPOSED-459 Open AbstractQuery.copyTo() to allow custom Query class extension by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2173](https://github.com/JetBrains/Exposed/pull/2173) - feat: EXPOSED-461 Add time column in Joda-Time module by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2175](https://github.com/JetBrains/Exposed/pull/2175) Bug fixes: - fix: EXPOSED-424 ClassCastException exception when using `fetchBatchedResults` with `alias` by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2140](https://github.com/JetBrains/Exposed/pull/2140) - fix: EXPOSED-407 compositeMoney() nullability definition issues by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2137](https://github.com/JetBrains/Exposed/pull/2137) - fix: EXPOSED-415 SchemaUtils incorrectly generates ALTER statements for existing nullable columns by [@​obabichevjb](https://github.com/obabichevjb) in [https://github.com/JetBrains/Exposed/pull/2136](https://github.com/JetBrains/Exposed/pull/2136) - fix: EXPOSED-363 LocalTime and literal(LocalTime) are not the same by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2152](https://github.com/JetBrains/Exposed/pull/2152) - fix: EXPOSED-432 CurrentDate default is generated as null in MariaDB by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2149](https://github.com/JetBrains/Exposed/pull/2149) - fix: Allow column reference in default expressions for MySQL and MariaDB by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2159](https://github.com/JetBrains/Exposed/pull/2159) - fix: EXPOSED-430 Insert and BatchInsert do not return default values by [@​obabichevjb](https://github.com/obabichevjb) in [https://github.com/JetBrains/Exposed/pull/2158](https://github.com/JetBrains/Exposed/pull/2158) - fix: EXPOSED-452 Flaky H2\_Oracle test `testTimestampWithTimeZoneDefaults` by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2169](https://github.com/JetBrains/Exposed/pull/2169) - EXPOSED-457 The column default value always compares unequal by [@​obabichevjb](https://github.com/obabichevjb) in [https://github.com/JetBrains/Exposed/pull/2170](https://github.com/JetBrains/Exposed/pull/2170) - EXPOSED-409 Custom primary key. Access to the primary key fails with ClassCastException by [@​obabichevjb](https://github.com/obabichevjb) in [https://github.com/JetBrains/Exposed/pull/2151](https://github.com/JetBrains/Exposed/pull/2151) - fix: EXPOSED-447 Eager loading does not work with composite PK entity by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2177](https://github.com/JetBrains/Exposed/pull/2177) Docs: - chore: Add migration sample by [@​joc-a](https://github.com/joc-a) in [https://github.com/JetBrains/Exposed/pull/2144](https://github.com/JetBrains/Exposed/pull/2144) - docs: Change repetitionAttempts to maxAttempts in website docs by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2164](https://github.com/JetBrains/Exposed/pull/2164) - docs: EXPOSED-445 Add documentation for DSL & DAO composite primary keys by [@​bog-walk](https://github.com/bog-walk) in [https://github.com/JetBrains/Exposed/pull/2165](https://github.com/JetBrains/Exposed/pull/2165) - docs: EXPOSED-419 Rework the getting started tutorial by [@​vnikolova](https://github.com/vnikolova) in [https://github.com/JetBrains/Exposed/pull/2160](https://github.com/JetBrains/Exposed/pull/2160) - Configure API documentation for Exposed by [@​e5l](https://github.com/e5l) in [https://github.com/JetBrains/Exposed/pull/2171](https://github.com/JetBrains/Exposed/pull/2171) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View the [repository job log](https://developer.mend.io/github/DonRobo/home-former). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40NDAuNyIsInVwZGF0ZWRJblZlciI6IjM3LjQ0MC43IiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Adds a new
CompositeIdTable
that allows id columns to be marked forEntityID
inclusion using the originalColumn.entityID()
:Adds all the necessary entity class objects, including a
CompositeID
that will be wrapped byEntityID
for value comparison:CompositeIdTable.id
represents the collective component columns, and is not actually registered with the DB, to keep existing logic clean, with a few advantages:It can be used in DSL, as for any
IdTable
, for example withinsertAndGetId()
:CompositeIdTable.id
can be used to access all component columns inResultRow
. Even though the result set returns each column value separately, using the previous accessor returns a newEntityID<CompositeID>
, just as for a standard entity id column:CompositeIdTable.id
can be used in the same way anyIdTable.id
can (for example, in WHERE or SELECT clauses), to easily include all deconstructed PK columns in queries:A
CompositeIdTable
can be referenced by creating a table-level foreign key constraint on the child table, just as for DSL:Remaining tasks: