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

String fields without max length constraint default to varchar(255) #47

Open
roberthannon opened this issue Feb 15, 2017 · 7 comments
Open

Comments

@roberthannon
Copy link

Hi I'm working with a 3rd party framework which maps some fields without a HasMaxLength constraint. The provider looks like it defaults to varchar(255) columns for these fields. This seems fine, but it makes the provider inconsistent with the SQL Server provider which instead defaults to using nvarchar(max) (which is similar to TEXT). This is causing data to be truncated and errors to occur in some situations.

The obvious solution would be to modify the entity mappings and add the max length constraints. Unfortunately I cannot do this with the 3rd party framework I'm using, but I've asked the developers if they'd be willing to do this.

Just wondering if it would also make sense to change the default in this provider to create columns as TEXT when no max length is specified. This would increase compatibility between this provider and the SQL Server provider. What are your thoughts?

@SapientGuardian
Copy link
Owner

SapientGuardian commented Feb 15, 2017

Seems reasonable to me on the surface, but I'm worried about the semantic differences between varchar and text (such as not being able to declare defaults for text), as well as the impact on the apps already using this library and expecting the existing behavior. If we did change this, we'd need a major version bump.

I'm curious as to what the official MySql provider does.

@roberthannon
Copy link
Author

Ok well I'll try fix my issues by making sure max length constraints are specified on all fields. Relying on provider defaults probably isn't a good idea anyway.

Last time I checked migrations didn't work with the official MySQL provider so I gave up on it. Your provider is currently the best for MySQL 👍

@Blastnsmash
Copy link

Blastnsmash commented Mar 2, 2017

I am trying to find a work around for this issue by changing the migration but no matter what I do I can't figure out how to get the Insert statement to not say "(size = 255)."

You can get the migration files here:
http://docs.identityserver.io/en/release/quickstarts/8_entity_framework.html
dotnet ef migrations add InitialIdentityServerConfigurationDbMigration

I've changed the migration files in the following way: (I've also tried several other permutations including removing the HasMaxLength method)

PersistedGrantDbContextModelSnapshot.cs-->
b.Property("Data")
.HasMaxLength(50000)
.HasColumnType("text")
.IsRequired();

20170227232536_InitialIdentityServerPersistedGrantDbMigration.cs->
Method: protected override void Up(MigrationBuilder migrationBuilder)
Data = table.Column<System.String>(nullable: false, type: "text", maxLength:50000),
Method: protected override void BuildTargetModel(ModelBuilder modelBuilder)
b.Property("Data")
.HasMaxLength(50000)
.IsRequired().HasColumnType("text");

The database generates Text as the data type but the insert statement still will clip the column because it says (size = 255)

Executed DbCommand (3ms) [Parameters=[@p0='?' (Size = 200), @p1='?' (Size = 50), @p2='?' (Size = 200), @p3='?', @P4='?' (Size = 255), @p5='?', @p6='?' (Size = 200)], CommandType='Text', CommandTimeout='30']
INSERT INTO PersistedGrants (Key, Type, ClientId, CreationTime, Data, Expiration, SubjectId)
VALUES (@p0, @p1, @p2, @p3, @P4, @p5, @p6);
dbug: MySQL.Data.Entity.MySQLRelationalConnection[6]
Committing transaction.
dbug: MySQL.Data.Entity.MySQLRelationalConnection[4]
Closing connection to database 'UserIdentityCore' on server '127.0.0.1'.

Seems like all strings has (size=) in the insert. How do you remove or change it so that it respects the maxlength instead of limiting number to 255.

@roberthannon
Copy link
Author

@Blastnsmash You've changed the migrations, but the entity mapping will still be the same and will have a default max length of 255 applied. To fix your issue I think you'll need to modify the mapping somehow and I'm not sure we can currently do that. Here's how identity server is do the entity mappings: https://github.com/IdentityServer/IdentityServer4.EntityFramework/blob/dev/src/IdentityServer4.EntityFramework/Extensions/ModelBuilderExtensions.cs

Hopefully we have a solution to this at some stage: https://github.com/IdentityServer/IdentityServer4.EntityFramework/issues/54

@Linksonder
Copy link

I am running into the same issue as @Blastnsmash.
Has there been any resolution for the 'underwater' clipping of strings?

@SapientGuardian
Copy link
Owner

This library is no longer maintained, I would suggest switching to the Pomelo provider.

@eByte23
Copy link

eByte23 commented Sep 15, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants