From 8266130b5fbf46c8ebec36020cd5cedd23dc0fb9 Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Mon, 5 Jun 2023 19:01:15 -0400 Subject: [PATCH 1/3] fix: EXPOSED-57 BatchInsertStatement can't be used with MySQL upsert Enable codeblock that was commented out by issue #129. Add condition that prevents AssertionError if autoGeneratedKeys size mismatch occurs with a dialect that supports a third affected-row value (namely MySQL, MariaDB, and both related H2 modes). Add KDocs explaining that returned value of InsertStatement.insertedCount is calculated as the sum of individual statement values when used in a batch. Include note that some vendors (MySQL-related dialects) return 2 if an existing row is updated, so returned value from an upsert mimics the affected-row value from the database. Refactor test helper, excludingH2Version1, to use a pre-existing H2Dialect function, isSecondVersion. --- .../exposed/sql/statements/InsertStatement.kt | 22 ++++-- .../jetbrains/exposed/sql/vendors/Default.kt | 3 + .../org/jetbrains/exposed/sql/vendors/H2.kt | 3 + .../jetbrains/exposed/sql/vendors/Mysql.kt | 2 + .../exposed/sql/tests/DatabaseTestsBase.kt | 4 +- .../sql/tests/shared/dml/UpsertTests.kt | 75 +++++++++++++++---- 6 files changed, 84 insertions(+), 25 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt index 8a16738aa4..62f030b5c4 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt @@ -2,8 +2,7 @@ package org.jetbrains.exposed.sql.statements import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.statements.api.PreparedStatementApi -import org.jetbrains.exposed.sql.vendors.PostgreSQLDialect -import org.jetbrains.exposed.sql.vendors.currentDialect +import org.jetbrains.exposed.sql.vendors.* import org.jetbrains.exposed.sql.vendors.inProperCase import java.sql.ResultSet import java.sql.SQLException @@ -11,6 +10,15 @@ import kotlin.properties.Delegates open class InsertStatement(val table: Table, val isIgnore: Boolean = false) : UpdateBuilder(StatementType.INSERT, listOf(table)) { + /** + * Returns the number of rows affected by the insert operation. + * + * When returned by a `BatchInsertStatement` or `BatchUpsertStatement`, the returned value is calculated using the + * sum of the individual values generated by each statement. + * + * **Note**: Some vendors support returning the affected-row value of 2 if an existing row is updated by an upsert + * operation; please check the documentation. + */ var insertedCount: Int by Delegates.notNull() var resultedValues: List? = null @@ -62,12 +70,10 @@ open class InsertStatement(val table: Table, val isIgnore: Boolean = } } - /** TODO: https://github.com/JetBrains/Exposed/issues/129 - * doesn't work with MySQL `INSERT ... ON DUPLICATE UPDATE` - */ -// assert(isIgnore || autoGeneratedKeys.isEmpty() || autoGeneratedKeys.size == inserted) { -// "Number of autoincs (${autoGeneratedKeys.size}) doesn't match number of batch entries ($inserted)" -// } + assert(isIgnore || autoGeneratedKeys.isEmpty() || + autoGeneratedKeys.size == inserted || currentDialect.supportsTernaryAffectedRowValues) { + "Number of autoincs (${autoGeneratedKeys.size}) doesn't match number of batch entries ($inserted)" + } } } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt index ab1e29adb0..4c0f648764 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Default.kt @@ -793,6 +793,9 @@ interface DatabaseDialect { val supportsSequenceAsGeneratedKeys: Boolean get() = supportsCreateSequence val supportsOnlyIdentifiersInGeneratedKeys: Boolean get() = false + /** Returns `true` if the dialect supports an upsert operation returning an affected-row value of 0, 1, or 2. */ + val supportsTernaryAffectedRowValues: Boolean get() = false + /** Returns`true` if the dialect supports schema creation. */ val supportsCreateSchema: Boolean get() = true diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt index ebaf55ef32..5ba6474ac9 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt @@ -211,6 +211,9 @@ open class H2Dialect : VendorDialect(dialectName, H2DataTypeProvider, H2Function override val supportsSequenceAsGeneratedKeys: Boolean by lazy { resolveDelegatedDialect()?.supportsSequenceAsGeneratedKeys ?: super.supportsSequenceAsGeneratedKeys } + override val supportsTernaryAffectedRowValues: Boolean by lazy { + resolveDelegatedDialect()?.supportsTernaryAffectedRowValues ?: super.supportsTernaryAffectedRowValues + } override val supportsCreateSchema: Boolean by lazy { resolveDelegatedDialect()?.supportsCreateSchema ?: super.supportsCreateSchema } override val supportsSubqueryUnions: Boolean by lazy { resolveDelegatedDialect()?.supportsSubqueryUnions ?: super.supportsSubqueryUnions } override val supportsDualTableConcept: Boolean by lazy { resolveDelegatedDialect()?.supportsDualTableConcept ?: super.supportsDualTableConcept } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt index 4b6740c616..ee75bad9e5 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/Mysql.kt @@ -205,6 +205,8 @@ open class MysqlDialect : VendorDialect(dialectName, MysqlDataTypeProvider, Mysq override val supportsCreateSequence: Boolean = false + override val supportsTernaryAffectedRowValues: Boolean = true + override val supportsSubqueryUnions: Boolean = true override val supportsOrderByNullsFirstLast: Boolean = false diff --git a/exposed-tests/src/main/kotlin/org/jetbrains/exposed/sql/tests/DatabaseTestsBase.kt b/exposed-tests/src/main/kotlin/org/jetbrains/exposed/sql/tests/DatabaseTestsBase.kt index a3e078f18c..db7f2a5e65 100644 --- a/exposed-tests/src/main/kotlin/org/jetbrains/exposed/sql/tests/DatabaseTestsBase.kt +++ b/exposed-tests/src/main/kotlin/org/jetbrains/exposed/sql/tests/DatabaseTestsBase.kt @@ -7,11 +7,11 @@ import org.jetbrains.exposed.sql.transactions.inTopLevelTransaction import org.jetbrains.exposed.sql.transactions.nullableTransactionScope import org.jetbrains.exposed.sql.transactions.transaction import org.jetbrains.exposed.sql.transactions.transactionManager +import org.jetbrains.exposed.sql.vendors.H2Dialect import org.junit.Assume import org.junit.AssumptionViolatedException import org.testcontainers.containers.MySQLContainer import org.testcontainers.containers.PostgreSQLContainer -import java.math.BigDecimal import java.sql.Connection import java.sql.SQLException import java.time.Duration @@ -286,7 +286,7 @@ abstract class DatabaseTestsBase { } fun Transaction.excludingH2Version1(dbSettings: TestDB, statement: Transaction.(TestDB) -> Unit) { - if (dbSettings !in TestDB.allH2TestDB || db.isVersionCovers(BigDecimal("2.0"))) { + if (dbSettings !in TestDB.allH2TestDB || (db.dialect as H2Dialect).isSecondVersion) { statement(dbSettings) } } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpsertTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpsertTests.kt index af52e76c46..ba24fdbbf1 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpsertTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpsertTests.kt @@ -7,11 +7,13 @@ import org.jetbrains.exposed.sql.SqlExpressionBuilder.concat import org.jetbrains.exposed.sql.SqlExpressionBuilder.less import org.jetbrains.exposed.sql.SqlExpressionBuilder.minus import org.jetbrains.exposed.sql.SqlExpressionBuilder.plus +import org.jetbrains.exposed.sql.statements.BatchUpsertStatement import org.jetbrains.exposed.sql.tests.* import org.jetbrains.exposed.sql.tests.shared.assertEquals import org.jetbrains.exposed.sql.tests.shared.expectException import org.junit.Test import java.util.* +import kotlin.properties.Delegates // Upsert implementation does not support H2 version 1 // https://youtrack.jetbrains.com/issue/EXPOSED-30/Phase-Out-Support-for-H2-Version-1.x @@ -21,31 +23,24 @@ class UpsertTests : DatabaseTestsBase() { @Test fun testUpsertWithPKConflict() { - val tester = object : Table("tester") { - val id = integer("id").autoIncrement() - val name = varchar("name", 64) - - override val primaryKey = PrimaryKey(id) - } - - withTables(tester) { testDb -> + withTables(AutoIncTable) { testDb -> excludingH2Version1(testDb) { - val id1 = tester.insert { + val id1 = AutoIncTable.insert { it[name] = "A" - } get tester.id + } get AutoIncTable.id - tester.upsert { + AutoIncTable.upsert { if (testDb in upsertViaMergeDB) it[id] = 2 it[name] = "B" } - tester.upsert { + AutoIncTable.upsert { it[id] = id1 it[name] = "C" } - assertEquals(2, tester.selectAll().count()) - val updatedResult = tester.select { tester.id eq id1 }.single() - assertEquals("C", updatedResult[tester.name]) + assertEquals(2, AutoIncTable.selectAll().count()) + val updatedResult = AutoIncTable.select { AutoIncTable.id eq id1 }.single() + assertEquals("C", updatedResult[AutoIncTable.name]) } } } @@ -424,6 +419,56 @@ class UpsertTests : DatabaseTestsBase() { } } + @Test + fun testInsertedCountWithBatchUpsert() { + withTables(AutoIncTable) { testDb -> + excludingH2Version1(testDb) { + // SQL Server requires statements to be executed before results can be obtained + val isNotSqlServer = testDb != TestDB.SQLSERVER + val data = listOf(1 to "A", 2 to "B", 3 to "C") + val newDataSize = data.size + var statement: BatchUpsertStatement by Delegates.notNull() + + // all new rows inserted + AutoIncTable.batchUpsert(data, shouldReturnGeneratedValues = isNotSqlServer) { (id, name) -> + statement = this + this[AutoIncTable.id] = id + this[AutoIncTable.name] = name + } + assertEquals(newDataSize, statement.insertedCount) + + // all existing rows set to their current values + val isH2MysqlMode = testDb == TestDB.H2_MYSQL || testDb == TestDB.H2_MARIADB + var expected = if (isH2MysqlMode) 0 else newDataSize + AutoIncTable.batchUpsert(data, shouldReturnGeneratedValues = isNotSqlServer) { (id, name) -> + statement = this + this[AutoIncTable.id] = id + this[AutoIncTable.name] = name + } + assertEquals(expected, statement.insertedCount) + + // all existing rows updated & 1 new row inserted + val updatedData = data.map { it.first to "new${it.second}" } + (4 to "D") + expected = if (testDb in TestDB.mySqlRelatedDB) newDataSize * 2 + 1 else newDataSize + 1 + AutoIncTable.batchUpsert(updatedData, shouldReturnGeneratedValues = isNotSqlServer) { (id, name) -> + statement = this + this[AutoIncTable.id] = id + this[AutoIncTable.name] = name + } + assertEquals(expected, statement.insertedCount) + + assertEquals(updatedData.size.toLong(), AutoIncTable.selectAll().count()) + } + } + } + + private object AutoIncTable : Table("auto_inc_table") { + val id = integer("id").autoIncrement() + val name = varchar("name", 64) + + override val primaryKey = PrimaryKey(id) + } + private object Words : Table("words") { val word = varchar("name", 64).uniqueIndex() val count = integer("count").default(1) From 8e779903742514cbca2769cbdb08977f51f9d7c1 Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Mon, 5 Jun 2023 20:22:45 -0400 Subject: [PATCH 2/3] Fix detekt issues causing max threshold build fail --- .../exposed/sql/statements/InsertStatement.kt | 22 ++++++++++++++----- .../org/jetbrains/exposed/sql/vendors/H2.kt | 4 +++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt index 62f030b5c4..96920d586f 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt @@ -8,7 +8,10 @@ import java.sql.ResultSet import java.sql.SQLException import kotlin.properties.Delegates -open class InsertStatement(val table: Table, val isIgnore: Boolean = false) : UpdateBuilder(StatementType.INSERT, listOf(table)) { +open class InsertStatement( + val table: Table, + val isIgnore: Boolean = false +) : UpdateBuilder(StatementType.INSERT, listOf(table)) { /** * Returns the number of rows affected by the insert operation. @@ -41,7 +44,8 @@ open class InsertStatement(val table: Table, val isIgnore: Boolean = val autoGeneratedKeys = arrayListOf, Any?>>() if (inserted > 0) { - val returnedColumns = (if (currentDialect.supportsOnlyIdentifiersInGeneratedKeys) autoIncColumns else table.columns).mapNotNull { col -> + val columns = if (currentDialect.supportsOnlyIdentifiersInGeneratedKeys) autoIncColumns else table.columns + val returnedColumns = columns.mapNotNull { col -> @Suppress("SwallowedException") try { rs?.findColumn(col.name)?.let { col to it } @@ -53,12 +57,17 @@ open class InsertStatement(val table: Table, val isIgnore: Boolean = val firstAutoIncColumn = autoIncColumns.firstOrNull { it.autoIncColumnType != null } ?: autoIncColumns.firstOrNull() if (firstAutoIncColumn != null || returnedColumns.isNotEmpty()) { while (rs?.next() == true) { - val returnedValues = returnedColumns.associateTo(mutableMapOf()) { it.first to rs.getObject(it.second) } - if (returnedValues.isEmpty() && firstAutoIncColumn != null) returnedValues[firstAutoIncColumn] = rs.getObject(1) + val returnedValues = returnedColumns.associateTo(mutableMapOf()) { + it.first to rs.getObject(it.second) + } + if (returnedValues.isEmpty() && firstAutoIncColumn != null) { + returnedValues[firstAutoIncColumn] = rs.getObject(1) + } autoGeneratedKeys.add(returnedValues) } - if (inserted > 1 && firstAutoIncColumn != null && autoGeneratedKeys.isNotEmpty() && !currentDialect.supportsMultipleGeneratedKeys) { + if (inserted > 1 && firstAutoIncColumn != null && + autoGeneratedKeys.isNotEmpty() && !currentDialect.supportsMultipleGeneratedKeys) { // H2/SQLite only returns one last generated key... (autoGeneratedKeys[0][firstAutoIncColumn] as? Number)?.toLong()?.let { var id = it @@ -115,7 +124,8 @@ open class InsertStatement(val table: Table, val isIgnore: Boolean = override fun prepareSQL(transaction: Transaction): String { val values = arguments!!.first() val sql = values.toSqlString() - return transaction.db.dialect.functionProvider.insert(isIgnore, table, values.map { it.first }, sql, transaction) + val dialect = transaction.db.dialect + return dialect.functionProvider.insert(isIgnore, table, values.map { it.first }, sql, transaction) } protected fun List, Any?>>.toSqlString(): String { diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt index 5ba6474ac9..39c2768d92 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt @@ -133,7 +133,9 @@ open class H2Dialect : VendorDialect(dialectName, H2DataTypeProvider, H2Function val isSecondVersion get() = majorVersion == H2MajorVersion.Two - private fun exactH2Version(transaction: Transaction): String = transaction.db.metadata { databaseProductVersion.substringBefore(" (") } + private fun exactH2Version(transaction: Transaction): String = transaction.db.metadata { + databaseProductVersion.substringBefore(" (") + } enum class H2CompatibilityMode { MySQL, MariaDB, SQLServer, Oracle, PostgreSQL From 2e2622eea776b90ae0cc2d9acc60bca6ec75fc6a Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Tue, 6 Jun 2023 11:07:00 -0400 Subject: [PATCH 3/3] Revert "Fix detekt issues causing max threshold build fail" This reverts commit 7c2c52767be2a4bfd18bf8222c2e261e8af87461. Relevant Detekt issues are addressed in PR#1752 --- .../exposed/sql/statements/InsertStatement.kt | 22 +++++-------------- .../org/jetbrains/exposed/sql/vendors/H2.kt | 4 +--- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt index 96920d586f..62f030b5c4 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt @@ -8,10 +8,7 @@ import java.sql.ResultSet import java.sql.SQLException import kotlin.properties.Delegates -open class InsertStatement( - val table: Table, - val isIgnore: Boolean = false -) : UpdateBuilder(StatementType.INSERT, listOf(table)) { +open class InsertStatement(val table: Table, val isIgnore: Boolean = false) : UpdateBuilder(StatementType.INSERT, listOf(table)) { /** * Returns the number of rows affected by the insert operation. @@ -44,8 +41,7 @@ open class InsertStatement( val autoGeneratedKeys = arrayListOf, Any?>>() if (inserted > 0) { - val columns = if (currentDialect.supportsOnlyIdentifiersInGeneratedKeys) autoIncColumns else table.columns - val returnedColumns = columns.mapNotNull { col -> + val returnedColumns = (if (currentDialect.supportsOnlyIdentifiersInGeneratedKeys) autoIncColumns else table.columns).mapNotNull { col -> @Suppress("SwallowedException") try { rs?.findColumn(col.name)?.let { col to it } @@ -57,17 +53,12 @@ open class InsertStatement( val firstAutoIncColumn = autoIncColumns.firstOrNull { it.autoIncColumnType != null } ?: autoIncColumns.firstOrNull() if (firstAutoIncColumn != null || returnedColumns.isNotEmpty()) { while (rs?.next() == true) { - val returnedValues = returnedColumns.associateTo(mutableMapOf()) { - it.first to rs.getObject(it.second) - } - if (returnedValues.isEmpty() && firstAutoIncColumn != null) { - returnedValues[firstAutoIncColumn] = rs.getObject(1) - } + val returnedValues = returnedColumns.associateTo(mutableMapOf()) { it.first to rs.getObject(it.second) } + if (returnedValues.isEmpty() && firstAutoIncColumn != null) returnedValues[firstAutoIncColumn] = rs.getObject(1) autoGeneratedKeys.add(returnedValues) } - if (inserted > 1 && firstAutoIncColumn != null && - autoGeneratedKeys.isNotEmpty() && !currentDialect.supportsMultipleGeneratedKeys) { + if (inserted > 1 && firstAutoIncColumn != null && autoGeneratedKeys.isNotEmpty() && !currentDialect.supportsMultipleGeneratedKeys) { // H2/SQLite only returns one last generated key... (autoGeneratedKeys[0][firstAutoIncColumn] as? Number)?.toLong()?.let { var id = it @@ -124,8 +115,7 @@ open class InsertStatement( override fun prepareSQL(transaction: Transaction): String { val values = arguments!!.first() val sql = values.toSqlString() - val dialect = transaction.db.dialect - return dialect.functionProvider.insert(isIgnore, table, values.map { it.first }, sql, transaction) + return transaction.db.dialect.functionProvider.insert(isIgnore, table, values.map { it.first }, sql, transaction) } protected fun List, Any?>>.toSqlString(): String { diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt index 39c2768d92..5ba6474ac9 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt @@ -133,9 +133,7 @@ open class H2Dialect : VendorDialect(dialectName, H2DataTypeProvider, H2Function val isSecondVersion get() = majorVersion == H2MajorVersion.Two - private fun exactH2Version(transaction: Transaction): String = transaction.db.metadata { - databaseProductVersion.substringBefore(" (") - } + private fun exactH2Version(transaction: Transaction): String = transaction.db.metadata { databaseProductVersion.substringBefore(" (") } enum class H2CompatibilityMode { MySQL, MariaDB, SQLServer, Oracle, PostgreSQL