Skip to content

Commit

Permalink
fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK (
Browse files Browse the repository at this point in the history 
JetBrains#1797)

* fix: EXPOSED-83 createMissingTablesAndColumns not detecting missing PK

createMissingTablesAndColumns does not create a new primary key (PK) if the latter
is defined by columns that already exist in the table.

A new function, existingPrimaryKeys(), has been added to retrieve relevant metadata,
which is used as a reference in addMissingColumnsStatements() to determine if the
current table's PK should be added to an existing column(s).

Add PrimaryKeyMetadata data class as a cleaner return value from
existingPrimaryKeys().
  • Loading branch information
@bog-walk @saral
bog-walk authored and saral committed Oct 3, 2023
1 parent 359b2ae commit 2da3807
Show file tree
Hide file tree
Showing 8 changed files with 258 additions and 41 deletions.
19 changes: 19 additions & 0 deletions exposed-core/api/exposed-core.api
View file
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
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
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

0 comments on commit 2da3807

Please sign in to comment.