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

.text creates a VARCHAR(255) column #290

Open
bottlehall opened this issue Aug 10, 2020 · 6 comments
Open

.text creates a VARCHAR(255) column #290

bottlehall opened this issue Aug 10, 2020 · 6 comments
Labels
enhancement New feature or request question Further information is requested
Projects

Comments

@bottlehall
Copy link

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 tanner0101 added bug Something isn't working question Further information is requested and removed bug Something isn't working labels Aug 13, 2020
@tanner0101 tanner0101 added this to To Do in Vapor 4 via automation Aug 13, 2020
@tanner0101 tanner0101 added the enhancement New feature or request label Aug 13, 2020
@tanner0101
Copy link
Member

@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
Copy link
Member

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

@bottlehall
Copy link
Author

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

bottlehall added a commit to bottlehall/mysql-kit that referenced this issue Aug 20, 2020
@bottlehall
Copy link
Author

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
Copy link
Author

bottlehall commented Aug 20, 2020

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

@tanner0101
Copy link
Member

tanner0101 commented Aug 20, 2020

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 #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"))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
Vapor 4
  
To Do
Development

No branches or pull requests

2 participants