feat: EXPOSED-255 Generate database migration script that can be used with any migration tool #1968
Conversation
joc-a
force-pushed
the
joc/generate-database-migration-script
branch
4 times, most recently
from
f51458a
to
dba9edd
Compare
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
joc-a
force-pushed
the
joc/generate-database-migration-script
branch
7 times, most recently
from
5a7f198
to
323b5c7
Compare
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
| @@ -513,56 +514,97 @@ object SchemaUtils { | |||
| */ | |||
| fun checkMappingConsistence(vararg tables: Table, withLogs: Boolean = true): List<String> { | |||
| if (withLogs) { | |||
| checkExcessiveIndices(tables = tables) | |||
| checkExcessiveForeignKeyConstraints(tables = tables, withLogs = true) | |||
There was a problem hiding this comment.
I would drop withLogs flag and rely on the log level
There was a problem hiding this comment.
For computations, we can make our own inline function with message: () -> String argument or check if the logging library has one.
@bog-walk, what do you think?
There was a problem hiding this comment.
I agree that I would drop withLogs, at least for the new generate function.
We could check for the enabled level, but, for example, the results in logTimeSpent() are meant for the INFO level, which I believe a lot of users set as the default. So relying on a guard like exposedLogger.isInfoEnabled, or the implicit level check in info(), might evaluate to true for many and result in users getting logs they don't want. Maybe if we were logging everything to DEBUG it would be easier.
There was a problem hiding this comment.
For computations, we can make our own inline function with
message: () -> Stringargument or check if the logging library has one.
@e5l What do you mean by "computations"?
There was a problem hiding this comment.
If you put an expression in a string template, it will be computed regardless of the log level:
logger.info("foo: ${heavyFunctionCall()}")Otherwise, we could have an extension function with block:
logger.info { "foo: ${heavyFunctionCall()}" }In this case, the string template is computed only when we call lambda (and we can condition it to the log level)
I think it's not critical for this PR, feel free to implement it later or ignore
There was a problem hiding this comment.
I'm going to leave this as is right now just to keep this PR small, but I made a note of it for the next PR.
There was a problem hiding this comment.
@e5l @joc-a If the end goal is to avoid these computations, we could also switch to parameterized logging, which is supported by slf4j. This would mean using the existing function variants that take a second argument:
exposedLogger.info("foo: {}", heavyFunctionCall())According to the docs, if I'm understanding correctly:
This form avoids superfluous object creation when the logger is disabled for the INFO level.
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
...d-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SchemaUtils.kt
Outdated
Show resolved
Hide resolved
joc-a
force-pushed
the
joc/generate-database-migration-script
branch
3 times, most recently
from
d58ea0f
to
118a196
Compare
...d-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/ddl/DatabaseMigrationTests.kt
Show resolved
Hide resolved
… with any migration tool -The script has the .sql extension -The script's name is decided by the user -The directory in which the script is created is decided by the user -The script is editable after it's generated -The user bears full responsibility for how the migration script is used. It should NOT be executed as regular SQL statements because there is no guarantee that it can be rolled back if a failure happens at any point. It should only be executed using a database migration tool like Flyway, for example, which is used in the tests for this feature.
-simplify logic for generateMigrationScript function -add testMigrationScriptOverwrittenIfAlreadyExists
-modify KDoc for checkExcessiveForeignKeyConstraints and make it public
-rearrange logic in checkExcessiveForeignKeyConstraints
-make tables argument required -add test
joc-a
force-pushed
the
joc/generate-database-migration-script
branch
from
2448133
to
cdca926
Compare
Two new functions have been added:
-
SchemaUtils.statementsRequiredForDatabaseMigration-
SchemaUtils.generateMigrationScript(marked as experimental)The conditions of usage of the generated migration script:
-It has the .sql extension.
-Its name is decided by the user.
-The directory in which it is created is decided by the user.
-It is editable after it's generated.
-The user bears full responsibility for how the migration script is used. It should NOT be executed as regular SQL statements because there is no guarantee that it can be rolled back if a failure happens at any point. It should only be executed using a database migration tool like Flyway, for example, which is used in the tests for this feature.