Skip to content
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.

Sign up for GitHub

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

fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK #1797

Merged
merged 4 commits into from
Jul 26, 2023
Merged

fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK #1797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39cc56d
fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK
bog-walk Jul 19, 2023
19c367d
fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK
bog-walk Jul 20, 2023
2609e92
fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK
bog-walk Jul 20, 2023
d7426da
fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK
bog-walk Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions exposed-core/api/exposed-core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2968,6 +2968,7 @@ public abstract class org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMe
public abstract fun cleanCache ()V
public abstract fun columns ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public abstract fun existingIndices ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public abstract fun existingPrimaryKeys ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public abstract fun getCurrentScheme ()Ljava/lang/String;
public final fun getDatabase ()Ljava/lang/String;
public abstract fun getDatabaseDialectName ()Ljava/lang/String;
Expand Down Expand Up @@ -3202,6 +3203,7 @@ public abstract class org/jetbrains/exposed/sql/vendors/DataTypeProvider {

public abstract interface class org/jetbrains/exposed/sql/vendors/DatabaseDialect {
public static final field Companion Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect$Companion;
public abstract fun addPrimaryKey (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;[Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/String;
public abstract fun allTablesNames ()Ljava/util/List;
public abstract fun catalog (Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/String;
public abstract fun checkTableMapping (Lorg/jetbrains/exposed/sql/Table;)Z
Expand All @@ -3213,6 +3215,7 @@ public abstract interface class org/jetbrains/exposed/sql/vendors/DatabaseDialec
public abstract fun dropIndex (Ljava/lang/String;Ljava/lang/String;ZZ)Ljava/lang/String;
public abstract fun dropSchema (Lorg/jetbrains/exposed/sql/Schema;Z)Ljava/lang/String;
public abstract fun existingIndices ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public abstract fun existingPrimaryKeys ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public abstract fun getDataTypeProvider ()Lorg/jetbrains/exposed/sql/vendors/DataTypeProvider;
public abstract fun getDatabase ()Ljava/lang/String;
public abstract fun getDefaultReferenceOption ()Lorg/jetbrains/exposed/sql/ReferenceOption;
Expand Down Expand Up @@ -3256,6 +3259,7 @@ public final class org/jetbrains/exposed/sql/vendors/DatabaseDialect$DefaultImpl
public static fun dropDatabase (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Ljava/lang/String;)Ljava/lang/String;
public static fun dropSchema (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;Z)Ljava/lang/String;
public static fun existingIndices (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;[Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public static fun existingPrimaryKeys (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;[Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public static fun getDefaultReferenceOption (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;)Lorg/jetbrains/exposed/sql/ReferenceOption;
public static fun getLikePatternSpecialChars (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;)Ljava/util/Map;
public static fun getNeedsQuotesWhenSymbolsInNames (Lorg/jetbrains/exposed/sql/vendors/DatabaseDialect;)Z
Expand Down Expand Up @@ -3554,6 +3558,19 @@ public class org/jetbrains/exposed/sql/vendors/PostgreSQLNGDialect : org/jetbrai
public final class org/jetbrains/exposed/sql/vendors/PostgreSQLNGDialect$Companion : org/jetbrains/exposed/sql/vendors/VendorDialect$DialectNameProvider {
}

public final class org/jetbrains/exposed/sql/vendors/PrimaryKeyMetadata {
public fun <init> (Ljava/lang/String;Ljava/util/List;)V
public final fun component1 ()Ljava/lang/String;
public final fun component2 ()Ljava/util/List;
public final fun copy (Ljava/lang/String;Ljava/util/List;)Lorg/jetbrains/exposed/sql/vendors/PrimaryKeyMetadata;
public static synthetic fun copy$default (Lorg/jetbrains/exposed/sql/vendors/PrimaryKeyMetadata;Ljava/lang/String;Ljava/util/List;ILjava/lang/Object;)Lorg/jetbrains/exposed/sql/vendors/PrimaryKeyMetadata;
public fun equals (Ljava/lang/Object;)Z
public final fun getColumnNames ()Ljava/util/List;
public final fun getName ()Ljava/lang/String;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

public class org/jetbrains/exposed/sql/vendors/SQLServerDialect : org/jetbrains/exposed/sql/vendors/VendorDialect {
public static final field Companion Lorg/jetbrains/exposed/sql/vendors/SQLServerDialect$Companion;
public fun <init> ()V
Expand Down Expand Up @@ -3598,6 +3615,7 @@ public final class org/jetbrains/exposed/sql/vendors/SQLiteDialect$Companion : o

public abstract class org/jetbrains/exposed/sql/vendors/VendorDialect : org/jetbrains/exposed/sql/vendors/DatabaseDialect {
public fun <init> (Ljava/lang/String;Lorg/jetbrains/exposed/sql/vendors/DataTypeProvider;Lorg/jetbrains/exposed/sql/vendors/FunctionProvider;)V
public fun addPrimaryKey (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/String;[Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/String;
public fun allTablesNames ()Ljava/util/List;
public fun catalog (Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/String;
public fun checkTableMapping (Lorg/jetbrains/exposed/sql/Table;)Z
Expand All @@ -3610,6 +3628,7 @@ public abstract class org/jetbrains/exposed/sql/vendors/VendorDialect : org/jetb
public fun dropIndex (Ljava/lang/String;Ljava/lang/String;ZZ)Ljava/lang/String;
public fun dropSchema (Lorg/jetbrains/exposed/sql/Schema;Z)Ljava/lang/String;
public fun existingIndices ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
public fun existingPrimaryKeys ([Lorg/jetbrains/exposed/sql/Table;)Ljava/util/Map;
protected fun fillConstraintCacheForTables (Ljava/util/List;)V
public final fun filterCondition (Lorg/jetbrains/exposed/sql/Index;)Ljava/lang/String;
public final fun getAllTablesNames ()Ljava/util/List;
Expand Down
98 changes: 69 additions & 29 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.jetbrains.exposed.sql.transactions.TransactionManager
import org.jetbrains.exposed.sql.vendors.*
import java.math.BigDecimal

@Suppress("TooManyFunctions")
object SchemaUtils {
private inline fun <R> logTimeSpent(message: String, withLogs: Boolean, block: () -> R): R {
return if (withLogs) {
Expand All @@ -19,8 +20,9 @@ object SchemaUtils {

private class TableDepthGraph(val tables: Iterable<Table>) {
val graph = fetchAllTables().let { tables ->
if (tables.isEmpty()) emptyMap()
else {
if (tables.isEmpty()) {
emptyMap()
} else {
tables.associateWith { t ->
t.columns.mapNotNull { c ->
c.referee?.let { it.table to c.columnType.nullable }
Expand Down Expand Up @@ -125,7 +127,9 @@ object SchemaUtils {
)
fun createFKey(reference: Column<*>): List<String> {
val foreignKey = reference.foreignKey
require(foreignKey != null && (foreignKey.deleteRule != null || foreignKey.updateRule != null)) { "$reference does not reference anything" }
require(foreignKey != null && (foreignKey.deleteRule != null || foreignKey.updateRule != null)) {
"$reference does not reference anything"
}
return createFKey(foreignKey)
}

Expand Down Expand Up @@ -196,6 +200,10 @@ object SchemaUtils {
currentDialect.tableColumns(*tables)
}

val existingPrimaryKeys = logTimeSpent("Extracting primary keys", withLogs) {
currentDialect.existingPrimaryKeys(*tables)
}

val dbSupportsAlterTableWithAddColumn = TransactionManager.current().db.supportsAlterTableWithAddColumn

for (table in tables) {
Expand All @@ -222,37 +230,56 @@ object SchemaUtils {
val columnType = col.columnType
val incorrectNullability = existingCol.nullable != columnType.nullable
// Exposed doesn't support changing sequences on columns
val incorrectAutoInc = existingCol.autoIncrement != columnType.isAutoInc && col.autoIncColumnType?.autoincSeq == null
val incorrectDefaults =
existingCol.defaultDbValue != col.dbDefaultValue?.let { dataTypeProvider.dbDefaultToString(col, it) }
val incorrectAutoInc = existingCol.autoIncrement != columnType.isAutoInc &&
col.autoIncColumnType?.autoincSeq == null
val incorrectDefaults = existingCol.defaultDbValue != col.dbDefaultValue?.let {
dataTypeProvider.dbDefaultToString(col, it)
}
val incorrectCaseSensitiveName = existingCol.name.inProperCase() != col.nameInDatabaseCase()
ColumnDiff(incorrectNullability, incorrectAutoInc, incorrectDefaults, incorrectCaseSensitiveName)
}
.filterValues { it.hasDifferences() }

redoColumns.flatMapTo(statements) { (col, changedState) -> col.modifyStatements(changedState) }

// add missing primary key
val missingPK = table.primaryKey?.takeIf { pk -> pk.columns.none { it in missingTableColumns } }
if (missingPK != null && existingPrimaryKeys[table] == null) {
val missingPKName = missingPK.name.takeIf { table.isCustomPKNameDefined() }
statements.add(
currentDialect.addPrimaryKey(table, missingPKName, pkColumns = missingPK.columns)
)
}
}
}

if (dbSupportsAlterTableWithAddColumn) {
val existingColumnConstraint = logTimeSpent("Extracting column constraints", withLogs) {
currentDialect.columnConstraints(*tables)
}
statements.addAll(addMissingColumnConstraints(*tables, withLogs = withLogs))
}

val foreignKeyConstraints = tables.flatMap { table ->
table.foreignKeys.map { it to existingColumnConstraint[table to it.from]?.firstOrNull() }
}
return statements
}

for ((foreignKey, existingConstraint) in foreignKeyConstraints) {
if (existingConstraint == null) {
statements.addAll(createFKey(foreignKey))
} else if (existingConstraint.targetTable != foreignKey.targetTable ||
foreignKey.deleteRule != existingConstraint.deleteRule ||
foreignKey.updateRule != existingConstraint.updateRule
) {
statements.addAll(existingConstraint.dropStatement())
statements.addAll(createFKey(foreignKey))
}
private fun addMissingColumnConstraints(vararg tables: Table, withLogs: Boolean): List<String> {
val existingColumnConstraint = logTimeSpent("Extracting column constraints", withLogs) {
currentDialect.columnConstraints(*tables)
}

val foreignKeyConstraints = tables.flatMap { table ->
table.foreignKeys.map { it to existingColumnConstraint[table to it.from]?.firstOrNull() }
}

val statements = ArrayList<String>()

for ((foreignKey, existingConstraint) in foreignKeyConstraints) {
if (existingConstraint == null) {
statements.addAll(createFKey(foreignKey))
} else if (existingConstraint.targetTable != foreignKey.targetTable ||
foreignKey.deleteRule != existingConstraint.deleteRule ||
foreignKey.updateRule != existingConstraint.updateRule
) {
statements.addAll(existingConstraint.dropStatement())
statements.addAll(createFKey(foreignKey))
}
}

Expand Down Expand Up @@ -300,7 +327,9 @@ object SchemaUtils {
"${currentDialect.name} requires autoCommit to be enabled for CREATE DATABASE",
exception
)
} else throw exception
} else {
throw exception
}
}
}

Expand All @@ -327,7 +356,9 @@ object SchemaUtils {
"${currentDialect.name} requires autoCommit to be enabled for DROP DATABASE",
exception
)
} else throw exception
} else {
throw exception
}
}
}

Expand Down Expand Up @@ -367,7 +398,8 @@ object SchemaUtils {
}
val executedStatements = createStatements + alterStatements
logTimeSpent("Checking mapping consistence", withLogs) {
val modifyTablesStatements = checkMappingConsistence(tables = tables, withLogs).filter { it !in executedStatements }
val modifyTablesStatements = checkMappingConsistence(tables = tables, withLogs)
.filter { it !in executedStatements }
execStatements(inBatch, modifyTablesStatements)
commit()
}
Expand All @@ -389,7 +421,8 @@ object SchemaUtils {
}
val executedStatements = createStatements + alterStatements
val modifyTablesStatements = logTimeSpent("Checking mapping consistence", withLogs) {
checkMappingConsistence(tables = tablesToAlter.toTypedArray(), withLogs).filter { it !in executedStatements }
checkMappingConsistence(tables = tablesToAlter.toTypedArray(), withLogs)
.filter { it !in executedStatements }
}
return executedStatements + modifyTablesStatements
}
Expand Down Expand Up @@ -426,7 +459,9 @@ object SchemaUtils {
}

val excessiveIndices =
currentDialect.existingIndices(*tables).flatMap { it.value }.groupBy { Triple(it.table, it.unique, it.columns.joinToString { it.name }) }
currentDialect.existingIndices(*tables)
.flatMap { it.value }
.groupBy { Triple(it.table, it.unique, it.columns.joinToString { it.name }) }
.filter { it.value.size > 1 }
if (excessiveIndices.isNotEmpty()) {
exposedLogger.warn("List of excessive indices:")
Expand Down Expand Up @@ -486,7 +521,8 @@ object SchemaUtils {
nameDiffers.add(mappedIndex)
}

notMappedIndices.getOrPut(table.nameInDatabaseCase()) { hashSetOf() }.addAll(existingTableIndices.subtract(mappedIndices))
notMappedIndices.getOrPut(table.nameInDatabaseCase()) { hashSetOf() }
.addAll(existingTableIndices.subtract(mappedIndices))

missingIndices.addAll(mappedIndices.subtract(existingTableIndices))
}
Expand Down Expand Up @@ -601,7 +637,11 @@ object SchemaUtils {
fun dropSchema(vararg schemas: Schema, cascade: Boolean = false, inBatch: Boolean = false) {
if (schemas.isEmpty()) return
with(TransactionManager.current()) {
val schemasForDeletion = if (currentDialect.supportsIfNotExists) schemas.distinct() else schemas.distinct().filter { it.exists() }
val schemasForDeletion = if (currentDialect.supportsIfNotExists) {
schemas.distinct()
} else {
schemas.distinct().filter { it.exists() }
}
val dropStatements = schemasForDeletion.flatMap { it.dropStatement(cascade) }

execStatements(inBatch, dropStatements)
Expand Down
3 changes: 3 additions & 0 deletions ...-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/api/ExposedDatabaseMetadata.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.jetbrains.exposed.sql.ForeignKeyConstraint
import org.jetbrains.exposed.sql.Index
import org.jetbrains.exposed.sql.Table
import org.jetbrains.exposed.sql.vendors.ColumnMetadata
import org.jetbrains.exposed.sql.vendors.PrimaryKeyMetadata
import java.math.BigDecimal

abstract class ExposedDatabaseMetadata(val database: String) {
Expand Down Expand Up @@ -33,6 +34,8 @@ abstract class ExposedDatabaseMetadata(val database: String) {

abstract fun existingIndices(vararg tables: Table): Map<Table, List<Index>>

abstract fun existingPrimaryKeys(vararg tables: Table): Map<Table, PrimaryKeyMetadata?>

abstract fun tableConstraints(tables: List<Table>): Map<String, List<ForeignKeyConstraint>>

abstract fun cleanCache()
Expand Down
Loading