From 0daf194ba1e1c3e26c2e4792b18026a11b834034 Mon Sep 17 00:00:00 2001 From: bog-walk <82039410+bog-walk@users.noreply.github.com> Date: Fri, 4 Aug 2023 07:55:20 -0400 Subject: [PATCH] fix: EXPOSED-111 Allow check constraint statements in MySQL8 (#1817) All databases can have column check constraints included when a table is created, but MySQL is prevented from creating/dropping its constraints using ALTER TABLE. This makes sense given that older MySQL versions simply parsed and ignored any CHECK constraint in the CREATE TABLE statement. But MySQL has bully supported check constraints since version 8.0.16, so this limitation is now removed from newer versions. Additional: - 2 tests that misleading seemed to do nothing were more appropriately named to describe their purpose. - Refactor some tests to remove detekt errors. --- .../org/jetbrains/exposed/sql/Constraints.kt | 18 ++- .../exposed/sql/tests/shared/DDLTests.kt | 105 ++++++++++++------ .../sql/tests/shared/dml/InsertTests.kt | 26 +++-- 3 files changed, 96 insertions(+), 53 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt index 415f844b0d..a7086cfe71 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Constraints.kt @@ -1,13 +1,8 @@ package org.jetbrains.exposed.sql import org.jetbrains.exposed.sql.transactions.TransactionManager -import org.jetbrains.exposed.sql.vendors.H2Dialect -import org.jetbrains.exposed.sql.vendors.MariaDBDialect -import org.jetbrains.exposed.sql.vendors.MysqlDialect -import org.jetbrains.exposed.sql.vendors.OracleDialect -import org.jetbrains.exposed.sql.vendors.currentDialect +import org.jetbrains.exposed.sql.vendors.* import org.jetbrains.exposed.sql.vendors.currentDialectIfAvailable -import org.jetbrains.exposed.sql.vendors.h2Mode import org.jetbrains.exposed.sql.vendors.inProperCase import java.sql.DatabaseMetaData @@ -196,9 +191,12 @@ data class CheckConstraint( internal val checkPart = "CONSTRAINT $checkName CHECK ($checkOp)" + private val DatabaseDialect.cannotAlterCheckConstraint: Boolean + get() = this is SQLiteDialect || (this as? MysqlDialect)?.isMysql8 == false + override fun createStatement(): List { - return if (currentDialect is MysqlDialect) { - exposedLogger.warn("Creation of CHECK constraints is not currently supported by MySQL") + return if (currentDialect.cannotAlterCheckConstraint) { + exposedLogger.warn("Creation of CHECK constraints is not currently supported by ${currentDialect.name}") listOf() } else { listOf("ALTER TABLE $tableName ADD $checkPart") @@ -208,8 +206,8 @@ data class CheckConstraint( override fun modifyStatement(): List = dropStatement() + createStatement() override fun dropStatement(): List { - return if (currentDialect is MysqlDialect) { - exposedLogger.warn("Deletion of CHECK constraints is not currently supported by MySQL") + return if (currentDialect.cannotAlterCheckConstraint) { + exposedLogger.warn("Deletion of CHECK constraints is not currently supported by ${currentDialect.name}") listOf() } else { listOf("ALTER TABLE $tableName DROP CONSTRAINT $checkName") diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/DDLTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/DDLTests.kt index 273efff218..b61cda8fbd 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/DDLTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/DDLTests.kt @@ -787,25 +787,27 @@ class DDLTests : DatabaseTestsBase() { val negative = integer("negative").check("subZero") { it less 0 } } - withTables(listOf(TestDB.MYSQL), checkTable) { - checkTable.insert { - it[positive] = 42 - it[negative] = -14 - } + withTables(checkTable) { + if (!isOldMySql()) { + checkTable.insert { + it[positive] = 42 + it[negative] = -14 + } - assertEquals(1L, checkTable.selectAll().count()) + assertEquals(1L, checkTable.selectAll().count()) - assertFailAndRollback("Check constraint 1") { - checkTable.insert { - it[positive] = -472 - it[negative] = -354 + assertFailAndRollback("Check constraint 1") { + checkTable.insert { + it[positive] = -472 + it[negative] = -354 + } } - } - assertFailAndRollback("Check constraint 2") { - checkTable.insert { - it[positive] = 538 - it[negative] = 915 + assertFailAndRollback("Check constraint 2") { + checkTable.insert { + it[positive] = 538 + it[negative] = 915 + } } } } @@ -821,32 +823,73 @@ class DDLTests : DatabaseTestsBase() { } } - withTables(listOf(TestDB.MYSQL), checkTable) { - checkTable.insert { - it[positive] = 57 - it[negative] = -32 - } + withTables(checkTable) { + if (!isOldMySql()) { + checkTable.insert { + it[positive] = 57 + it[negative] = -32 + } - assertEquals(1L, checkTable.selectAll().count()) + assertEquals(1L, checkTable.selectAll().count()) - assertFailAndRollback("Check constraint 1") { - checkTable.insert { - it[positive] = -47 - it[negative] = -35 + assertFailAndRollback("Check constraint 1") { + checkTable.insert { + it[positive] = -47 + it[negative] = -35 + } + } + + assertFailAndRollback("Check constraint 2") { + checkTable.insert { + it[positive] = 53 + it[negative] = 91 + } } } + } + } - assertFailAndRollback("Check constraint 2") { - checkTable.insert { - it[positive] = 53 - it[negative] = 91 + @Test + fun testCreateAndDropCheckConstraint() { + val tester = object : Table("tester") { + val amount = integer("amount") + } + + withTables(tester) { testDb -> + val constraintName = "check_tester_positive" + val constraintOp = "${"amount".inProperCase()} > 0" + val (createConstraint, dropConstraint) = CheckConstraint("tester", constraintName, constraintOp).run { + createStatement() to dropStatement() + } + + if (testDb == TestDB.SQLITE || isOldMySql()) { // cannot alter existing check constraint + assertTrue(createConstraint.isEmpty() && dropConstraint.isEmpty()) + } else { + val negative = -9 + tester.insert { it[amount] = negative } + + // fails to create check constraint because negative values already stored + assertFailAndRollback("Check constraint violation") { + exec(createConstraint.single()) + } + + tester.deleteAll() + exec(createConstraint.single()) + + assertFailAndRollback("Check constraint violation") { + tester.insert { it[amount] = negative } } + + exec(dropConstraint.single()) + + tester.insert { it[amount] = negative } + assertEquals(negative, tester.selectAll().single()[tester.amount]) } } } @Test - fun testCheckConstraint03() { + fun testEqOperatorWithoutDBConnection() { object : Table("test") { val testColumn: Column = integer("test_column").nullable() @@ -859,7 +902,7 @@ class DDLTests : DatabaseTestsBase() { } @Test - fun testCheckConstraint04() { + fun testNeqOperatorWithoutDBConnection() { object : Table("test") { val testColumn: Column = integer("test_column").nullable() 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 2b81d1b798..a43c1dca35 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 @@ -461,8 +461,9 @@ class InsertTests : DatabaseTestsBase() { } } - @Test fun `rollback on constraint exception normal transactions`() { - val TestTable = object : IntIdTable("TestRollback") { + @Test + fun testRollbackOnConstraintExceptionWithNormalTransactions() { + val testTable = object : IntIdTable("TestRollback") { val foo = integer("foo").check { it greater 0 } } val dbToTest = TestDB.enabledInTests() - setOfNotNull( @@ -474,16 +475,16 @@ class InsertTests : DatabaseTestsBase() { try { try { withDb(db) { - SchemaUtils.create(TestTable) - TestTable.insert { it[foo] = 1 } - TestTable.insert { it[foo] = 0 } + SchemaUtils.create(testTable) + testTable.insert { it[foo] = 1 } + testTable.insert { it[foo] = 0 } } fail("Should fail on constraint > 0 with $db") } catch (_: SQLException) { // expected } withDb(db) { - assertTrue(TestTable.selectAll().empty()) + assertTrue(testTable.selectAll().empty()) } } finally { withDb(db) { @@ -493,8 +494,9 @@ class InsertTests : DatabaseTestsBase() { } } - @Test fun `rollback on constraint exception normal suspended transactions`() { - val TestTable = object : IntIdTable("TestRollback") { + @Test + fun testRollbackOnConstraintExceptionWithSuspendTransactions() { + val testTable = object : IntIdTable("TestRollback") { val foo = integer("foo").check { it greater 0 } } val dbToTest = TestDB.enabledInTests() - setOfNotNull( @@ -506,12 +508,12 @@ class InsertTests : DatabaseTestsBase() { try { try { withDb(db) { - SchemaUtils.create(TestTable) + SchemaUtils.create(testTable) } runBlocking { newSuspendedTransaction(db = db.db) { - TestTable.insert { it[foo] = 1 } - TestTable.insert { it[foo] = 0 } + testTable.insert { it[foo] = 1 } + testTable.insert { it[foo] = 0 } } } fail("Should fail on constraint > 0") @@ -520,7 +522,7 @@ class InsertTests : DatabaseTestsBase() { } withDb(db) { - assertTrue(TestTable.selectAll().empty()) + assertTrue(testTable.selectAll().empty()) } } finally { withDb(db) {