Skip to content

Require MaxLength property to be set for DataColumns of type string #91

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

Closed
osudude opened this issue Aug 14, 2017 · 4 comments
Closed
Labels
enhancement up-for-grabs This issue waits for a contributor to fix it.

Comments

@osudude
Copy link
Contributor

osudude commented Aug 14, 2017

The auto create process will throw an error if you have a datacolumn of type string and the max length is not set. Specfically, SelfLog just shows that it blew up during table auto creation but doesn't say why. I had to run a sql trace to see what it was blowing up. I had to run a sql trace to see the actual error:

"Line 2: Length or precision specification 0 is invalid."

Once you set the MaxLength property for the DataColumn the issue no longer happens. Would be nice if MaxLength property was required when column type is of type string.

@nblumhardt nblumhardt added the bug label Aug 14, 2017
@nblumhardt
Copy link
Contributor

Thanks for the report! Sounds like an oversight. 👍

If anyone's able to send a PR, all help appreciated.

Unsure whether we should blow up in a more controlled manner, or just set some sensible defaults; probably the former.

@nblumhardt nblumhardt added the up-for-grabs This issue waits for a contributor to fix it. label Jan 3, 2018
@MV10
Copy link
Contributor

MV10 commented May 9, 2018

Since my PR to split the project into Framework ConfigManager and Standard IConfig involves separating the DataColumn creation into a class shared between targets, I'll add exceptions for missing lengths (I assume that's what you mean by "blow up in a controlled manner").

I think true MAX can be changed by the DBA (or maybe just DBO, I've never had to do it), so in theory I don't think we could set a default with 100% safety without querying, which seems like overkill to me.

Maybe after the PR, this stops being a "bug" and becomes a "feature" request, in case exception is too severe? I haven't used auto-create (I'm a little surprised it's so popular), but I know my assumption would have been MAX on something like varchar before I thought about how to get that value.

@MV10
Copy link
Contributor

MV10 commented Aug 29, 2018

The PR added an exception -- the enhancement angle is whether to provide sensible defaults (if it's even possible/reasonable).

@MV10
Copy link
Contributor

MV10 commented Sep 5, 2018

Turns out a length of -1 is treated as MAX, so that case is covered.

@MV10 MV10 closed this as completed Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement up-for-grabs This issue waits for a contributor to fix it.
Projects
None yet
Development

No branches or pull requests

3 participants