Error on composite foreign key constraints #121
Comments
|
@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. |
|
@dmke Would you mind telling me what database you are using? |
|
@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 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
endNow still, I have this issue: I suspect there's some issue with the raw execute sql code: Any thoughts on how I can get around this? I'm using pg 1.5.6, Rails 7.1.3.4. |
Oh don't worry :)
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 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
endResulting in the following 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"
endWhen 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
endThat 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 |
This is the kind of advanced schema manipulation which requires switching from config.active_record.schema_format = :sql
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 [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 |
This PR adds support for composite foreign keys. Credit goes to @carldr for ctran/annotate_models#1013. Closes #121.
|
Thanks for testing it out. I'll cut a new release with the change soon. |
|
@dmke just a fyi, I pushed v4.10.0 to Rubygems and it contains the change to support composite foreign keys. |
I have a schema with the following FK constraint setup:
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_idmatches on both the document and customer (i.e.document.tenant_id == document.customer.tenant_id).Commands
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:
Possible Solution
Splatting the
fk.columnsdid solve the error:Now the annotation is updated, but the display is a bit wonky. I'm not sure how improve that, though :)
Version
The text was updated successfully, but these errors were encountered: