From 77548ed2f5850b053c397e684a004fa964f8930b Mon Sep 17 00:00:00 2001 From: obabichevjb <166523824+obabichevjb@users.noreply.github.com> Date: Wed, 20 Nov 2024 09:07:28 +0100 Subject: [PATCH] =?UTF-8?q?fix!=20EXPOSED-458=20Stop=20sending=20default?= =?UTF-8?q?=20and=20null=20values=20in=20insert=20state=E2=80=A6=20(#2295)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix! EXPOSED-458 Stop sending default and null values in insert statements --- .../Writerside/topics/Breaking-Changes.md | 25 +++ .../topics/DSL-CRUD-operations.topic | 6 + exposed-core/api/exposed-core.api | 6 +- .../org/jetbrains/exposed/sql/Column.kt | 2 +- .../org/jetbrains/exposed/sql/ColumnType.kt | 10 +- .../statements/BaseBatchInsertStatement.kt | 37 ++-- .../sql/statements/BatchUpsertStatement.kt | 9 +- .../exposed/sql/statements/InsertStatement.kt | 197 +++++++++++------- .../exposed/sql/statements/UpsertStatement.kt | 8 +- .../exposed/sql/vendors/FunctionProvider.kt | 14 +- .../exposed/sql/vendors/MysqlDialect.kt | 9 +- .../org/jetbrains/exposed/DefaultsTest.kt | 3 +- .../jetbrains/exposed/JodaTimeDefaultsTest.kt | 6 +- .../sql/kotlin/datetime/DefaultsTest.kt | 6 +- .../ddl/CreateMissingTablesAndColumnsTests.kt | 2 +- .../sql/tests/shared/dml/InsertTests.kt | 38 ++++ .../sql/tests/shared/dml/UpsertTests.kt | 67 ++++-- .../shared/functions/MathFunctionTests.kt | 3 +- 18 files changed, 304 insertions(+), 144 deletions(-) diff --git a/documentation-website/Writerside/topics/Breaking-Changes.md b/documentation-website/Writerside/topics/Breaking-Changes.md index 64f14a2f81..b7e737d407 100644 --- a/documentation-website/Writerside/topics/Breaking-Changes.md +++ b/documentation-website/Writerside/topics/Breaking-Changes.md @@ -1,5 +1,30 @@ # Breaking Changes +## 0.57.0 +* Insert, Upsert, and Replace statements will no longer implicitly send all default values (except for client-side default values) in every SQL request. + This change will reduce the amount of data Exposed sends to the database and make Exposed rely more on the database's default values. + However, this may uncover previously hidden issues related to actual database default values, which were masked by Exposed's insert/upsert statements. + Also from `InsertStatement` was removed protected method `isColumnValuePreferredFromResultSet()` and method `valuesAndDefaults()` was marked as deprecated. + + Let's say you have a table with columns that have default values, and you use an insert statement like this: + ```kotlin + object TestTable : IntIdTable("test") { + val number = integer("number").default(100) + val expression = integer("exp") + .defaultExpression(intLiteral(100) + intLiteral(200)) + } + + TestTable.insert { } + ``` + This insert statement would generate the following SQL in the H2 database: + ```sql + -- For versions before 0.57.0 + INSERT INTO TEST ("number", "exp") VALUES (100, (100 + 200)) + + -- Starting from version 0.57.0 + INSERT INTO TEST DEFAULT VALUES + ``` + ## 0.56.0 * If the `distinct` parameter of `groupConcat()` is set to `true`, when using Oracle or SQL Server, this will now fail early with an `UnsupportedByDialectException`. Previously, the setting would be ignored and SQL function generation would not include a `DISTINCT` clause. diff --git a/documentation-website/Writerside/topics/DSL-CRUD-operations.topic b/documentation-website/Writerside/topics/DSL-CRUD-operations.topic index 2f6c65c282..0f53539b4a 100644 --- a/documentation-website/Writerside/topics/DSL-CRUD-operations.topic +++ b/documentation-website/Writerside/topics/DSL-CRUD-operations.topic @@ -356,6 +356,12 @@ it[director] = "Rian Johnson" } + + + If the insertValue() for a particular column is used within the onUpdate block, + that column must be defined in the insert section, unless it has a client side default value. + +

If the update operation should be identical to the insert operation except for a few columns, then onUpdateExclude should be provided an argument with the specific columns to exclude. This parameter could also be used for the reverse case when only a small subset of columns should be updated diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index 14025ab23f..4b06fe891f 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -3093,7 +3093,6 @@ public abstract class org/jetbrains/exposed/sql/statements/BaseBatchInsertStatem public fun prepared (Lorg/jetbrains/exposed/sql/Transaction;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi; public fun set (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)V public fun setArguments (Ljava/util/List;)V - protected fun valuesAndDefaults (Ljava/util/Map;)Ljava/util/Map; } public final class org/jetbrains/exposed/sql/statements/BatchDataInconsistentException : java/lang/Exception { @@ -3136,7 +3135,6 @@ public class org/jetbrains/exposed/sql/statements/BatchUpsertStatement : org/jet public final fun getOnUpdateExclude ()Ljava/util/List; public final fun getWhere ()Lorg/jetbrains/exposed/sql/Op; public fun insertValue (Lorg/jetbrains/exposed/sql/Column;)Lorg/jetbrains/exposed/sql/ExpressionWithColumnType; - protected fun isColumnValuePreferredFromResultSet (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)Z public fun prepareSQL (Lorg/jetbrains/exposed/sql/Transaction;Z)Ljava/lang/String; public fun prepared (Lorg/jetbrains/exposed/sql/Transaction;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi; public fun storeUpdateValues (Lkotlin/jvm/functions/Function2;)V @@ -3199,6 +3197,7 @@ public class org/jetbrains/exposed/sql/statements/InsertStatement : org/jetbrain public synthetic fun (Lorg/jetbrains/exposed/sql/Table;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V public synthetic fun arguments ()Ljava/lang/Iterable; public fun arguments ()Ljava/util/List; + protected final fun clientDefaultColumns ()Ljava/util/List; protected fun execInsertFunction (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;)Lkotlin/Pair; public fun executeInternal (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/Integer; public synthetic fun executeInternal (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/Object; @@ -3218,6 +3217,8 @@ public class org/jetbrains/exposed/sql/statements/InsertStatement : org/jetbrain public fun setArguments (Ljava/util/List;)V public final fun setInsertedCount (I)V protected final fun toSqlString (Ljava/util/List;Z)Ljava/lang/String; + protected final fun valuesAndClientDefaults (Ljava/util/Map;)Ljava/util/Map; + public static synthetic fun valuesAndClientDefaults$default (Lorg/jetbrains/exposed/sql/statements/InsertStatement;Ljava/util/Map;ILjava/lang/Object;)Ljava/util/Map; protected fun valuesAndDefaults (Ljava/util/Map;)Ljava/util/Map; public static synthetic fun valuesAndDefaults$default (Lorg/jetbrains/exposed/sql/statements/InsertStatement;Ljava/util/Map;ILjava/lang/Object;)Ljava/util/Map; } @@ -3484,7 +3485,6 @@ public class org/jetbrains/exposed/sql/statements/UpsertStatement : org/jetbrain public final fun getOnUpdateExclude ()Ljava/util/List; public final fun getWhere ()Lorg/jetbrains/exposed/sql/Op; public fun insertValue (Lorg/jetbrains/exposed/sql/Column;)Lorg/jetbrains/exposed/sql/ExpressionWithColumnType; - protected fun isColumnValuePreferredFromResultSet (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)Z public fun prepareSQL (Lorg/jetbrains/exposed/sql/Transaction;Z)Ljava/lang/String; public fun prepared (Lorg/jetbrains/exposed/sql/Transaction;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi; public fun storeUpdateValues (Lkotlin/jvm/functions/Function2;)V diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt index 8ebe030bf1..cdbb2b8025 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt @@ -126,7 +126,7 @@ class Column( val expressionSQL = currentDialect.dataTypeProvider.processForDefaultValue(defaultValue) if (!currentDialect.isAllowedAsColumnDefault(defaultValue)) { val clientDefault = when { - defaultValueFun != null -> " Expression will be evaluated on the client." + defaultValueFun != null && dbDefaultValue == null -> " Expression will be evaluated on the client." !columnType.nullable -> " Column will be created with NULL marker." else -> "" } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt index a3058dfa76..92002e22f8 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt @@ -374,6 +374,10 @@ open class ColumnWithTransform( } } +internal fun unwrapColumnValues(values: Map, Any?>): Map, Any?> = values.mapValues { (col, value) -> + value?.let { (col.columnType as? ColumnWithTransform)?.unwrapRecursive(it) } ?: value +} + /** * A class that handles the transformation between a source column type and a target type, * but also supports transformations involving `null` values. @@ -1014,7 +1018,11 @@ class BlobColumnType( else -> error("Unexpected value of type Blob: $value of ${value::class.qualifiedName}") } - override fun nonNullValueToString(value: ExposedBlob): String = currentDialect.dataTypeProvider.hexToDb(value.hexString()) + override fun nonNullValueToString(value: ExposedBlob): String { + // For H2 Blobs the original dataTypeProvider must be taken (even if H2 in other DB mode) + return ((currentDialect as? H2Dialect)?.originalDataTypeProvider ?: currentDialect.dataTypeProvider) + .hexToDb(value.hexString()) + } override fun readObject(rs: ResultSet, index: Int) = when { currentDialect is SQLServerDialect -> rs.getBytes(index)?.let(::ExposedBlob) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BaseBatchInsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BaseBatchInsertStatement.kt index a6352c17ff..d7abc30399 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BaseBatchInsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BaseBatchInsertStatement.kt @@ -1,11 +1,6 @@ package org.jetbrains.exposed.sql.statements -import org.jetbrains.exposed.sql.Column -import org.jetbrains.exposed.sql.EntityIDColumnType -import org.jetbrains.exposed.sql.ResultRow -import org.jetbrains.exposed.sql.Table -import org.jetbrains.exposed.sql.Transaction -import org.jetbrains.exposed.sql.isAutoInc +import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.statements.api.PreparedStatementApi import org.jetbrains.exposed.sql.transactions.TransactionManager @@ -88,18 +83,26 @@ abstract class BaseBatchInsertStatement( override var arguments: List, Any?>>>? = null get() = field ?: run { - val nullableColumns by lazy { - allColumnsInDataSet().filter { it.columnType.nullable && !it.isDatabaseGenerated } - } - data.map { single -> - val valuesAndDefaults = super.valuesAndDefaults(single) as MutableMap - val nullableMap = (nullableColumns - valuesAndDefaults.keys).associateWith { null } - valuesAndDefaults.putAll(nullableMap) - valuesAndDefaults.toList().sortedBy { it.first } - }.apply { field = this } - } + val columnsToInsert = (allColumnsInDataSet() + clientDefaultColumns()).toSet() - override fun valuesAndDefaults(values: Map, Any?>) = arguments!!.first().toMap() + data + .map { valuesAndClientDefaults(it) as MutableMap } + .map { values -> + columnsToInsert.map { column -> + column to when { + values.contains(column) -> values[column] + column.dbDefaultValue != null || column.isDatabaseGenerated -> DefaultValueMarker + else -> { + require(column.columnType.nullable) { + "The value for the column ${column.name} was not provided. " + + "The value for non-nullable column without defaults must be specified." + } + null + } + } + } + }.apply { field = this } + } override fun prepared(transaction: Transaction, sql: String): PreparedStatementApi { return if (!shouldReturnGeneratedValues) { diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BatchUpsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BatchUpsertStatement.kt index fcf3e427f7..cfd5f3235c 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BatchUpsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BatchUpsertStatement.kt @@ -3,6 +3,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.MysqlFunctionProvider +import org.jetbrains.exposed.sql.vendors.OracleDialect import org.jetbrains.exposed.sql.vendors.currentDialect /** @@ -54,8 +55,9 @@ open class BatchUpsertStatement( val keyColumns = if (functionProvider is MysqlFunctionProvider) keys.toList() else getKeyColumns(keys = keys) val insertValues = arguments!!.first() val insertValuesSql = insertValues.toSqlString(prepared) + val updateExcludeColumns = (onUpdateExclude ?: emptyList()) + if (dialect is OracleDialect) keyColumns else emptyList() val updateExpressions = updateValues.takeIf { it.isNotEmpty() }?.toList() - ?: getUpdateExpressions(insertValues.unzip().first, onUpdateExclude, keyColumns) + ?: getUpdateExpressions(insertValues.unzip().first, updateExcludeColumns, keyColumns) return functionProvider.upsert(table, insertValues, insertValuesSql, updateExpressions, keyColumns, where, transaction) } @@ -74,9 +76,4 @@ open class BatchUpsertStatement( return super.prepared(transaction, sql) } - - override fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean { - return isEntityIdClientSideGeneratedUUID(column) || - super.isColumnValuePreferredFromResultSet(column, value) - } } 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 1f54a8534a..9eeed48353 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 @@ -52,99 +52,122 @@ open class InsertStatement( */ fun getOrNull(column: Column): T? = resultedValues?.firstOrNull()?.getOrNull(column) - @Suppress("NestedBlockDepth", "ComplexMethod", "TooGenericExceptionCaught") private fun processResults(rs: ResultSet?, inserted: Int): List { - val autoGeneratedKeys = arrayListOf, Any?>>() + val allResultSetsValues = rs?.returnedValues(inserted) - if (inserted > 0) { - val returnedColumns = (if (currentDialect.supportsOnlyIdentifiersInGeneratedKeys) autoIncColumns else table.columns).mapNotNull { col -> - @Suppress("SwallowedException") - try { - rs?.findColumn(col.name)?.let { col to it } - } catch (e: SQLException) { - null - } + @Suppress("UNCHECKED_CAST") + return arguments!! + // Join the values from ResultSet with arguments + .mapIndexed { index, columnValues -> + val resultSetValues = allResultSetsValues?.getOrNull(index) ?: hashMapOf() + val argumentValues = columnValues.toMap() + .filterValues { it != DefaultValueMarker } + .let { unwrapColumnValues(it) } + + argumentValues + resultSetValues } + .map { unwrapColumnValues(defaultAndNullableValues(exceptColumns = it.keys)) + it } + .map { ResultRow.createAndFillValues(it as Map, Any?>) } + } - val firstAutoIncColumn = autoIncColumns.firstOrNull { it.autoIncColumnType != null } ?: autoIncColumns.firstOrNull() - if (firstAutoIncColumn != null || returnedColumns.isNotEmpty()) { - while (rs?.next() == true) { - try { - val returnedValues = returnedColumns.associateTo(mutableMapOf()) { - it.first to it.first.columnType.readObject(rs, it.second) - } - if (returnedValues.isEmpty() && firstAutoIncColumn != null) { - returnedValues[firstAutoIncColumn] = rs.getObject(1) - } - autoGeneratedKeys.add(returnedValues) - } catch (cause: ArrayIndexOutOfBoundsException) { - // EXPOSED-191 Flaky Oracle test on TC build - // this try/catch should help to get information about the flaky test. - // try/catch can be safely removed after the fixing the issue. - // TooGenericExceptionCaught suppress also can be removed - - val preparedSql = this.prepareSQL(TransactionManager.current(), prepared = true) - - val returnedColumnsString = returnedColumns - .mapIndexed { index, pair -> "column: ${pair.first.name}, index: ${pair.second} (columns-list-index: $index)" } - .joinToString(prefix = "[", postfix = "]", separator = ", ") - - exposedLogger.error( - "ArrayIndexOutOfBoundsException on processResults. " + - "Table: ${table.tableName}, firstAutoIncColumn: ${firstAutoIncColumn?.name}, " + - "inserted: $inserted, returnedColumnsString: $returnedColumnsString. " + - "Failed SQL: $preparedSql", - cause - ) - throw cause - } + private fun defaultAndNullableValues(exceptColumns: Collection>): Map, Any?> { + return table.columns + .filter { column -> !exceptColumns.contains(column) } + .mapNotNull { column -> + val defaultFn = column.defaultValueFun + when { + defaultFn != null -> column to defaultFn() + column.columnType.nullable -> column to null + else -> null } + } + .toMap() + } - 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 + @Suppress("NestedBlockDepth", "TooGenericExceptionCaught") + private fun ResultSet.returnedValues(inserted: Int): ArrayList, Any?>> { + if (inserted == 0) return arrayListOf() - while (autoGeneratedKeys.size < inserted) { - id -= 1 - autoGeneratedKeys.add(0, mutableMapOf(firstAutoIncColumn to id)) - } - } - } + val resultSetsValues = arrayListOf, Any?>>() + + val columnIndexesInResultSet = returnedColumns() - assert( - isIgnore || autoGeneratedKeys.isEmpty() || autoGeneratedKeys.size == inserted || - currentDialect.supportsTernaryAffectedRowValues - ) { - "Number of autoincs (${autoGeneratedKeys.size}) doesn't match number of batch entries ($inserted)" + val firstAutoIncColumn = autoIncColumns.firstOrNull() + if (firstAutoIncColumn != null || columnIndexesInResultSet.isNotEmpty()) { + while (next()) { + try { + val returnedValues = columnIndexesInResultSet.associateTo(mutableMapOf()) { + it.first to it.first.columnType.readObject(this, it.second) + } + if (returnedValues.isEmpty() && firstAutoIncColumn != null) { + returnedValues[firstAutoIncColumn] = getObject(1) + } + resultSetsValues.add(returnedValues) + } catch (cause: ArrayIndexOutOfBoundsException) { + // EXPOSED-191 Flaky Oracle test on TC build + // this try/catch should help to get information about the flaky test. + // try/catch can be safely removed after the fixing the issue. + // TooGenericExceptionCaught suppress also can be removed + + val preparedSql = prepareSQL(TransactionManager.current(), prepared = true) + + val returnedColumnsString = columnIndexesInResultSet + .mapIndexed { index, pair -> "column: ${pair.first.name}, index: ${pair.second} (columns-list-index: $index)" } + .joinToString(prefix = "[", postfix = "]", separator = ", ") + + exposedLogger.error( + "ArrayIndexOutOfBoundsException on processResults. " + + "Table: ${table.tableName}, firstAutoIncColumn: ${firstAutoIncColumn?.name}, " + + "inserted: $inserted, returnedColumnsString: $returnedColumnsString. " + + "Failed SQL: $preparedSql", + cause + ) + throw cause } } - } - arguments!!.forEachIndexed { itemIndx, pairs -> - val map = autoGeneratedKeys.getOrNull(itemIndx) ?: hashMapOf, Any?>().apply { - autoGeneratedKeys.add(itemIndx, this) - } - pairs.forEach { (col, value) -> - if (value != DefaultValueMarker) { - val unwrappedValue = value?.let { (col.columnType as? ColumnWithTransform)?.unwrapRecursive(it) } ?: value - if (isColumnValuePreferredFromResultSet(col, value)) { - map.getOrPut(col) { unwrappedValue } - } else { - map[col] = unwrappedValue + if (inserted > 1 && firstAutoIncColumn != null && resultSetsValues.isNotEmpty() && !currentDialect.supportsMultipleGeneratedKeys) { + // H2/SQLite only returns one last generated key... + (resultSetsValues[0][firstAutoIncColumn] as? Number)?.toLong()?.let { + var id = it + + while (resultSetsValues.size < inserted) { + id -= 1 + resultSetsValues.add(0, mutableMapOf(firstAutoIncColumn to id)) } } } + + assert( + isIgnore || resultSetsValues.isEmpty() || resultSetsValues.size == inserted || + currentDialect.supportsTernaryAffectedRowValues + ) { + "Number of autoincs (${resultSetsValues.size}) doesn't match number of batch entries ($inserted)" + } } - @Suppress("UNCHECKED_CAST") - return autoGeneratedKeys.map { ResultRow.createAndFillValues(it as Map, Any?>) } + + return resultSetsValues } - protected open fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean { - return column.columnType.isAutoInc || value is NextVal<*> + /** + * Returns indexes of the table columns in [ResultSet] + */ + private fun ResultSet?.returnedColumns() = (if (currentDialect.supportsOnlyIdentifiersInGeneratedKeys) autoIncColumns else table.columns).mapNotNull { col -> + @Suppress("SwallowedException") + try { + this?.findColumn(col.name)?.let { col to it } + } catch (e: SQLException) { + null + } } @Suppress("NestedBlockDepth") + @Deprecated( + "This function is used in derived classes to build a list of arguments. " + + "It's recommended to avoid including all default and nullable values in insert statements, " + + "as these values can often be generated automatically by the database. " + + "There are no usages of that function inside Exposed. Saved as deprecated for back compatability" + ) protected open fun valuesAndDefaults(values: Map, Any?> = this.values): Map, Any?> { val result = values.toMutableMap() targets.forEach { table -> @@ -161,6 +184,27 @@ open class InsertStatement( return result } + @Deprecated( + "This function has been obsolete since version 0.57.0, " + + "following the removal of default values from insert statements. " + + "It's safe to remove any overrides of this function from your code." + ) + protected open fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean { + return column.columnType.isAutoInc || value is NextVal<*> + } + + protected fun clientDefaultColumns() = targets + // The current check for existing client side without db side default value + .flatMap { it.columns.filter { column -> column.dbDefaultValue == null && column.defaultValueFun != null } } + + protected fun valuesAndClientDefaults(values: Map, Any?> = this.values): Map, Any?> { + val clientDefaultValues = clientDefaultColumns() + .filter { column -> column !in values.keys } + .map { column -> column to column.defaultValueFun!!() } + + return clientDefaultValues.toMap() + values + } + override fun prepareSQL(transaction: Transaction, prepared: Boolean): String { val values = arguments!!.first() val sql = values.toSqlString(prepared) @@ -231,11 +275,8 @@ open class InsertStatement( open var arguments: List, Any?>>>? = null get() = field ?: run { - val nullableColumns = table.columns.filter { it.columnType.nullable && !it.isDatabaseGenerated } - val valuesAndDefaults = valuesAndDefaults() as MutableMap - valuesAndDefaults.putAll((nullableColumns - valuesAndDefaults.keys).associateWith { null }) - val result = valuesAndDefaults.toList() - listOf(result).apply { field = this } + listOf(valuesAndClientDefaults().toList()) + .apply { field = this } } override fun arguments(): List, Any?>>> { diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpsertStatement.kt index d8cff20ece..027aa162f5 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpsertStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpsertStatement.kt @@ -50,8 +50,9 @@ open class UpsertStatement( val keyColumns = if (functionProvider is MysqlFunctionProvider) keys.toList() else getKeyColumns(keys = keys) val insertValues = arguments!!.first() val insertValuesSql = insertValues.toSqlString(prepared) + val updateExcludeColumns = (onUpdateExclude ?: emptyList()) + if (dialect is OracleDialect) keyColumns else emptyList() val updateExpressions = updateValues.takeIf { it.isNotEmpty() }?.toList() - ?: getUpdateExpressions(insertValues.unzip().first, onUpdateExclude, keyColumns) + ?: getUpdateExpressions(insertValues.unzip().first, updateExcludeColumns, keyColumns) return functionProvider.upsert(table, insertValues, insertValuesSql, updateExpressions, keyColumns, where, transaction) } @@ -62,11 +63,6 @@ open class UpsertStatement( } } - override fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean { - return isEntityIdClientSideGeneratedUUID(column) || - super.isColumnValuePreferredFromResultSet(column, value) - } - override fun prepared(transaction: Transaction, sql: String): PreparedStatementApi { // We must return values from upsert because returned id could be different depending on insert or upsert happened if (!currentDialect.supportsOnlyIdentifiersInGeneratedKeys) { diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt index b78942ffc4..82aec9a53e 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt @@ -719,12 +719,14 @@ abstract class FunctionProvider { append("T.$columnName=S.$columnName") } - +" WHEN MATCHED THEN UPDATE SET " - onUpdate.appendTo { (columnToUpdate, updateExpression) -> - append("T.${transaction.identity(columnToUpdate)}=") - when (updateExpression) { - is QueryParameter<*>, !is Expression<*> -> registerArgument(columnToUpdate.columnType, updateExpression) - else -> append(updateExpression.toString().replace("$tableIdentifier.", "T.")) + if (onUpdate.isNotEmpty()) { + +" WHEN MATCHED THEN UPDATE SET " + onUpdate.appendTo { (columnToUpdate, updateExpression) -> + append("T.${transaction.identity(columnToUpdate)}=") + when (updateExpression) { + is QueryParameter<*>, !is Expression<*> -> registerArgument(columnToUpdate.columnType, updateExpression) + else -> append(updateExpression.toString().replace("$tableIdentifier.", "T.")) + } } } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt index f6862701ae..16c10c25f9 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt @@ -362,13 +362,18 @@ open class MysqlDialect : VendorDialect(dialectName, MysqlDataTypeProvider, Mysq /** Returns `true` if a MySQL JDBC connector is being used and its version is greater than or equal to 8.0. */ fun isTimeZoneOffsetSupported(): Boolean = (currentDialect !is MariaDBDialect) && isMysql8 + private val notAcceptableDefaults = mutableListOf("CURRENT_DATE()", "CURRENT_DATE") + override fun isAllowedAsColumnDefault(e: Expression<*>): Boolean { if (super.isAllowedAsColumnDefault(e)) return true if ((currentDialect is MariaDBDialect && fullVersion >= "10.2.1") || (currentDialect !is MariaDBDialect && fullVersion >= "8.0.13")) { return true } - val acceptableDefaults = mutableListOf("CURRENT_TIMESTAMP", "CURRENT_TIMESTAMP()", "NOW()", "CURRENT_TIMESTAMP(6)", "NOW(6)") - return e.toString().trim() in acceptableDefaults && isFractionDateTimeSupported() + + // This check is quite optimistic, it will not allow to create a varchar columns with "CURRENT_DATE" default value for example + // Comparing to the previous variant with white list of functions the new variant does not reject valid values, + // it could be checked on the test UpsertTests::testUpsertWithColumnExpressions() + return e.toString().trim() !in notAcceptableDefaults } override fun fillConstraintCacheForTables(tables: List) { diff --git a/exposed-java-time/src/test/kotlin/org/jetbrains/exposed/DefaultsTest.kt b/exposed-java-time/src/test/kotlin/org/jetbrains/exposed/DefaultsTest.kt index 9db9d609af..2ed097d4af 100644 --- a/exposed-java-time/src/test/kotlin/org/jetbrains/exposed/DefaultsTest.kt +++ b/exposed-java-time/src/test/kotlin/org/jetbrains/exposed/DefaultsTest.kt @@ -320,7 +320,8 @@ class DefaultsTest : DatabaseTestsBase() { val defaultInt = integer("defaultInteger").defaultExpression(abs(-100)) } - withTables(foo) { + // MySql 5 is excluded because it does not support `CURRENT_DATE()` as a default value + withTables(excludeSettings = listOf(TestDB.MYSQL_V5), foo) { val id = foo.insertAndGetId { it[foo.name] = "bar" } diff --git a/exposed-jodatime/src/test/kotlin/org/jetbrains/exposed/JodaTimeDefaultsTest.kt b/exposed-jodatime/src/test/kotlin/org/jetbrains/exposed/JodaTimeDefaultsTest.kt index d2fd169eb8..407cda1494 100644 --- a/exposed-jodatime/src/test/kotlin/org/jetbrains/exposed/JodaTimeDefaultsTest.kt +++ b/exposed-jodatime/src/test/kotlin/org/jetbrains/exposed/JodaTimeDefaultsTest.kt @@ -82,7 +82,8 @@ class JodaTimeDefaultsTest : DatabaseTestsBase() { @Test fun testDefaultsWithExplicit02() { - withTables(TableWithDBDefault) { + // MySql 5 is excluded because it does not support `CURRENT_DATE()` as a default value + withTables(excludeSettings = listOf(TestDB.MYSQL_V5), TableWithDBDefault) { val created = listOf( DBDefault.new { field = "2" @@ -255,7 +256,8 @@ class JodaTimeDefaultsTest : DatabaseTestsBase() { val defaultInt = integer("defaultInteger").defaultExpression(abs(-100)) } - withTables(foo) { + // MySql 5 is excluded because it does not support `CURRENT_DATE()` as a default value + withTables(excludeSettings = listOf(TestDB.MYSQL_V5), foo) { val id = foo.insertAndGetId { it[foo.name] = "bar" } diff --git a/exposed-kotlin-datetime/src/test/kotlin/org/jetbrains/exposed/sql/kotlin/datetime/DefaultsTest.kt b/exposed-kotlin-datetime/src/test/kotlin/org/jetbrains/exposed/sql/kotlin/datetime/DefaultsTest.kt index 91c4db471c..22dcbf87e0 100644 --- a/exposed-kotlin-datetime/src/test/kotlin/org/jetbrains/exposed/sql/kotlin/datetime/DefaultsTest.kt +++ b/exposed-kotlin-datetime/src/test/kotlin/org/jetbrains/exposed/sql/kotlin/datetime/DefaultsTest.kt @@ -119,7 +119,8 @@ class DefaultsTest : DatabaseTestsBase() { @Test fun testDefaultsWithExplicit02() { - withTables(TableWithDBDefault) { + // MySql 5 is excluded because it does not support `CURRENT_DATE()` as a default value + withTables(excludeSettings = listOf(TestDB.MYSQL_V5), TableWithDBDefault) { val created = listOf( DBDefault.new { field = "2" @@ -317,7 +318,8 @@ class DefaultsTest : DatabaseTestsBase() { val defaultInt = integer("defaultInteger").defaultExpression(abs(-100)) } - withTables(foo) { + // MySql 5 is excluded because it does not support `CURRENT_DATE()` as a default value + withTables(excludeSettings = listOf(TestDB.MYSQL_V5), foo) { val id = foo.insertAndGetId { it[foo.name] = "bar" } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt index b9d7f27580..05b222cf3c 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/CreateMissingTablesAndColumnsTests.kt @@ -366,7 +366,7 @@ class CreateMissingTablesAndColumnsTests : DatabaseTestsBase() { actual.forEach { exec(it) } } else { SchemaUtils.drop(whiteSpaceTable) - SchemaUtils.create(whiteSpaceTable) + SchemaUtils.create(emptyTable) } val emptyId = emptyTable.insertAndGetId { } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt index aa1a4f541e..c2ae367b87 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt @@ -750,4 +750,42 @@ class InsertTests : DatabaseTestsBase() { assertNotNull(result[tester.id]) } } + + @Test + fun testDefaultValuesAndNullableColumnsNotInBatchInsertArguments() { + val tester = object : IntIdTable("test_batch_insert_defaults") { + val number = integer("number") + val default = varchar("default", 128).default("default") + val defaultExpression = varchar("defaultExpression", 128).defaultExpression(stringLiteral("defaultExpression")) + val nullable = varchar("nullable", 128).nullable() + val nullableDefaultNull = varchar("nullableDefaultNull", 128).nullable().default(null) + val nullableDefaultNotNull = varchar("nullableDefaultNotNull", 128).nullable().default("nullableDefaultNotNull") + val databaseGenerated = integer("databaseGenerated").withDefinition("DEFAULT 1").databaseGenerated() + } + + val testerWithFakeDefaults = object : IntIdTable("test_batch_insert_defaults") { + val number = integer("number") + val default = varchar("default", 128).default("default-fake") + val defaultExpression = varchar("defaultExpression", 128).defaultExpression(stringLiteral("defaultExpression-fake")) + val nullable = varchar("nullable", 128).nullable().default("null-fake") + val nullableDefaultNull = varchar("nullableDefaultNull", 128).nullable().default("null-fake") + val nullableDefaultNotNull = varchar("nullableDefaultNotNull", 128).nullable().default("nullableDefaultNotNull-fake") + val databaseGenerated = integer("databaseGenerated").default(-1) + } + + withTables(tester) { + testerWithFakeDefaults.batchInsert(listOf(1, 2, 3)) { + this[testerWithFakeDefaults.number] = 10 + } + + testerWithFakeDefaults.selectAll().forEach { + assertEquals("default", it[testerWithFakeDefaults.default]) + assertEquals("defaultExpression", it[testerWithFakeDefaults.defaultExpression]) + assertEquals(null, it[testerWithFakeDefaults.nullable]) + assertEquals(null, it[testerWithFakeDefaults.nullableDefaultNull]) + assertEquals("nullableDefaultNotNull", it[testerWithFakeDefaults.nullableDefaultNotNull]) + assertEquals(1, it[testerWithFakeDefaults.databaseGenerated]) + } + } + } } 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 1611be915c..ea41c6b501 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 @@ -2,7 +2,6 @@ package org.jetbrains.exposed.sql.tests.shared.dml import org.jetbrains.exposed.dao.id.IntIdTable import org.jetbrains.exposed.dao.id.UUIDTable -import org.jetbrains.exposed.exceptions.ExposedSQLException import org.jetbrains.exposed.exceptions.UnsupportedByDialectException import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.SqlExpressionBuilder.concat @@ -109,24 +108,16 @@ class UpsertTests : DatabaseTestsBase() { withTables(excludeSettings = TestDB.ALL_H2_V1, tester) { testDb -> val primaryKeyValues = Pair("User A", "Key A") - if (testDb == TestDB.ORACLE) { - // Oracle explicitly prohibits using key columns in update clause - // throws 'ORA-38104: Columns referenced in the ON Clause cannot be updated' - expectException { - upsertOnlyKeyColumns(primaryKeyValues) - } - } else { - // insert new row - upsertOnlyKeyColumns(primaryKeyValues) - // 'update' existing row to have identical values - upsertOnlyKeyColumns(primaryKeyValues) + // insert new row + upsertOnlyKeyColumns(primaryKeyValues) + // 'update' existing row to have identical values + upsertOnlyKeyColumns(primaryKeyValues) - val result = tester.selectAll().singleOrNull() - assertNotNull(result) + val result = tester.selectAll().singleOrNull() + assertNotNull(result) - val resultValues = Pair(result[tester.userId], result[tester.keyId]) - assertEquals(primaryKeyValues, resultValues) - } + val resultValues = Pair(result[tester.userId], result[tester.keyId]) + assertEquals(primaryKeyValues, resultValues) } } @@ -289,6 +280,10 @@ class UpsertTests : DatabaseTestsBase() { it[item] = "Item B" it[gains] = 200 it[losses] = 0 + // `amount` must be passed explicitly now due to usage of that column inside the custom onUpdate statement + // There is an option to call `tester.amount.defaultValueFun?.let { it() }!!`, + // it looks ugly but prevents regression on changes in default value + it[amount] = 25 } val insertResult = tester.selectAll().where { tester.item neq itemA }.single() @@ -778,4 +773,42 @@ class UpsertTests : DatabaseTestsBase() { val word = varchar("name", 64).uniqueIndex() val count = integer("count").default(1) } + + @Test + fun testDefaultValuesAndNullableColumnsNotInArguments() { + val tester = object : UUIDTable("test_batch_insert_defaults") { + val number = integer("number") + val default = varchar("default", 128).default("default") + val defaultExpression = varchar("defaultExpression", 128).defaultExpression(stringLiteral("defaultExpression")) + val nullable = varchar("nullable", 128).nullable() + val nullableDefaultNull = varchar("nullableDefaultNull", 128).nullable().default(null) + val nullableDefaultNotNull = varchar("nullableDefaultNotNull", 128).nullable().default("nullableDefaultNotNull") + val databaseGenerated = integer("databaseGenerated").withDefinition("DEFAULT 1").databaseGenerated() + } + + val testerWithFakeDefaults = object : UUIDTable("test_batch_insert_defaults") { + val number = integer("number") + val default = varchar("default", 128).default("default-fake") + val defaultExpression = varchar("defaultExpression", 128).defaultExpression(stringLiteral("defaultExpression-fake")) + val nullable = varchar("nullable", 128).nullable().default("null-fake") + val nullableDefaultNull = varchar("nullableDefaultNull", 128).nullable().default("null-fake") + val nullableDefaultNotNull = varchar("nullableDefaultNotNull", 128).nullable().default("nullableDefaultNotNull-fake") + val databaseGenerated = integer("databaseGenerated").default(-1) + } + + withTables(excludeSettings = listOf(TestDB.H2_V1), tester) { + testerWithFakeDefaults.batchUpsert(listOf(1, 2, 3)) { + this[testerWithFakeDefaults.number] = 10 + } + + testerWithFakeDefaults.selectAll().forEach { + assertEquals("default", it[testerWithFakeDefaults.default]) + assertEquals("defaultExpression", it[testerWithFakeDefaults.defaultExpression]) + assertEquals(null, it[testerWithFakeDefaults.nullable]) + assertEquals(null, it[testerWithFakeDefaults.nullableDefaultNull]) + assertEquals("nullableDefaultNotNull", it[testerWithFakeDefaults.nullableDefaultNotNull]) + assertEquals(1, it[testerWithFakeDefaults.databaseGenerated]) + } + } + } } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/functions/MathFunctionTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/functions/MathFunctionTests.kt index 15ccacf6a9..a8262ce176 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/functions/MathFunctionTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/functions/MathFunctionTests.kt @@ -172,7 +172,8 @@ class MathFunctionTests : FunctionsTestBase() { } // MySQL and MariaDB are the only supported databases that allow referencing another column in a default expression - withTables(excludeSettings = TestDB.ALL - TestDB.ALL_MYSQL_MARIADB, foo) { + // MySQL 5 does not support functions in default values. + withTables(excludeSettings = TestDB.ALL - TestDB.ALL_MYSQL_MARIADB + TestDB.MYSQL_V5, foo) { val id = foo.insertAndGetId { it[foo.integer] = -100 it[foo.double] = 100.70