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.

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

Add composite foreign key #1385

Merged
merged 10 commits into from
Dec 4, 2021
Merged

Conversation

naftalmm
Copy link
Contributor

This PR closes #511 (and its duplicate #1234).

From the DSL point of view, there was added a new method for the Table class (foreignKey), supposed to be called in init
block.

It has two overloads, allowing two ways for defining composite foreign keys:

  1. listing pairs of the column to column references (set of referenced columns must form an explicit unique constraint)
  2. listing columns set referencing primary key:
val TableC = object : Table("TableB") {
    val idA1 = integer("id_a1")
    val idA2 = integer("id_a2")
    val idB1 = integer("id_b1")
    val idB2 = integer("id_b2")

    init {
        foreignKey(idA1 to TableA.idA1, idA2 to TableA.idA2)
        foreignKey(idB1, idB2, target = TableB.primaryKey) // foreignKey(idB1 to TableB.idB1, idB2 to TableB.idB2) is also possible
    }
}

val TableA = object : Table("TableA") {
    val idA1 = integer("id_a1")
    val idA2 = integer("id_a2")

    init {
        uniqueIndex(idA1, idA2)
    }
}
val TableB = object : Table("TableB") {
    val idB1 = integer("id_b1")
    val idB2 = integer("id_b2")
    override val primaryKey = PrimaryKey(idB1, idB2)
}

- `from` and `target` properties are now ordered sets of columns, not just a single columns
- adjusted other properties and their usage sites accordingly
- added new constructor, accepting map of columns
- added plus operation
@naftalmm naftalmm marked this pull request as ready for review November 25, 2021 11:33
Copy link
Contributor

@Tapac Tapac left a comment

Choose a reason for hiding this comment

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

Thank you very much for fixing issue requested so many time!
I kept a few comments to fix but I think they are all more like a minor fixes

onDelete: ReferenceOption? = null,
name: String? = null
) {
_foreignKeys.add(ForeignKeyConstraint(from.zip(target.columns).toMap(), onUpdate, onDelete, name))
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that check that size of from and target.columns is the same. Also, maybe it's worth to check column types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check for collections sizes. But columns type check is rather tricky - naive type equality check (from.columnType == target.columnType) will be too restrictive because SQL (at least postgreSQL) allows:

  • referencing from INT to SERIAL
  • referencing from INT NOT NULL to INT NULL (and vice versa!)
  • referencing from VARCHAR(50) to VARCHAR(100) (and vice versa!)
  • referencing from INT/SMALLINT to BIGINT (and vice versa!)

And definitely, this list is not exhaustive - it's just my experiments in sqlfiddle

So, the new method IColumnType.isCompatibleWith(other: IColumnType) : Boolean needs to be added first to make this check possible. But it looks like a big research task, at least I couldn't quickly find any docs about types compatibility (which type could be referenced to which in foreign key constraint) for any of SQL dialects.

@@ -26,6 +26,7 @@ open class SpringCoroutineTest : SpringTransactionTestBase() {

@RepeatableTest(times = 5)
@Test @Transactional @Commit
// Is this test flaky?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it is.

- all referenced columns in fk are unique
- amount of referencing columns is equal to referenced (in case of referencing to primary key)
@naftalmm naftalmm requested a review from Tapac November 29, 2021 17:14
@Tapac Tapac merged commit 5f6b27c into JetBrains:master Dec 4, 2021
tpasipanodya added a commit to tpasipanodya/exposed that referenced this pull request Dec 23, 2021
* Libs update:
kotlin 1.6.0
kotlin coroutines 1.6.0-RC
kotlinx-datetime-jvm 0.3.1
Spring framework 5.3.13
Spring boot 2.6.1

* Add composite foreign key (JetBrains#1385)

* optReference column should allow update { it[column] = nullableValue } JetBrains#1275 / Fix test on SQLServer

* Performance: cache enumConstants as it makes copy on access

* Use h2 version variable in exposed-money (JetBrains#1402)

* Better handling for opened result sets + logging

* Add H2 version 2.0.202 dialect (JetBrains#1397)

* Detekt 1.19.0

* Add H2 version 2.0.202 dialect / Fixes for test configuration

* Add H2 version 2.0.202 dialect / Fixes for test configuration #2

* Add H2 version 2.0.202 dialect / Fixes for bitwise operations

* Add composite foreign key (JetBrains#1385) / Fix test for SQLServer

Co-authored-by: Andrey.Tarashevskiy <fantocci@gmail.com>
Co-authored-by: naftalmm <naftalmm@gmail.com>
Co-authored-by: Philip Wedemann <22521688+hfhbd@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Composite foreign key
2 participants