grate icon indicating copy to clipboard operation
grate copied to clipboard

Unable to use grate on UTF-8 collations

Open SamuelMcAravey opened this issue 1 year ago • 1 comments

Describe the bug Unable to use UTF-8 collations with SQL Server due to using the obsolete text column type.

To Reproduce Create a new database using a UTF-8 collation, such as Latin1_General_100_CI_AS_SC_UTF8, and run grate against this database.

Expected behavior A successful migration happens.

Screenshots

image

Desktop (please complete the following information):

  • OS: Windows
  • Version: 1.8.0

Additional context When running this through the debugger, I get this specific exception which indicates that this is due to the text type:

'Column or parameter 'text_of_script' has type 'text' and collation 'Latin1_General_100_CI_AS_SC_UTF8'.
The legacy LOB types do not support UTF-8 or UTF-16 encodings. 
Use types varchar(max), nvarchar(max) or a collation which does not have the _SC or _UTF8 flags.'

In my local version I changed all of the text types to nvarchar(max) and everything functioned as expected.

There appears to have been some prior discussion here, but it appears that the column type was not updated: https://github.com/erikbra/grate/issues/250

SamuelMcAravey avatar Oct 23 '24 16:10 SamuelMcAravey

Actually, this is fixed, but in an update script, so it looks like it doesn't work if it's not possible to create a column with text type at all, when running for the first time.

We need to think about how we can ensure that the Baseline scripts can actually be run the first time too, without trying to run them twice (if we change them). Good ideas and PRs are appreciated.

We do alter the type of the columns in a migration afterwards:

https://github.com/erikbra/grate/blob/main/src/grate.sqlserver/Bootstrapping/Sql/GrateStructure/up/03_scripts_run_errors_unicode_support.sql

erikbra avatar Jan 19 '25 15:01 erikbra

Any update on this? I understand that this is an open-source project, and likely done on your own time and not a day job. I would submit a PR for this issue, but it does seem like there are a lot of PRs the better part of 6+ months along and stagnant. Should this project be considered active still?

tracker1 avatar Jul 16 '25 15:07 tracker1

This project is very much a labour of love from a very small number of people, in between paid work, families, mental and physical health and other commitments. Erik has clearly been caught up with any/all of the above for a while now.

I'd still suggest you submit a PR if you're able to as:

  • It's much easier to just tick and flick a well tested PR when we're time poor
  • The more people with the knowledge to contribute the less load on any one individual
  • It allows myself and others to review the PR, and not be reliant on Erik for that portion of the work, and
  • It allows yourself and others to run your fixed version of the code in the short term, and not be blocked waiting on getting it merged/released.

I (and others?) have the permissions to merge PRs, but I generally choose not to excercise that right unless it's something critical.

wokket avatar Jul 16 '25 20:07 wokket

Okay, I'll try to get a PR ready in the next couple days... just going to switch the TEXT fields in the initial create to VARCHAR(MAX) and add sanity check for the alter statements in the update scripts.

tracker1 avatar Jul 30 '25 22:07 tracker1

Are y'all on a slack or discord channel by chance? I'm having trouble just getting tests to run properly... I've run dotnet test from the cli as well as loaded vscode with the devcontainer to run it, and getting constant connection errors... from portainer I can see the containers spinning up and down.

Beyodn this, I cannot seem to get Jetbrains Rider to load the devcontainer, I'm using the flatpak release under Linux on my personal desktop since my work desktop is too locked down to debug properly.

My only local changes are in the SQL create database, I've added a utf8 collation, as well as in the bootstrapping replace TEXT with NVARCHAR(MAX)... If I could manage to just test/debug the sql config it would be kind of nice... the complexity in this project is a bit maddening coming in from scratch.

tracker1 avatar Sep 26 '25 19:09 tracker1

Aside, I cannot seem to build a standalone executable following the directions... it publishes the typical .net bin directory.

dotnet publish -c release -r linux-musl-x64 -o /tmp/grate --framework net8.0

Also, I did update the devcontainer.json to .net 9 to match the build/test default

tracker1 avatar Sep 26 '25 19:09 tracker1

I don't have enough time to be on any discord/slack channels, and I'm not aware of a grate specific server.

the complexity in this project is a bit maddening coming in from scratch.

That's... disheartening... given grate was started due to difficulty contributing to Roundhouse. I feel your pain though, there's been a few changes that added huge amounts of configurability to how people can run the tool, but at the cost of lots of code complexity. As we've added different DBMSs, .Net versions, ways of distributing the tool etc the matrix has increased exponentially. If you have any ideas on how to smooth that pain please let us know!

I'm having trouble just getting tests to run properly...

I dev on a windows machine, using Rider (previously used VS as well). I do not use the devcontainer. Running the tests in Rider generally just works, except for all the tests that randomly fail!. I normally either:

  • Run the full suite (then go for a sit down meal), then re-run just the failing tests one at a time which invariably then pass, or
  • Only run the tests I care about for the change, and rely on CI when I push to the fork

I don't know if that's a windows PC thing, a docker desktop thing, a Rider thing or just a me a thing. The CI build seems reliable here so that's all that really matters I guess?

wokket avatar Sep 27 '25 00:09 wokket

Aside, I cannot seem to build a standalone executable following the directions... it publishes the typical .net bin directory.

dotnet publish -c release -r linux-musl-x64 -o /tmp/grate --framework net8.0

I'm not sure what you're trying to achieve here or which directions you're referring to, but if you want a self-contained file then just add --self-contained to your command line above

wokket avatar Sep 27 '25 00:09 wokket

I don't know if that's a windows PC thing, a docker desktop thing, a Rider thing or just a me a thing. The CI build seems reliable here so that's all that really matters I guess?

@wokket I'm not sure... I'm running a recent Linux kernel and release so definitely not a windows thing. I'll probably stick to just Rider on the desktop then instead of the dev container, which I did get working in VS Code, but had similar testing issues under the dev container as well as directly on desktop use... glad to know it isn't just me at least. Given the complexity of multiple databases, I can understand. I've not really had to run a project with this level of testing complexity... I've usually done things a little closer to the surface via shell scripts with docker-compose file(s) to setup testing. Not 100% sure where some of the complexity is, I'm thinking it might be on the timing of setup/teardown of the containers... but that's just speculation on my part not having dug into that.

I'm not sure what you're trying to achieve here or which directions you're referring to, but if you want a self-contained file then just add --self-contained to your command line above

Mostly just looked into the CONTRIBUTING.md file in the base directory for the repository... Looking like it hadn't been updated in a while.

Thank you for the response... just glad to know the failing tests aren't just a me thing... I spent a bit of time in a couple different paths just wanting to sanity check that my environment was okay... I had to install the .Net 9 sdk (I had jumped from 8 to 10) and had a couple hiccups as mentioned with the dev container... also since I'm using the flatpak of Rider couldn't manage to get the devcontainer config working at all there, just vs code. Rider does better with .Net projects than VS Code tends to, the larger the project/solution gets.

I will probably have to just test against my changes locally that I've made so far to see if they work against a fresh UTF8 collated db, then circle back to upstream my commits... If you're curious about my changes so far, will probable add a couple sanity checks.

https://github.com/tracker1/grate/tree/tracker1/fix-mssql-utf8

tracker1 avatar Sep 29 '25 19:09 tracker1

My initial thought is to update the bootstrap script as @tracker1 suggested; however, it will cause the existing grate version (1.8) failure due to the bootstrap script has been run in the target database.

I've created the PR to fix the issue by adding a check if not exist and bypassing the error if any (using WarnOnOneTimeScriptChanges = true for grate internal table only.

Hope it will help.

Thanks.

hoangthanh28 avatar Sep 30 '25 03:09 hoangthanh28

@hoangthanh28 thanks, I was planning on adding the checks you already did. I wasn't sure of the logic involved in the creation/migration of the grate tables themselves so wanted to look into it slightly deeper first... thanks for making a PR on this.

Hopefully it can get through release and into dotnet tool install path quickly.

tracker1 avatar Oct 01 '25 18:10 tracker1