Error on composite foreign key constraints
Closed this issue · 14 comments
I have a schema with the following FK constraint setup:
class SetupFailure < ActiveRecord::Migration[7.1]
def up
create_table :tenants do |t|
t.string :name, null: false
t.timestamps null: false
end
create_table :customers do |t|
t.belongs_to :tenant, null: false
t.string :name, null: false
t.timestamps null: false
end
create_table :documents do |t|
t.belongs_to :tenant, null: false
t.belongs_to :customer, null: false
t.string :title
t.timestamps null: false
end
# remove FK documents(customer_id) => customers(id)
remove_foreign_key :documents, :customers, column: :customer_id
# replace with documents(tenant_id,customer_id) => customers(tenant_id,id)
execute <<~SQL.squish
alter table documents
add constraint #{connection.send(:foreign_key_name, :documents, column: :customer_id)}
foreign key (tenant_id,customer_id)
references customers(tenant_id, id)
SQL
end
end
The application supports multi-tenancy; tenants have customers, and customers have documents.
We use this FK to ensure on the DB layer, that the tenant_id
matches on both the document and customer (i.e. document.tenant_id == document.customer.tenant_id
).
Commands
$ bundle exec annotaterb models
bundler: failed to load command: annotaterb (GEM_PATH/bin/annotaterb)
GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/foreign_key_annotation/annotation_builder.rb:34:in `sort_by': comparison of Array with Array failed (ArgumentError)
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/foreign_key_annotation/annotation_builder.rb:34:in `build'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/annotation_builder.rb:44:in `build'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/project_annotator.rb:42:in `build_instructions_for_file'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/project_annotator.rb:17:in `block in annotate'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/project_annotator.rb:13:in `map'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/project_annotator.rb:13:in `annotate'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/annotator.rb:21:in `do_annotations'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/model_annotator/annotator.rb:8:in `do_annotations'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/commands/annotate_models.rb:17:in `call'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/runner.rb:24:in `run'
from GEM_PATH/gems/annotaterb-4.9.0/lib/annotate_rb/runner.rb:7:in `run'
from GEM_PATH/gems/annotaterb-4.9.0/exe/annotaterb:18:in `<top (required)>'
from GEM_PATH/bin/annotaterb:25:in `load'
from GEM_PATH/bin/annotaterb:25:in `<top (required)>'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/cli/exec.rb:58:in `load'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/cli/exec.rb:58:in `kernel_load'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/cli/exec.rb:23:in `run'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/cli.rb:492:in `exec'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/cli.rb:34:in `dispatch'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/cli.rb:28:in `start'
from GEM_PATH/gems/bundler-2.4.19/exe/bundle:37:in `block in <top (required)>'
from GEM_PATH/gems/bundler-2.4.19/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors'
from GEM_PATH/gems/bundler-2.4.19/exe/bundle:29:in `<top (required)>'
from GEM_PATH/bin/bundle:23:in `load'
from GEM_PATH/bin/bundle:23:in `<main>'
Debugging
I've added a debug-print to foreign_key_annotation/annotation_builder.rb:34
:
...
max_size = foreign_keys.map(&format_name).map(&:size).max + 1
# debug
puts "-- #{@model.table_name} --"
foreign_keys.map { |fk| pp [format_name.call(fk), fk.column] }
# /debug
foreign_keys.sort_by { |fk| [format_name.call(fk), fk.column] }.each do |fk|
...
That gave the following output:
-- documents --
["fk_rails_...", ["tenant_id", "customer_id"]]
["fk_rails_...", "tenant_id"]
Possible Solution
Splatting the fk.columns
did solve the error:
foreign_keys.sort_by { |fk| [format_name.call(fk), *fk.column] }.each do |fk|
Now the annotation is updated, but the display is a bit wonky. I'm not sure how improve that, though :)
# ...
# Foreign Keys
#
# fk_rails_... (tenant_id => tenants.id)
# fk_rails_... (["tenant_id", "customer_id"] => customers.["tenant_id", "id"])
Version
- annotaterb version: 4.9.0
- rails version: 7.1.3.3
- ruby version: 3.0
@dmke woah, thanks for this awesome write up + explanation + possible solutions. Let me try and replicate the issue so I can better understand what is going on.
@drwl, I'm using PostgreSQL; any currently supported PostgreSQL version will do (I'm currently on 14.11).
I believe (haven't tested) both MariaDB and SQLite also allow to reference multiple columns in a foreign key, although it looks like having a column tuple as source in SQLite isn't allowed.
@dmke apologies for the delay. I was preparing for interviews and also got sick recently.
I'm trying to replicate the setup but having issues with the SetupFailure
migration. So far, I found that I needed to add a foreign_key: true
to the customer reference in documents table:
create_table :documents do |t|
t.belongs_to :tenant, null: false
t.belongs_to :customer, null: false, foreign_key: true
t.string :title
t.timestamps null: false
end
Now still, I have this issue:
Caused by:
PG::InvalidForeignKey: ERROR: there is no unique constraint matching given keys for referenced table "customers"
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:55:in `exec'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:55:in `block (2 levels) in raw_execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/abstract_adapter.rb:1028:in `block in with_raw_connection'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activesupport-7.1.3.4/lib/active_support/concurrency/null_lock.rb:9:in `synchronize'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/abstract_adapter.rb:1000:in `with_raw_connection'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:54:in `block in raw_execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activesupport-7.1.3.4/lib/active_support/notifications/instrumenter.rb:58:in `instrument'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/abstract_adapter.rb:1143:in `log'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:53:in `raw_execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:521:in `internal_execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:131:in `execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/abstract/query_cache.rb:25:in `execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/connection_adapters/postgresql/database_statements.rb:47:in `execute'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/migration/default_strategy.rb:10:in `method_missing'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/migration.rb:1047:in `block in method_missing'
/Users/leea/.rbenv/versions/3.0.5/lib/ruby/gems/3.0.0/gems/activerecord-7.1.3.4/lib/active_record/migration.rb:1017:in `block in say_with_time'
I suspect there's some issue with the raw execute sql code:
execute <<~SQL.squish
alter table documents
add constraint #{connection.send(:foreign_key_name, :documents, column: :customer_id)}
foreign key (tenant_id,customer_id)
references customers(tenant_id, id)
SQL
Any thoughts on how I can get around this? I'm using pg 1.5.6, Rails 7.1.3.4.
apologies for the delay
Oh don't worry :)
I'm trying to replicate the setup but having issues with the
SetupFailure
migration
Hm, I'll look into that.
I've extracted the migration from one of our production apps (maybe a little hastily). It could very well be that I've missed something.
It took some time, but the problem was essentially a missing unique constraint on documents(tenant_id, id)
(composite FK source and destination columns must be unique).
The corrected migration looks like this:
class SetupFailure < ActiveRecord::Migration[7.1]
def up
create_table :tenants do |t|
t.string :name, null: false
t.timestamps null: false
end
create_table :customers do |t|
t.belongs_to :tenant, null: false, foreign_key: true
t.string :name, null: false
t.timestamps null: false
t.index %i[tenant_id id], unique: true
end
create_table :documents do |t|
t.belongs_to :tenant, null: false, foreign_key: true
t.belongs_to :customer, null: false, foreign_key: true
t.string :title
t.timestamps null: false
t.index %i[tenant_id id], unique: true
end
# remove FK documents(customer_id) => customers(id)
remove_foreign_key :documents, :customers, column: :customer_id
# replace with documents(tenant_id,customer_id) => customers(tenant_id,id)
execute <<~SQL.squish
alter table documents
add constraint #{connection.send(:foreign_key_name, :documents, column: :customer_id)}
foreign key (tenant_id, customer_id)
references customers(tenant_id, id)
SQL
end
end
@dmke so this migration seems to run but not give the expected output... I think.
This is the migration I'm running:
class AddSetupFailure < ActiveRecord::Migration[7.0]
def up
create_table :tenants do |t|
t.string :name, null: false
t.timestamps null: false
end
create_table :customers do |t|
t.belongs_to :tenant, null: false, foreign_key: true
t.string :name, null: false
t.timestamps null: false
t.index %i[tenant_id id], unique: true
end
create_table :documents do |t|
t.belongs_to :tenant, null: false, foreign_key: true
t.belongs_to :customer, null: false, foreign_key: true
t.string :title
t.timestamps null: false
t.index %i[tenant_id id], unique: true
end
# remove FK documents(customer_id) => customers(id)
remove_foreign_key :documents, :customers, column: :customer_id
# replace with documents(tenant_id,customer_id) => customers(tenant_id,id)
execute <<~SQL.squish
alter table documents
add constraint #{connection.send(:foreign_key_name, :documents, column: :customer_id)}
foreign key (tenant_id, customer_id)
references customers(tenant_id, id)
SQL
end
def down
drop_table :documents
drop_table :customers
drop_table :tenants
end
end
Resulting in the following schema.rb
:
ActiveRecord::Schema[7.0].define(version: 2024_06_16_002409) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
create_table "customers", force: :cascade do |t|
t.bigint "tenant_id", null: false
t.string "name", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["tenant_id", "id"], name: "index_customers_on_tenant_id_and_id", unique: true
t.index ["tenant_id"], name: "index_customers_on_tenant_id"
end
create_table "documents", force: :cascade do |t|
t.bigint "tenant_id", null: false
t.bigint "customer_id", null: false
t.string "title"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["customer_id"], name: "index_documents_on_customer_id"
t.index ["tenant_id", "id"], name: "index_documents_on_tenant_id_and_id", unique: true
t.index ["tenant_id"], name: "index_documents_on_tenant_id"
end
create_table "tenants", force: :cascade do |t|
t.string "name", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end
add_foreign_key "customers", "tenants"
add_foreign_key "documents", "customers", column: "tenant_id", primary_key: "tenant_id"
add_foreign_key "documents", "tenants"
end
When I run annotaterb it runs successfully (without errors), but gives the wrong output:
# app/models/document.rb
# == Schema Information
#
# Table name: documents
#
# id :bigint not null, primary key
# title :string
# created_at :datetime not null
# updated_at :datetime not null
# customer_id :bigint not null
# tenant_id :bigint not null
#
# Indexes
#
# index_documents_on_customer_id (customer_id)
# index_documents_on_tenant_id (tenant_id)
# index_documents_on_tenant_id_and_id (tenant_id,id) UNIQUE
#
# Foreign Keys
#
# fk_rails_... (tenant_id => tenants.id)
# fk_rails_... (tenant_id => customers.tenant_id)
#
class Document < ApplicationRecord
end
That being said, I think I have enough information from your initial report on how to make changes. In terms of output, what do you think about?
# ...
# Foreign Keys
#
# fk_rails_... (tenant_id => tenants.id)
# fk_rails_... ([tenant_id, customer_id] => customers[tenant_id, id])
Looks like there's a PR in the old gem I can also reference: ctran/annotate_models#1013
Resulting in the following
schema.rb
:
This is the kind of advanced schema manipulation which requires switching from db/schema.rv
to db/structure.sql
. You need to add the following to your config/application.rb
:
config.active_record.schema_format = :sql
In terms of output, what do you think about?
Looks good :)
@dmke thanks for the suggestion about changing schema format option, I wasn't aware of that.
Just checking did you have any special relationship code in any of the models? I was able to successfully run migrations again and populate a structure.sql
but still seeing the same foreign key definition (from my understanding it populates data from postgres)
[1] pry(#<AnnotateRb::ModelAnnotator::ForeignKeyAnnotation::AnnotationBuilder>)> foreign_keys
=> [#<struct ActiveRecord::ConnectionAdapters::ForeignKeyDefinition
from_table="documents",
to_table="tenants",
options=
{:column=>"tenant_id",
:name=>"fk_rails_5ca55da786",
:primary_key=>"id",
:on_delete=>nil,
:on_update=>nil,
:deferrable=>false,
:validate=>true}>,
#<struct ActiveRecord::ConnectionAdapters::ForeignKeyDefinition
from_table="documents",
to_table="customers",
options=
{:column=>"tenant_id", # <-- expecting this to be an array
:name=>"fk_rails_8b49d7b757",
:primary_key=>"tenant_id",
:on_delete=>nil,
:on_update=>nil,
:deferrable=>false,
:validate=>true}>]
Hmm... apart from the obvious has_many
/belongs_to
calls in the Document and Customer models I don't think there was any special setup. I'd need to double check when I'm back in the office.
Thanks for testing it out. I'll cut a new release with the change soon.