Skip to content

Commit

Permalink
Fix 4714 postgresql alter column nullability (#4831)
Browse files Browse the repository at this point in the history
* Change postgresql grammar

Create a rule to express the not null constraint

* Update AlterTableAlterColumnMixin

Find the not null column constraint SET or DROP and set
nullability on QueryColumn

* Update TableInterfaceGenerator

If the QueryColumn has been modified the nullable flag will be will either true or false
Override the columnDef nullable type

* Add migration tests

Testing that the data class properties have update nullable types and unmodified remain the same

* Refactor applyTo

The QueryColumn nullable property can be null, true or false
We can always assign queryColumn.copy(nullable = nullableColumn) when the column matches
  • Loading branch information
griffio authored Nov 27, 2023
1 parent 6f19d12 commit f25d1c3
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,11 @@ type_clause ::= 'TYPE'

data_clause ::= 'DATA'

column_not_null_clause ::= (SET | DROP) NOT NULL

alter_table_alter_column ::= ALTER [COLUMN] {column_name}
( [ SET data_clause ] type_clause {column_type} [USING {column_name}'::'{column_type}]
| (SET | DROP) NOT NULL
| column_not_null_clause
| SET DEFAULT <<expr '-1'>> | DROP DEFAULT | DROP identity_clause [ IF EXISTS ] | ADD {generated_clause}
| SET GENERATED (ALWAYS | BY DEFAULT )
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,38 @@ package app.cash.sqldelight.dialects.postgresql.grammar.mixins
import app.cash.sqldelight.dialects.postgresql.grammar.psi.PostgreSqlAlterTableAlterColumn
import com.alecstrong.sql.psi.core.psi.AlterTableApplier
import com.alecstrong.sql.psi.core.psi.LazyQuery
import com.alecstrong.sql.psi.core.psi.SqlColumnName
import com.alecstrong.sql.psi.core.psi.SqlCompositeElementImpl
import com.alecstrong.sql.psi.core.psi.SqlTypes
import com.intellij.lang.ASTNode
import com.intellij.psi.util.elementType

internal abstract class AlterTableAlterColumnMixin(
node: ASTNode,
) : SqlCompositeElementImpl(node),
PostgreSqlAlterTableAlterColumn,
AlterTableApplier {

private val columnName
get() = children.filterIsInstance<SqlColumnName>().first()

override fun applyTo(lazyQuery: LazyQuery): LazyQuery {
return lazyQuery
val nullableColumn: Boolean? = columnNotNullClause?.let {
when (it.firstChild.elementType) {
SqlTypes.DROP -> true
SqlTypes.SET -> false
else -> null
}
}

return LazyQuery(
tableName = lazyQuery.tableName,
query = {
val columns = lazyQuery.query.columns.map { queryColumn ->
if (queryColumn.element.textMatches(columnName)) queryColumn.copy(nullable = nullableColumn) else queryColumn
}
lazyQuery.query.copy(columns = columns)
},
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import app.cash.sqldelight.core.lang.ADAPTER_NAME
import app.cash.sqldelight.core.lang.psi.ColumnTypeMixin
import app.cash.sqldelight.core.lang.util.childOfType
import app.cash.sqldelight.core.lang.util.columnDefSource
import app.cash.sqldelight.core.lang.util.type
import app.cash.sqldelight.core.psi.SqlDelightStmtIdentifier
import com.alecstrong.sql.psi.core.psi.LazyQuery
import com.alecstrong.sql.psi.core.psi.NamedElement
Expand Down Expand Up @@ -51,11 +52,15 @@ internal class TableInterfaceGenerator(private val table: LazyQuery) {

val constructor = FunSpec.constructorBuilder()

table.query.columns.map { it.element as NamedElement }.forEach { column ->
table.query.columns.forEach { queryColumn ->
val column = queryColumn.element as NamedElement
val columnName = allocateName(column)
val columnDef = column.columnDefSource()!!
val columnType = columnDef.columnType as ColumnTypeMixin
val javaType = columnType.type().javaType
val javaType = queryColumn.nullable?.let { isNullable ->
if (isNullable) columnType.type().asNullable().javaType else columnType.type().asNonNullable().javaType
} ?: columnType.type().javaType

val typeWithoutAnnotations = javaType.copy(annotations = emptyList())
typeSpec.addProperty(
PropertySpec.builder(columnName, typeWithoutAnnotations)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ class MigrationQueryTest {
checkFixtureCompiles("alter-table-rename-column", PostgreSqlDialect())
}

@Test fun `alter table alter column statement`() {
checkFixtureCompiles("alter-table-alter-column", PostgreSqlDialect())
}

private fun checkFixtureCompiles(fixtureRoot: String, dialect: SqlDelightDialect = SqliteDialect()) {
val result = FixtureCompiler.compileFixture(
overrideDialect = dialect,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
CREATE TABLE test (
first TEXT NOT NULL,
second INTEGER,
third TEXT NOT NULL,
fourth INTEGER
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE test ALTER COLUMN first DROP NOT NULL;
ALTER TABLE test ALTER COLUMN second SET NOT NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
insertFirst:
INSERT INTO test (first, second, third, fourth) VALUES (?, ?, ?, ?);
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.example

import app.cash.sqldelight.TransacterImpl
import app.cash.sqldelight.db.SqlDriver
import app.cash.sqldelight.driver.jdbc.JdbcPreparedStatement
import kotlin.Int
import kotlin.String

public class DataQueries(
driver: SqlDriver,
) : TransacterImpl(driver) {
public fun insertFirst(
first: String?,
second: Int,
third: String,
fourth: Int?,
) {
driver.execute(-2_134_278_654,
"""INSERT INTO test (first, second, third, fourth) VALUES (?, ?, ?, ?)""", 4) {
check(this is JdbcPreparedStatement)
bindString(0, first)
bindInt(1, second)
bindString(2, third)
bindInt(3, fourth)
}
notifyQueries(-2_134_278_654) { emit ->
emit("test")
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.example

import kotlin.Int
import kotlin.String

public data class Test(
public val first: String?,
public val second: Int,
public val third: String,
public val fourth: Int?,
)

0 comments on commit f25d1c3

Please sign in to comment.