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

chore! EXPOSED-655 Join column default fields defaultValueFun, defautValueInDb, isDatabaseGenerated into one field #2319

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

chore! EXPOSED-655 Join column default fields defaultValueFun, defautValueInDb, isDatabaseGenerated into one field #2319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 14 additions & 0 deletions documentation-website/Writerside/topics/Breaking-Changes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@
-- Starting from version 0.57.0
INSERT INTO TEST DEFAULT VALUES
```

* The fields `defaultValueFun`, `dbDefaultValue`, and `isDatabaseGenerated` have been consolidated into a single field,
`default`, of type `ColumnDefault`. The previous setup with the three separate fields often led to inconsistencies
and confusion regarding their various combinations.
Comment on lines +28 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, this document should only include information that is relevant to the user. It's good to justify the rationale behind our decisions, but 2 of these fields are actually internal and will mean nothing to most users.

For the list below, a user would never have used dbDefaultValue != null or isDatabaseGenerated == true in their code, so I don't think they need to be noted.
The breaking changes that should be mentioned are how:

Column.defaultValueInDb() -> Column.databaseDefaultExpression()
Column.isDatabaseGenerated() -> Column.hasDatabaseDefault()

// this part I'm not sure how to suggest a migration for...
Column.defaultValueFun -> Column.default ???

Also, is there a migration element that is maybe missing?
What if for example the field defaultValueFun was being invoked somewhere in a user's logic like:

Column.defaultValueFun?.invoke() -> ???
// what would this become?
// Column.defaultValue() or Column.clientDefaultValue() or Column.clientDefaultValueOrExpression() ?
// would the user need to know the underlying ColumnDefault and cast?

Does there maybe need to something like ColumnDefault.invoke() that returns the result of invoking the Kotlin function if it exists or throws an error otherwise?


Here’s how you can migrate the common combinations:
- `defaultValueFun != null && dbDefaultValue != null` (knows as `default()`) becomes `DatabaseColumnDefaultExpressionWithValue`
- `dbDefaultValue != null` (knows as `defaultExpression()`) becomes `DatabaseColumnDefaultExpression`
- `defaultValueFun != null` (knows as `clientDefault()`) becomes `ClientColumnDefaultValue`
- `isDatabaseGenerated == true` (knows as `databaseGenerated`) becomes `DatabaseGeneratedColumnDefault`
Comment on lines +33 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean "known" instead of "knows" here?

Suggested change
- `defaultValueFun != null && dbDefaultValue != null` (knows as `default()`) becomes `DatabaseColumnDefaultExpressionWithValue`
- `dbDefaultValue != null` (knows as `defaultExpression()`) becomes `DatabaseColumnDefaultExpression`
- `defaultValueFun != null` (knows as `clientDefault()`) becomes `ClientColumnDefaultValue`
- `isDatabaseGenerated == true` (knows as `databaseGenerated`) becomes `DatabaseGeneratedColumnDefault`
- `defaultValueFun != null && dbDefaultValue != null` (known as `default()`) becomes `DatabaseColumnDefaultExpressionWithValue`
- `dbDefaultValue != null` (known as `defaultExpression()`) becomes `DatabaseColumnDefaultExpression`
- `defaultValueFun != null` (known as `clientDefault()`) becomes `ClientColumnDefaultValue`
- `isDatabaseGenerated == true` (known as `databaseGenerated`) becomes `DatabaseGeneratedColumnDefault`


If it is anticipated that a column will be transformed using `transform()`,
the default value for that column should implement `TransformableColumnDefault`. However, not all defaults need to
implement this interface; it is primarily necessary for columns with in-memory Kotlin values.

## 0.56.0
* If the `distinct` parameter of `groupConcat()` is set to `true`, when using Oracle or SQL Server, this will now fail early with an
Expand Down
63 changes: 58 additions & 5 deletions exposed-core/api/exposed-core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,15 @@ public final class org/jetbrains/exposed/sql/CheckConstraint : org/jetbrains/exp
public final class org/jetbrains/exposed/sql/CheckConstraint$Companion {
}

public abstract interface class org/jetbrains/exposed/sql/ClientColumnDefault : org/jetbrains/exposed/sql/ColumnDefault {
}

public final class org/jetbrains/exposed/sql/ClientColumnDefaultValue : org/jetbrains/exposed/sql/ClientColumnDefault, org/jetbrains/exposed/sql/ColumnDefaultValue, org/jetbrains/exposed/sql/TransformableColumnDefault {
public fun <init> (Lkotlin/jvm/functions/Function0;)V
public fun getValue ()Lkotlin/jvm/functions/Function0;
public fun transform (Lkotlin/jvm/functions/Function1;)Lorg/jetbrains/exposed/sql/ColumnDefault;
}

public final class org/jetbrains/exposed/sql/Coalesce : org/jetbrains/exposed/sql/Function {
public fun <init> (Lorg/jetbrains/exposed/sql/ExpressionWithColumnType;Lorg/jetbrains/exposed/sql/Expression;[Lorg/jetbrains/exposed/sql/Expression;)V
public fun toQueryBuilder (Lorg/jetbrains/exposed/sql/QueryBuilder;)V
Expand All @@ -423,32 +432,53 @@ public final class org/jetbrains/exposed/sql/Column : org/jetbrains/exposed/sql/
public synthetic fun compareTo (Ljava/lang/Object;)I
public fun compareTo (Lorg/jetbrains/exposed/sql/Column;)I
public fun createStatement ()Ljava/util/List;
public final fun defaultValueInDb ()Lorg/jetbrains/exposed/sql/Expression;
public final fun descriptionDdl (Z)Ljava/lang/String;
public static synthetic fun descriptionDdl$default (Lorg/jetbrains/exposed/sql/Column;ZILjava/lang/Object;)Ljava/lang/String;
public fun dropStatement ()Ljava/util/List;
public fun equals (Ljava/lang/Object;)Z
public fun getColumnType ()Lorg/jetbrains/exposed/sql/IColumnType;
public fun getDdl ()Ljava/util/List;
public final fun getDefaultValueFun ()Lkotlin/jvm/functions/Function0;
public final fun getDefault ()Lorg/jetbrains/exposed/sql/ColumnDefault;
public final fun getForeignKey ()Lorg/jetbrains/exposed/sql/ForeignKeyConstraint;
public final fun getName ()Ljava/lang/String;
public final fun getReferee ()Lorg/jetbrains/exposed/sql/Column;
public final fun getTable ()Lorg/jetbrains/exposed/sql/Table;
public fun hashCode ()I
public final fun isDatabaseGenerated ()Z
public fun modifyStatement ()Ljava/util/List;
public final fun modifyStatements (Lorg/jetbrains/exposed/sql/ColumnDiff;)Ljava/util/List;
public final fun nameInDatabaseCase ()Ljava/lang/String;
public final fun nameUnquoted ()Ljava/lang/String;
public final fun referee ()Lorg/jetbrains/exposed/sql/Column;
public final fun setDefaultValueFun (Lkotlin/jvm/functions/Function0;)V
public final fun setDefault (Lorg/jetbrains/exposed/sql/ColumnDefault;)V
public final fun setForeignKey (Lorg/jetbrains/exposed/sql/ForeignKeyConstraint;)V
public fun toQueryBuilder (Lorg/jetbrains/exposed/sql/QueryBuilder;)V
public fun toString ()Ljava/lang/String;
public final fun withColumnType (Lorg/jetbrains/exposed/sql/IColumnType;)Lorg/jetbrains/exposed/sql/Column;
}

public abstract interface class org/jetbrains/exposed/sql/ColumnDefault {
}

public abstract interface class org/jetbrains/exposed/sql/ColumnDefaultExpression : org/jetbrains/exposed/sql/ColumnDefault {
public abstract fun getExpression ()Lorg/jetbrains/exposed/sql/Expression;
}

public final class org/jetbrains/exposed/sql/ColumnDefaultKt {
public static final fun clientDefaultValue (Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/Object;
public static final fun clientDefaultValueOrExpression (Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/Object;
public static final fun databaseDefaultExpression (Lorg/jetbrains/exposed/sql/Column;)Lorg/jetbrains/exposed/sql/Expression;
public static final fun defaultValue (Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/Object;
public static final fun hasClientDefault (Lorg/jetbrains/exposed/sql/Column;)Z
public static final fun hasClientDefaultValue (Lorg/jetbrains/exposed/sql/Column;)Z
public static final fun hasDatabaseDefault (Lorg/jetbrains/exposed/sql/Column;)Z
public static final fun hasDefaultValue (Lorg/jetbrains/exposed/sql/Column;)Z
public static final fun transform (Lorg/jetbrains/exposed/sql/ColumnDefault;Lkotlin/jvm/functions/Function1;)Lorg/jetbrains/exposed/sql/ColumnDefault;
}

public abstract interface class org/jetbrains/exposed/sql/ColumnDefaultValue : org/jetbrains/exposed/sql/ColumnDefault {
public abstract fun getValue ()Lkotlin/jvm/functions/Function0;
}

public final class org/jetbrains/exposed/sql/ColumnDiff {
public static final field Companion Lorg/jetbrains/exposed/sql/ColumnDiff$Companion;
public fun <init> (ZZZZZ)V
Expand Down Expand Up @@ -688,6 +718,21 @@ public final class org/jetbrains/exposed/sql/Database$Companion {
public final fun registerJdbcDriver (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
}

public abstract interface class org/jetbrains/exposed/sql/DatabaseColumnDefault : org/jetbrains/exposed/sql/ColumnDefault {
}

public final class org/jetbrains/exposed/sql/DatabaseColumnDefaultExpression : org/jetbrains/exposed/sql/ColumnDefaultExpression, org/jetbrains/exposed/sql/DatabaseColumnDefault {
public fun <init> (Lorg/jetbrains/exposed/sql/Expression;)V
public fun getExpression ()Lorg/jetbrains/exposed/sql/Expression;
}

public final class org/jetbrains/exposed/sql/DatabaseColumnDefaultExpressionWithValue : org/jetbrains/exposed/sql/ColumnDefaultExpression, org/jetbrains/exposed/sql/ColumnDefaultValue, org/jetbrains/exposed/sql/DatabaseColumnDefault, org/jetbrains/exposed/sql/TransformableColumnDefault {
public fun <init> (Lorg/jetbrains/exposed/sql/Expression;Lkotlin/jvm/functions/Function0;)V
public fun getExpression ()Lorg/jetbrains/exposed/sql/Expression;
public fun getValue ()Lkotlin/jvm/functions/Function0;
public fun transform (Lkotlin/jvm/functions/Function1;)Lorg/jetbrains/exposed/sql/ColumnDefault;
}

public final class org/jetbrains/exposed/sql/DatabaseConfig {
public static final field Companion Lorg/jetbrains/exposed/sql/DatabaseConfig$Companion;
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/SqlLogger;ZLjava/lang/Integer;IIJJIJJZLjava/lang/Long;IZLorg/jetbrains/exposed/sql/vendors/DatabaseDialect;Lorg/jetbrains/exposed/sql/Schema;IZLkotlin/jvm/internal/DefaultConstructorMarker;)V
Expand Down Expand Up @@ -761,6 +806,10 @@ public final class org/jetbrains/exposed/sql/DatabaseConfig$Companion {
public abstract interface class org/jetbrains/exposed/sql/DatabaseConnectionAutoRegistration : kotlin/jvm/functions/Function1 {
}

public final class org/jetbrains/exposed/sql/DatabaseGeneratedColumnDefault : org/jetbrains/exposed/sql/DatabaseColumnDefault {
public fun <init> ()V
}

public final class org/jetbrains/exposed/sql/DatabaseKt {
public static final fun getName (Lorg/jetbrains/exposed/sql/Database;)Ljava/lang/String;
}
Expand Down Expand Up @@ -2712,6 +2761,10 @@ public class org/jetbrains/exposed/sql/Transaction : org/jetbrains/exposed/sql/U
public final class org/jetbrains/exposed/sql/Transaction$Companion {
}

public abstract interface class org/jetbrains/exposed/sql/TransformableColumnDefault : org/jetbrains/exposed/sql/ColumnDefault {
public abstract fun transform (Lkotlin/jvm/functions/Function1;)Lorg/jetbrains/exposed/sql/ColumnDefault;
}

public final class org/jetbrains/exposed/sql/Trim : org/jetbrains/exposed/sql/Function {
public fun <init> (Lorg/jetbrains/exposed/sql/Expression;)V
public final fun getExpr ()Lorg/jetbrains/exposed/sql/Expression;
Expand Down Expand Up @@ -3210,7 +3263,7 @@ public class org/jetbrains/exposed/sql/statements/InsertStatement : org/jetbrain
public final fun getResultedValues ()Ljava/util/List;
public final fun getTable ()Lorg/jetbrains/exposed/sql/Table;
protected fun isColumnValuePreferredFromResultSet (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)Z
protected final fun isEntityIdClientSideGeneratedUUID (Lorg/jetbrains/exposed/sql/Column;)Z
protected final fun isEntityIdClientSideGeneratedUUID (Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/Boolean;
public final fun isIgnore ()Z
public fun prepareSQL (Lorg/jetbrains/exposed/sql/Transaction;Z)Ljava/lang/String;
public fun prepared (Lorg/jetbrains/exposed/sql/Transaction;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;
Expand Down
2 changes: 1 addition & 1 deletion exposed-core/src/main/kotlin/org/jetbrains/exposed/dao/id/IdTable.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ open class CompositeIdTable(name: String = "") : IdTable<CompositeID>(name) {
}
)
return Column(this, "composite_id", EntityIDColumnType(placeholder)).apply {
defaultValueFun = null
default = null
}
}

Expand Down
4 changes: 1 addition & 3 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Alias.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ class Alias<out T : Table>(val delegate: T, val alias: String) : Table() {
val tableNameWithAlias: String = "${delegate.tableName} $alias"

private fun <T> Column<T>.clone() = Column<T>(this@Alias, name, columnType).also {
it.defaultValueFun = defaultValueFun
it.dbDefaultValue = dbDefaultValue
it.isDatabaseGenerated = isDatabaseGenerated
it.default = default
it.foreignKey = foreignKey
it.extraDefinitions = extraDefinitions
}
Expand Down
38 changes: 13 additions & 25 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Column.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,10 @@ class Column<T>(
@Suppress("UNCHECKED_CAST")
fun <S : T> referee(): Column<S>? = referee as? Column<S>

/** Returns the function that calculates the default value for this column. */
var defaultValueFun: (() -> T)? = null
internal var dbDefaultValue: Expression<T>? = null

/** Returns the default value for this column on the database-side. */
fun defaultValueInDb() = dbDefaultValue

internal var isDatabaseGenerated: Boolean = false

/** Returns whether this column's value will be generated in the database. */
fun isDatabaseGenerated() = isDatabaseGenerated
Comment on lines -31 to -41
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be great for users if we could try to go through the standard deprecation cycle where possible, using ReplaceWith options etc. If the var is difficult, it should at least be possible for the two functions.

/**
* The default value for this column.
*/
var default: ColumnDefault<T>? = null

internal var extraDefinitions = mutableListOf<Any>()

Expand Down Expand Up @@ -121,12 +114,13 @@ class Column<T>(
else -> append(columnType.sqlType())
}

val defaultValue = dbDefaultValue
if (defaultValue != null) {
val expressionSQL = currentDialect.dataTypeProvider.processForDefaultValue(defaultValue)
if (!currentDialect.isAllowedAsColumnDefault(defaultValue)) {
val defaultDatabaseValue = databaseDefaultExpression()

if (defaultDatabaseValue != null) {
val expressionSQL = currentDialect.dataTypeProvider.processForDefaultValue(defaultDatabaseValue)
if (!currentDialect.isAllowedAsColumnDefault(defaultDatabaseValue)) {
val clientDefault = when {
defaultValueFun != null && dbDefaultValue == null -> " Expression will be evaluated on the client."
hasClientDefault() -> " Expression will be evaluated on the client."
!columnType.nullable -> " Column will be created with NULL marker."
else -> ""
}
Expand All @@ -148,7 +142,7 @@ class Column<T>(
append(extraDefinitions.joinToString(separator = " ", prefix = " ") { "$it" })
}

if (columnType.nullable || (defaultValue != null && defaultValueFun == null && !currentDialect.isAllowedAsColumnDefault(defaultValue))) {
if (columnType.nullable || (defaultDatabaseValue != null && !currentDialect.isAllowedAsColumnDefault(defaultDatabaseValue))) {
append(" NULL")
} else if (!isPKColumn || (currentDialect is SQLiteDialect && !isSQLiteAutoIncColumn)) {
append(" NOT NULL")
Expand All @@ -163,14 +157,10 @@ class Column<T>(
val newColumn: Column<R> = Column(table, name, columnType)
newColumn.foreignKey = foreignKey
@Suppress("UNCHECKED_CAST")
newColumn.dbDefaultValue = dbDefaultValue as Expression<R>?
newColumn.isDatabaseGenerated = isDatabaseGenerated
newColumn.default = default as ColumnDefault<R>?
newColumn.extraDefinitions = extraDefinitions
body?.let { newColumn.it() }

if (defaultValueFun != null) {
require(newColumn.defaultValueFun != null) { "defaultValueFun was lost on cloning the column" }
}
return newColumn
}

Expand All @@ -183,9 +173,7 @@ class Column<T>(
columnType = columnType
).also {
it.foreignKey = this.foreignKey
it.defaultValueFun = this.defaultValueFun
it.dbDefaultValue = this.dbDefaultValue
it.isDatabaseGenerated = this.isDatabaseGenerated
it.default = this.default
it.extraDefinitions = this.extraDefinitions
}

Expand Down
Loading
Loading