Use :bigint for SQLite on references

[albertoalmagro]

Steps to reproduce

Discussing a possible documentation issue at #31321 with @sgrif the conclusion was that it was a bug at schema generation for SQLite.

When generating references, SQLite should also use :bigint type instead of :integer, as it will be mapped later to INTEGER by the database, see doc. Rails edge is already using bigint when creating active_storage_blobs table.

To demonstrate this I have created https://github.com/albertoalmagro/test-references-sqllite3

To summarize this test application:

1. The default migration for active storage is already using bigint for :byte_size:

# This migration comes from active_storage (originally 20170806125915)
class CreateActiveStorageTables < ActiveRecord::Migration[5.2]
  def change
    create_table :active_storage_blobs do |t|
      t.string   :key,        null: false
      t.string   :filename,   null: false
      t.string   :content_type
      t.text     :metadata
      t.bigint   :byte_size,  null: false
      t.string   :checksum,   null: false
      t.datetime :created_at, null: false

      t.index [ :key ], unique: true
    end

    create_table :active_storage_attachments do |t|
      t.string     :name,     null: false
      t.references :record,   null: false, polymorphic: true, index: false
      t.references :blob,     null: false

      t.datetime :created_at, null: false

      t.index [ :record_type, :record_id, :name, :blob_id ], name: "index_active_storage_attachments_uniqueness", unique: true
    end
  end
end

2. Once executed, this migration produces the following in db/schema.rb:

create_table "active_storage_attachments", force: :cascade do |t|
    t.string "name", null: false
    t.string "record_type", null: false
    t.integer "record_id", null: false
    t.integer "blob_id", null: false
    t.datetime "created_at", null: false
    t.index ["blob_id"], name: "index_active_storage_attachments_on_blob_id"
    t.index ["record_type", "record_id", "name", "blob_id"], name: "index_active_storage_attachments_uniqueness", unique: true
  end

  create_table "active_storage_blobs", force: :cascade do |t|
    t.string "key", null: false
    t.string "filename", null: false
    t.string "content_type"
    t.text "metadata"
    t.bigint "byte_size", null: false
    t.string "checksum", null: false
    t.datetime "created_at", null: false
    t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
  end

As it can be seen bigint is used for byte_syze, while :integer is used for record_id and blob_id.

This screenshot is a capture from DB Browser for SQLite:

Expected behavior

References and primary key use also :bigint for SQLite

Actual behavior

References and primary key use :integer for SQLite

System configuration

Rails version: Edge

Ruby version: 2.4.0

Please confirm bug. @sgrif could you also confirm please?

As I said at the above mentioned bug, once confirmed and clarified, I would like to write a fix for this.

Thanks!

[sgrif]

To be clear, on SQLite the primary key must be called exactly INTEGER PRIMARY KEY, or autoincrement doesn't apply. A column of that type will always be 64-bit.

[sgrif]

"A column of that type will always be 64-bit" is also misleading, since really it's based on the value, not the type. Any type that contains the characters "INT" in the type name are treated the same here (with the exception of the primary key which must have exactly the type name INTEGER PRIMARY KEY).

I'm not entirely sure that there's anything to be done here. It sounds like you may be getting confused by what Active Record gives to SQLite, which has little to no effect on the type itself. Can you provide more details on the actual application behavior that you're seeing which differs from what you expect?

[albertoalmagro]

I would like db/schema.rb to also use bigint when using references, as it already happens for MySQL and PostgreSQL. That way db/schema.rb will be the same for all adapters (at least on this topic) and the documentation, which now says that default type is bigint, will be accurate.

[sgrif]

I suspect the intent was to keep the foreign key the same as the primary key (which has to be called integer), leading to exporting :integer here (again, it literally doesn't matter what we do on SQLite here as long as it cobtsins the characters "int" somewhere)

That said, I'll see if there was any particular intent here.

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-1-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[albertoalmagro]

@sgrif lets keep this alive. I have looked up to some references that may help you make an opinion on this:

• In this PR primary keys are changed to BIGINT, except from SQLite as it wasn't supported in theory Change Default Primary Keys to BIGINT #26266
• In this PR it was also applied to references primary_key and references columns should be identical type #27334
• The same for ActiveStorage Use references to respect primary key type in active storage tables #30222
• SQLite documentation says that if we use BIGINT on a create table statement it will be automatically transformed to INTEGER.

[onyxraven]

for sqlite3 - the integer subtypes don't matter - they are all 8 bytes wide in memory

https://sqlite.org/datatype3.html

Forcing primary key to try using BIGINT is actually a problem in sqlite3 because 'INTEGER PRIMARY KEY' is an alias of ROWID, enabling autoincrement behavior.

https://sqlite.org/autoinc.html

I'm currently tracking down some annoyance when schema_dump from mysql and schema_load onto sqlite around this.

[albertoalmagro]

Thats interesting info, thanks for sharing @onyxraven ❤️

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-2-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[onyxraven]

RailsBot reminded me. Here's the patch we put into place in our 5.0 codebase. We're in the process of going to 5.2, so we'll figure out what happens to this approach.

# This patch is to work around a bug in Rails 5.0.x (and 5.1.x)
# Because we use SQLite for tests and MySQL in prod, the 'bigint' modifier for tables
# we've upgraded shows up in the 'create_table' statement in schema.rb. However,
# sqlite has special syntax about the primary key autoincrement - it must be specific
# and the 'bigint' type breaks it.
#
# related: https://github.com/rails/rails/issues/31329, https://github.com/rails/rails/pull/26266
#
# Because of this, we will strip the id type if it is bigint when create_table is called
# Here is the source of create_table, which is included on the specific connection adapters
# https://github.com/rails/rails/blob/5-0-stable/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb#L257
#
# This should be reevaluated for Rails 5.2 ++, which defaults _all_ new tables to bigint
# and changes how this patch would be applied
module TableTypeScrubber
  def self.prepended(_base)
    raise('Please re-validate the monkey patch for schema table type for Rails 5.2.x') if Gem::Version.new(Rails.version) >= Gem::Version.new(5.2)
  end

  def create_table(table_name, comment: nil, **options)
    options.delete(:id) if options&.fetch(:id, nil) == :bigint
    super(table_name, comment: comment, **options)
  end
end

if ActiveRecord::Base.connection.adapter_name.downcase.start_with? 'sqlite'
  module ActiveRecord
    module ConnectionAdapters
      class SQLite3Adapter
        prepend TableTypeScrubber
      end
    end
  end
end

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-2-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[collimarco]

Any updates on this?

I see that create_table t.references creates integer columns instead of bigint! That seems wrong and confusing.

I think that it would be much better to always use bigint for both primary keys and references, regardless of the database adapter.