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

Varchar doesn't validate length #300

Closed
aria42 opened this issue May 6, 2018 · 2 comments
Closed

Varchar doesn't validate length #300

aria42 opened this issue May 6, 2018 · 2 comments
Assignees

Comments

@aria42
Copy link

aria42 commented May 6, 2018

The VarCharColumnType doesn't validate the length of the value to write to DB, since it doesn't override StringColumnType::nonNullValueToString and error if the string is too long for the length. I'm not sure what the general policy is on trying to throw errors here (especially since some vendors like sqlite3) don't enforce any of this.

Is this something you'd like to check (and potentially disable for sqlite3 connections)? Would you welcome a PR.

@Tapac
Copy link
Contributor

Tapac commented May 7, 2018

PRs are always very welcomed, but what kind of an expection are you going to throw?
And also, should that exception be raised only on write to that column?
What if a real column has more length than one which mapped in Exposed?

@aria42
Copy link
Author

aria42 commented May 7, 2018

Yeah it should be when you try to make a DB valid value from nonNullValueToString since you can't make a valid value. I'm assuming the validation should be about the data supplied to the column data class, rather than whatever the "real" column length is. You can't do any validation if you assume the column could be arbitrarily different since the valid might be correct.

@Tapac Tapac self-assigned this Jun 25, 2018
Tapac added a commit that referenced this issue Jul 21, 2018
@Tapac Tapac closed this as completed Jul 21, 2018
Tapac added a commit that referenced this issue Sep 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants