p2-inc / keycloak-orgs

Single realm, multi-tenancy for SaaS apps
https://phasetwo.io
Other
418 stars 72 forks source link

changed org and invite attribute column lengths to varchar 4000 #188

Closed xgp closed 9 months ago

xgp commented 9 months ago

fixes https://github.com/p2-inc/keycloak-orgs/issues/184

sindrekroknes commented 2 weeks ago

Hi @xgp, I think this change may have broken compatibility with SQL Server. Previously the Value columns were nvarchar(255), now they're varchar(4000) (without the n). But Hibernate still expects them to be nvarchar (because of the @Nationalized attribute on the properties) And with SQL Server I'm getting an error stating "The conversion from varchar to NCHAR is unsupported" when it tries to fetch attributes. Manually changing the column to nvarchar fizes this

xgp commented 1 week ago

@sindrekroknes Feel free to PR a new liquibase changeSet that sets the type conditional on sqlserver. We don't have any customers running that database, so it's not really "supported" by us.

sindrekroknes commented 1 week ago

@xgp I work mainly in .NET, so my knowledge here is fairly limited. Most of my previous comment is based on what I discovered debugging that error. But if all that's needed is an extra changelog xml file it can give it a try 🙂

Since having the changeset file say nvarchar seems to have worked before, even though it's a SQL Server specific type. I was wondering if it would be better to change it back to nvarchar instead of having to specifically check for SQL Server? As there seems to already be some implicit translation from nvarchar to varchar for other dbms.

xgp commented 1 week ago

all that's needed is an extra changelog xml file it can give it a try

please do.

I don't want to change column types for instances that are working, so changing everything to nvarchar isn't possible.