fix: EXPOSED-393 H2 upsert with JSON column creates invalid data

exposed-core/api/exposed-core.api

@@ -202,6 +202,7 @@ public final class org/jetbrains/exposed/sql/AutoIncColumnType : org/jetbrains/e
 	public fun nonNullValueAsDefaultString (Ljava/lang/Object;)Ljava/lang/String;
 	public fun nonNullValueToString (Ljava/lang/Object;)Ljava/lang/String;
 	public fun notNullValueToDB (Ljava/lang/Object;)Ljava/lang/Object;
+	public fun parameterMarker (Ljava/lang/Object;)Ljava/lang/String;
 	public fun readObject (Ljava/sql/ResultSet;I)Ljava/lang/Object;
 	public fun setNullable (Z)V
 	public fun setParameter (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;ILjava/lang/Object;)V
@@ -467,6 +468,7 @@ public abstract class org/jetbrains/exposed/sql/ColumnType : org/jetbrains/expos
 	public fun nonNullValueAsDefaultString (Ljava/lang/Object;)Ljava/lang/String;
 	public fun nonNullValueToString (Ljava/lang/Object;)Ljava/lang/String;
 	public fun notNullValueToDB (Ljava/lang/Object;)Ljava/lang/Object;
+	public fun parameterMarker (Ljava/lang/Object;)Ljava/lang/String;
 	public fun readObject (Ljava/sql/ResultSet;I)Ljava/lang/Object;
 	public fun setNullable (Z)V
 	public fun setParameter (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;ILjava/lang/Object;)V
@@ -977,6 +979,7 @@ public abstract interface class org/jetbrains/exposed/sql/IColumnType {
 	public abstract fun nonNullValueAsDefaultString (Ljava/lang/Object;)Ljava/lang/String;
 	public abstract fun nonNullValueToString (Ljava/lang/Object;)Ljava/lang/String;
 	public abstract fun notNullValueToDB (Ljava/lang/Object;)Ljava/lang/Object;
+	public abstract fun parameterMarker (Ljava/lang/Object;)Ljava/lang/String;
 	public abstract fun readObject (Ljava/sql/ResultSet;I)Ljava/lang/Object;
 	public abstract fun setNullable (Z)V
 	public abstract fun setParameter (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;ILjava/lang/Object;)V
@@ -992,6 +995,7 @@ public final class org/jetbrains/exposed/sql/IColumnType$DefaultImpls {
 	public static fun nonNullValueAsDefaultString (Lorg/jetbrains/exposed/sql/IColumnType;Ljava/lang/Object;)Ljava/lang/String;
 	public static fun nonNullValueToString (Lorg/jetbrains/exposed/sql/IColumnType;Ljava/lang/Object;)Ljava/lang/String;
 	public static fun notNullValueToDB (Lorg/jetbrains/exposed/sql/IColumnType;Ljava/lang/Object;)Ljava/lang/Object;
+	public static fun parameterMarker (Lorg/jetbrains/exposed/sql/IColumnType;Ljava/lang/Object;)Ljava/lang/String;
 	public static fun readObject (Lorg/jetbrains/exposed/sql/IColumnType;Ljava/sql/ResultSet;I)Ljava/lang/Object;
 	public static fun setParameter (Lorg/jetbrains/exposed/sql/IColumnType;Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;ILjava/lang/Object;)V
 	public static fun validateValueBeforeUpdate (Lorg/jetbrains/exposed/sql/IColumnType;Ljava/lang/Object;)V

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/ColumnType.kt

@@ -22,6 +22,7 @@ import kotlin.reflect.full.isSubclassOf
 /**
  * Interface common to all column types.
  */
+@Suppress("TooManyFunctions")
 interface IColumnType<T> {
     /** Returns `true` if the column type is nullable, `false` otherwise. */
     var nullable: Boolean
@@ -95,6 +96,16 @@ interface IColumnType<T> {
      * */
     @Throws(IllegalArgumentException::class)
     fun validateValueBeforeUpdate(value: T?) {}
+
+    /**
+     * Defines the appearance of parameter markers in prepared SQL statements.
+     *
+     * The default parameter marker is `?`, but it can be overridden in specific cases.
+     *
+     * For example, H2 uses `? FORMAT JSON` for JSON columns,
+     * in Postgres a parameter marker can be explicitly cast to a specific type, etc.
+     */
+    fun parameterMarker(value: T?) = "?"
 }
 
 /**

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Expression.kt

@@ -80,28 +80,30 @@ class QueryBuilder(
 
     /** Adds the specified sequence of [arguments] as values of the specified [sqlType]. */
     fun <T> registerArguments(sqlType: IColumnType<*>, arguments: Iterable<T>) {
+        val sqlTypeT = (sqlType as IColumnType<T>)
+
+        // avoid potentially expensive valueToString call unless we need to sort values
         if (arguments is Collection && arguments.size <= 1) {
-            // avoid potentially expensive valueToString call unless we need to sort values
             arguments.forEach {
                 if (prepared) {
                     _args.add(sqlType to it)
-                    append("?")
+                    append(sqlTypeT.parameterMarker(it))
                 } else {
-                    append((sqlType as IColumnType<T>).valueToString(it))
+                    append(sqlTypeT.valueToString(it))
                 }
             }
         } else {
             fun toString(value: T) = when {
                 prepared && value is String -> value
-                else -> (sqlType as IColumnType<T>).valueToString(value)
+                else -> sqlTypeT.valueToString(value)
             }
 
             arguments.map { it to toString(it) }
                 .sortedBy { it.second }
                 .appendTo {
                     if (prepared) {
                         _args.add(sqlType to it.first)
-                        append("?")
+                        append(sqlTypeT.parameterMarker(it.first))
                     } else {
                         append(it.second)
                     }

exposed-json/api/exposed-json.api

@@ -42,6 +42,7 @@ public class org/jetbrains/exposed/sql/json/JsonColumnType : org/jetbrains/expos
 	public fun getUsesBinaryFormat ()Z
 	public fun nonNullValueToString (Ljava/lang/Object;)Ljava/lang/String;
 	public fun notNullValueToDB (Ljava/lang/Object;)Ljava/lang/Object;
+	public fun parameterMarker (Ljava/lang/Object;)Ljava/lang/String;
 	public fun setParameter (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;ILjava/lang/Object;)V
 	public fun sqlType ()Ljava/lang/String;
 	public fun valueFromDB (Ljava/lang/Object;)Ljava/lang/Object;

exposed-json/src/main/kotlin/org/jetbrains/exposed/sql/json/JsonColumnType.kt

@@ -38,6 +38,12 @@ open class JsonColumnType<T : Any>(
         }
     }
 
+    override fun parameterMarker(value: T?): String = if (currentDialect is H2Dialect && value != null) {
+        "? FORMAT JSON"
+    } else {
+        super.parameterMarker(value)
+    }
+
     override fun notNullValueToDB(value: T): Any = serialize(value)
 
     override fun valueToString(value: T?): String = when (value) {

exposed-json/src/test/kotlin/org/jetbrains/exposed/sql/json/JsonBColumnTests.kt

@@ -24,7 +24,7 @@ import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 
 class JsonBColumnTests : DatabaseTestsBase() {
-    private val binaryJsonNotSupportedDB = listOf(TestDB.SQLITE, TestDB.SQLSERVER) + TestDB.ORACLE
+    private val binaryJsonNotSupportedDB = listOf(TestDB.SQLITE, TestDB.SQLSERVER, TestDB.ORACLE)
 
     @Test
     fun testInsertAndSelect() {
@@ -97,36 +97,30 @@ class JsonBColumnTests : DatabaseTestsBase() {
         val dataTable = JsonTestsData.JsonBTable
         val dataEntity = JsonTestsData.JsonBEntity
 
-        withDb(excludeSettings = binaryJsonNotSupportedDB) { testDb ->
-            excludingH2Version1(testDb) {
-                SchemaUtils.create(dataTable)
+        withTables(excludeSettings = binaryJsonNotSupportedDB, dataTable) { testDb ->
+            val dataA = DataHolder(User("Admin", "Alpha"), 10, true, null)
+            val newUser = dataEntity.new {
+                jsonBColumn = dataA
+            }
 
-                val dataA = DataHolder(User("Admin", "Alpha"), 10, true, null)
-                val newUser = dataEntity.new {
-                    jsonBColumn = dataA
-                }
+            assertEquals(dataA, dataEntity.findById(newUser.id)?.jsonBColumn)
 
-                assertEquals(dataA, dataEntity.findById(newUser.id)?.jsonBColumn)
+            val updatedUser = dataA.copy(logins = 99)
+            dataTable.update {
+                it[jsonBColumn] = updatedUser
+            }
 
-                val updatedUser = dataA.copy(logins = 99)
-                dataTable.update {
-                    it[jsonBColumn] = updatedUser
-                }
+            assertEquals(updatedUser, dataEntity.all().single().jsonBColumn)
 
-                assertEquals(updatedUser, dataEntity.all().single().jsonBColumn)
-
-                if (testDb !in TestDB.allH2TestDB) {
-                    dataEntity.new { jsonBColumn = dataA }
-                    val loginCount = if (currentDialectTest is PostgreSQLDialect) {
-                        dataTable.jsonBColumn.extract<Int>("logins").castTo(IntegerColumnType())
-                    } else {
-                        dataTable.jsonBColumn.extract<Int>(".logins")
-                    }
-                    val frequentUser = dataEntity.find { loginCount greaterEq 50 }.single()
-                    assertEquals(updatedUser, frequentUser.jsonBColumn)
+            if (testDb !in TestDB.allH2TestDB) {
+                dataEntity.new { jsonBColumn = dataA }
+                val loginCount = if (currentDialectTest is PostgreSQLDialect) {
+                    dataTable.jsonBColumn.extract<Int>("logins").castTo(IntegerColumnType())
+                } else {
+                    dataTable.jsonBColumn.extract<Int>(".logins")
                 }
-
-                SchemaUtils.drop(dataTable)
+                val frequentUser = dataEntity.find { loginCount greaterEq 50 }.single()
+                assertEquals(updatedUser, frequentUser.jsonBColumn)
             }
         }
     }
@@ -244,30 +238,28 @@ class JsonBColumnTests : DatabaseTestsBase() {
         }
 
         withDb(excludeSettings = binaryJsonNotSupportedDB) { testDb ->
-            excludingH2Version1(testDb) {
-                if (isOldMySql()) {
-                    expectException<UnsupportedByDialectException> {
-                        SchemaUtils.createMissingTablesAndColumns(defaultTester)
-                    }
-                } else {
+            if (isOldMySql()) {
+                expectException<UnsupportedByDialectException> {
                     SchemaUtils.createMissingTablesAndColumns(defaultTester)
-                    assertTrue(defaultTester.exists())
-                    // ensure defaults match returned metadata defaults
-                    val alters = SchemaUtils.statementsRequiredToActualizeScheme(defaultTester)
-                    assertTrue(alters.isEmpty())
-
-                    defaultTester.insert {}
+                }
+            } else {
+                SchemaUtils.createMissingTablesAndColumns(defaultTester)
+                assertTrue(defaultTester.exists())
+                // ensure defaults match returned metadata defaults
+                val alters = SchemaUtils.statementsRequiredToActualizeScheme(defaultTester)
+                assertTrue(alters.isEmpty())
 
-                    defaultTester.selectAll().single().also {
-                        assertEquals(defaultUser.name, it[defaultTester.user1].name)
-                        assertEquals(defaultUser.team, it[defaultTester.user1].team)
+                defaultTester.insert {}
 
-                        assertEquals(defaultUser.name, it[defaultTester.user2].name)
-                        assertEquals(defaultUser.team, it[defaultTester.user2].team)
-                    }
+                defaultTester.selectAll().single().also {
+                    assertEquals(defaultUser.name, it[defaultTester.user1].name)
+                    assertEquals(defaultUser.team, it[defaultTester.user1].team)
 
-                    SchemaUtils.drop(defaultTester)
+                    assertEquals(defaultUser.name, it[defaultTester.user2].name)
+                    assertEquals(defaultUser.team, it[defaultTester.user2].team)
                 }
+
+                SchemaUtils.drop(defaultTester)
             }
         }
     }
@@ -282,31 +274,26 @@ class JsonBColumnTests : DatabaseTestsBase() {
             val intArray = jsonb<IntArray>("int_array", Json.Default)
         }
 
-        withDb(excludeSettings = binaryJsonNotSupportedDB) { testDb ->
-            excludingH2Version1(testDb) {
-                // the logger is left in to test that it does not throw ClassCastException on insertion of iterables
-                addLogger(StdOutSqlLogger)
-                SchemaUtils.create(iterables)
-
-                val user1 = User("A", "Team A")
-                val user2 = User("B", "Team B")
-                val integerList = listOf(1, 2, 3)
-                val integerArray = intArrayOf(1, 2, 3)
-                iterables.insert {
-                    it[userList] = listOf(user1, user2)
-                    it[intList] = integerList
-                    it[userArray] = arrayOf(user1, user2)
-                    it[intArray] = integerArray
-                }
-
-                val result = iterables.selectAll().single()
-                assertEqualCollections(listOf(user1, user2), result[iterables.userList])
-                assertEqualCollections(integerList, result[iterables.intList])
-                assertContentEquals(arrayOf(user1, user2), result[iterables.userArray])
-                assertContentEquals(integerArray, result[iterables.intArray])
-
-                SchemaUtils.drop(iterables)
+        withTables(excludeSettings = binaryJsonNotSupportedDB, iterables) { testDb ->
+            // the logger is left in to test that it does not throw ClassCastException on insertion of iterables
+            addLogger(StdOutSqlLogger)
+
+            val user1 = User("A", "Team A")
+            val user2 = User("B", "Team B")
+            val integerList = listOf(1, 2, 3)
+            val integerArray = intArrayOf(1, 2, 3)
+            iterables.insert {
+                it[userList] = listOf(user1, user2)
+                it[intList] = integerList
+                it[userArray] = arrayOf(user1, user2)
+                it[intArray] = integerArray
             }
+
+            val result = iterables.selectAll().single()
+            assertEqualCollections(listOf(user1, user2), result[iterables.userList])
+            assertEqualCollections(integerList, result[iterables.intList])
+            assertContentEquals(arrayOf(user1, user2), result[iterables.userArray])
+            assertContentEquals(integerArray, result[iterables.intArray])
         }
     }
 
@@ -316,25 +303,38 @@ class JsonBColumnTests : DatabaseTestsBase() {
             val user = jsonb<User>("user", Json.Default).nullable()
         }
 
-        withDb(excludeSettings = binaryJsonNotSupportedDB) { testDb ->
-            excludingH2Version1(testDb) {
-                SchemaUtils.create(tester)
+        withTables(excludeSettings = binaryJsonNotSupportedDB, tester) { testDb ->
+            val nullId = tester.insertAndGetId {
+                it[user] = null
+            }
+            val nonNullId = tester.insertAndGetId {
+                it[user] = User("A", "Team A")
+            }
 
-                val nullId = tester.insertAndGetId {
-                    it[user] = null
-                }
-                val nonNullId = tester.insertAndGetId {
-                    it[user] = User("A", "Team A")
-                }
+            val result1 = tester.select(tester.user).where { tester.id eq nullId }.single()
+            assertNull(result1[tester.user])
 
-                val result1 = tester.select(tester.user).where { tester.id eq nullId }.single()
-                assertNull(result1[tester.user])
+            val result2 = tester.select(tester.user).where { tester.id eq nonNullId }.single()
+            assertNotNull(result2[tester.user])
+        }
+    }
 
-                val result2 = tester.select(tester.user).where { tester.id eq nonNullId }.single()
-                assertNotNull(result2[tester.user])
+    @Test
+    fun testJsonBWithUpsert() {
+        withJsonBTable(exclude = binaryJsonNotSupportedDB) { tester, _, _, db ->
+            val newData = DataHolder(User("Pro", "Alpha"), 999, true, "A")
+            val newId = tester.insertAndGetId {
+                it[jsonBColumn] = newData
+            }
 
-                SchemaUtils.drop(tester)
+            val newData2 = newData.copy(active = false)
+            tester.upsert {
+                it[tester.id] = newId
+                it[tester.jsonBColumn] = newData2
             }
+
+            val newResult = tester.selectAll().where { tester.id eq newId }.singleOrNull()
+            assertEquals(newData2, newResult?.get(tester.jsonBColumn))
         }
     }
 }