search

vapor / mysql-kit

🐬 Pure Swift MySQL client built on non-blocking, event-driven sockets.
MIT License
222 stars 74 forks source link

`.text` creates a VARCHAR(255) column #290

Open bottlehall opened 4 years ago

bottlehall commented 4 years ago

I am trying to store text of approximately 600-1k characters. Using a field definition, such as:

@Field(key: "role")
var role: String?

Creates the usual VARCHAR(255) column in the MySQL data table. This results in the INSERT crashing because the data is too long.

I have tried putting an explicit definition in the Migration to use a TEXT column type as follows:

.field("role", .sql(.text))

However, in MySQLDialect.swift, at line 46, this defines a field specified as .text as still being VARCHAR(255). So, I get the same result.

If I manually change the column to TEXT using MySQL client between the migration to create the table and the one to insert the initial dataset, my application works as intended.

Shouldn't .text create a TEXT column instead of the same as .string?

tanner0101 commented 4 years ago

@bottlehall you can use .sql(raw: "TEXT") if you want specifically the TEXT datatype. AFAIK VARCHAR(255) is still the recommended column type for "normal length" strings. In other words, a good fit for Fluent's .string data type. However, I agree that .sql(.text) should probably translate to using TEXT in MySQL. It's a bit weird that would also use VARCHAR(255).

tanner0101 commented 4 years ago

Let me know if that seems right and if you'd like to submit a PR. Otherwise I will, thanks!

bottlehall commented 4 years ago

@tanner0101, thanks. I will do a PR in the next few days.

bottlehall commented 4 years ago

Tried doing a PR but it has errors during testing. Haven't checked them all exhaustively but this is indicative:

"MySQL error: Server error: BLOB, TEXT, GEOMETRY or JSON column 'name' can't have a default value"

bottlehall commented 4 years ago

Error above arises because SQLBenchmarker+Planets.swift creates a test table with a 'name' field that is type .text with a default value.

tanner0101 commented 4 years ago

Hmm... that's unfortunate. The easiest fix would be to check if db.dialect.name == "mysql" and use VARCHAR(255) explicitly. We can merge that through first then tests should pass for https://github.com/vapor/mysql-kit/pull/291.

Going forward, maybe some method for getting a "best fit" type for a Swift type (like bestDataType<T>(for: T.Type) -> SQLExpression) would be useful to add to SQLDialect. Then we could change that line in the benchmark test to:

.column("name", type: .best(for: String.self), .default("Unamed Planet"))