fluent-kit
fluent-kit copied to clipboard
vapor
Fix mysql SQLDropConstraint CONSTRAINT serialize
fix https://github.com/vapor/fluent/issues/722 and https://github.com/vapor/fluent-kit/issues/491
This looks OK to me but I don't know enough about MySQL to say what is the correct approach. I'll leave it to Gwynne. It may be that different versions require different dialects
Codecov Report
Merging #492 (6eb1e05) into main (aca2e99) will increase coverage by
2.61%. The diff coverage is100.00%.
@@ Coverage Diff @@
## main #492 +/- ##
==========================================
+ Coverage 37.80% 40.41% +2.61%
==========================================
Files 104 95 -9
Lines 5579 5164 -415
==========================================
- Hits 2109 2087 -22
+ Misses 3470 3077 -393
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 40.41% <100.00%> (+2.61%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
This looks OK to me but I don't know enough about MySQL to say what is the correct approach. I'll leave it to Gwynne. It may be that different versions require different dialects
@0xTim you were right, the old work around is for mysql 5.7, but.... after I testing the old work around on mysql 5.7, it also results in issue.
according to MySQL 5.7 Reference
Dropping Foreign Key Constraints
You can drop a foreign key constraint using the following [ALTER TABLE](https://dev.mysql.com/doc/refman/5.7/en/alter-table.html) syntax:
ALTER TABLE tbl_name DROP FOREIGN KEY fk_symbol;
So When to MySQL 5.7, it should be FOREIGN KEY in this case, but simply change KEY to FOREIGN KEY may broken other case.
as in MySQL 8.0 and later, CONSTRAINT works, and CONSTRAINT should be the right choice for the most case.
this problem is more complicated than I thought
if we want to solve this problem on MySQL 5.x, we need parameters to know if this constraint is on foreign key and db version.
if we just drop MySQL 5.x supports, simply merge this PR is okey.
Is there any likelihood of fixing this? My original issue https://github.com/vapor/fluent/issues/722 remains and it makes unit-testing very tedious as you can't revert the database automatically.
@bottlehall #522 should have solved this issue. Are you still running into it?
Hi Gwynne
\Yes, I’ve just created a minimal migration/reversion and it still happens. It fails with a MySQL error:
testIndex(): failed: caught error: "MySQL error: Server error: Cannot drop column 'defaultUnitId': needed in a foreign key constraint '864a4b794b34ca99a50c05a6504fa9f21b839e9a’"
This is the field that was added in the migration and the constraint is created during the migration.
struct UpdateUserTable2: AsyncMigration {
func prepare(on database: Database) async throws {
try await database.schema("User")
.field("defaultUnitId", .uuid, .references("Unit", "id"))
.update()
}
func revert(on database: Database) async throws {
try await database.schema("User")
.deleteField("defaultUnitId")
.update()
}
}
I have checked that the same error arises if I use the alternative method of creating the foreign key:
.foreignKey("createdByUserId", references: "User", "id", name: "created_user_id”)
And reverting using:
.deleteConstraint(name: "created_user_id")
Before trying to delete the field.
Best wishes
Nick
On 28 Jul 2023, at 16:47, Gwynne Raskind @.***> wrote:
@bottlehall #522 should have solved this issue. Are you still running into it? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>
Support for the missing functionality and information regarding its use has been added in #575