Migration causes id: serial in schema db, causing inserts to fail under sqlite3

[elondaits]

First of all, I'm rather new to Rails so I offer my apologies in advance because all I'm going to say might be the result of my own cluelessness.

I'm running Rails 6.1.3.1 on Ruby 2.6.3 using pg 1.2.3 in dev and sqlite3 1.4.2 on test.

Following the instructions in the README I added tags to my Model. It worked just fine in Development (I was able to add tags through a form and was able to verify they were stored correctly in the Gutentag tables).

The problems appeared when I tried setting up a simple test case that loaded a model object, set a tag, and saved it. This failed on saving with an error from Sqlite claiming that NOT NULL constraint failed: gutentag_tags.id. I simplified the test to directly replicate your own test cases, and it failed in the same way both using tag_names and adding a tag object through the tags collection. Finally I debugged the test case to get the SQL INSERT command that was failing and tried to execute it on my own in the sqlite database with the same result.

Then I noticed that the id fields for both gutentag_tags and gutentag_taggings were of type "serial"... which is a bit confusing to me, since the sqlite documentation claims that it doesn't support a "serial" type. In any case, where my db browser showed the id fields of other models to be of type integer, it claimed that gutentag's were serial.

Looking at schema.rb the create table commands for the gutentag tables had the following:

create_table "gutentag_taggings", id: :serial, force: :cascade do |t|
# ...
end 

create_table "gutentag_tags", id: :serial, force: :cascade do |t|
# ...
end

So I fixed my problems doing the following:

• Run the three gutentag migrations down
• Changed their superclass to ActiveRecord::Migration[6.1]
• Did a db:migrate again

After doing so the id: :serial disappeared from schema.rb, the id fields appear as "integer (auto increment)" in sqlite and my tests run ok.

So my possibly uninformed conclusion is that the default migrations created by the install procedure create a schema under rails 6.1 that does not work with sqlite. I was able to fix my own site by changing the migration superclass but I'm letting you know in case this serves to avoid other people the frustration.

[pat]

Huh, this is definitely an odd situation! Thank you for all the details, I'll try to reproduce it.

And I'll see if there's a better way to generate the migrations - it's one of the frustrating parts of engines, given the versioned migrations (though those versions are generally a very good thing!)

[elondaits]

In case it helps, this Stack Overflow post says that ActiveRecord::Migration[5.0] makes ids of type "serial" if they're not specified in the model, while ActiveRecord::Migration[5.1] doesn't. This was the reason that I first added the version to try to fix the problem.

[pat]

Okay, I've got my head around the situation - thank you for the link and all the details, it's been a great help!

Firstly, a recap of what's happening (which you likely already understand, but I'm just going to lay it out for anyone else who may come across this): As noted in that StackOverflow post, Rails switched from 32-bit integers to 64-bit integers as the default primary key data type in Rails 5.1. These data types have different names in different databases - and for PostgreSQL, they are serial and bigserial respectively.

Gutentag's migrations are flagged as being from Rails v4.2 - thus, they get the 32-bit integer primary keys. And because you're using PostgreSQL in your development environment, that gets translated to the serial data type in schema.rb. However, SQLite (in your test environment) has no idea about serial data types, and this is what caused the initial error you've been dealing with.

So, some thoughts:

• I do not like the fact that Rails uses something database-specific (i.e. serial for PostgreSQL) in schema.rb, when the Gutentag migrations are deliberately database-agnostic. I'd argue this is a bug in Rails, but, well, it's also very much an edge-case because…
• Using different database providers for different environments is strongly discouraged. Whatever's used in production should dictate what's being used in your development and test environments. There may be good reasons for mixing it up across environments, but it's almost never worth the pain.
• It would be ideal if the Gutentag migrations used the Rails version that was involved in copying them into your app (in this case, v6.1), so they get the default behaviour of that point in time (especially: defaulting to 64-bit integer primary keys), as opposed to the oldest supported default (v4.2). However, there's no way to do this automatically while maintaining support for multiple versions of Rails (unless I wanted to duplicate and then alter Rails' migration-copying code, and I really don't want to do that!).

As for solutions? This isn't anything that can be completely automated, I'm afraid. But the approach you've taken is the right one, and I've updated the README to strongly recommend people edit the migration files as part of the installation process.

Thanks again for reporting this :)

[pat]

… okay, I kept thinking about this, and have found what I feel is a Good Enough solution: instead of telling people to make edits to the migration files themselves, I've added a generator that does the same thing.

So, the setup steps are now:

bundle exec rake gutentag:install:migrations
bundle exec rails generate gutentag:migration_versions
bundle exec rake db:migrate

I considered combining the first two steps into a single migration, but that probably oversimplifies things (and this way, we have Rails handling the copying of migration files in the ways it prefers, which feels wise).

[tvdeyen]

@pat first of all thanks for a great gem I am using for lots of projects!

But I have to disagree on that solution shipped with 2.6.0.

The issue with this approach is that now all our CI builds stopped working because the migrations are missing the Rails version and fail until we also run the generator in a certain order (first install just the gutentag migrations, then patch them, then install remaining migrations of other engines) on every CI run. What makes the CI setup quite complex.

See this failing pipeline as an example https://github.com/AlchemyCMS/alchemy-solidus/runs/3299804002?check_suite_focus=true

And the fix necessary you can see here https://github.com/AlchemyCMS/alchemy-solidus/pull/82/files

And this is only one of the many Gems I maintain that uses Gutentag (as a dependency of Alchemy) as the tagging solution. I now need to update all CI scripts in order to make them work again.

I think the fix is unnecessarily complex solution to a rather simple problem.

Rails recommends to tag the migration files with the version the migration was written for (4.2 in your case). Then Rails will just "do the right thing"™️. In order to still support Rails 4.2 support you can keep that Rails version check you have in the generator. But the original migration files need to just work as before without needing to run the generator. This is more code for you to maintain and causes issues like mine.

The issue described above is because of the README is telling to use 6.1 as migration version not because of the wrong Rails version in the original migration files that where shipped with 2.5.

I can provide a PR with the fix, but the current solution should be reconsidered.

Happy to help

[pat]

Thanks Thomas for this considered feedback, and it's great to know you find Gutentag so useful :)

I agree that the generator step is not ideal. However, I also dislike the fact that if you create a new Rails app, tables will by default use 64-bit integer primary keys, but Gutentag tables will not (using the old solution). So, I've just made a change that's a bit of a hybrid of the two approaches:

• The imported Gutentag migrations, by default, are subclasses of ActiveRecord::Migration[4.2]. So, they will work straight out of the box, just like they did in Gutentag v2.5 and earlier - but only for Rails v5+.
• The generator still exists, and will update those generated migrations to use the latest subclass at that point in time, so the Gutentag tables get the same defaults as your own tables. If it happens that someone is using Gutentag in Rails 4.2 or earlier, then the subclasses simply become ActiveRecord::Migration.

The test suite continues to test against all supported Rails versions, so we can be sure that the Gutentag migration files work fine in each version. If it happens that Rails' migrations change enough for this to no longer be the case, then I will modify the generator to have a local maximum superclass version (e.g. 6.1).

This is all done in f2e0a21 if you wish to review it.

Does this approach sound feasible for how you're using the gem? I'm happy to get this release out quickly, but would appreciate confirmation that it fits your needs first :)

[tvdeyen]

Thanks @pat the solution in f2e0a21 seems like a good compromise. Have you released it yet?

[pat]

@tvdeyen released v2.6.1 just now with the fix :)

[tvdeyen]

@pat Thanks 🌷