search

gregnavis / active_record_doctor

Identify database issues before they hit production.
MIT License
1.76k stars 55 forks source link

Failing to ignore missing_unique_indexes for has_one #152

Closed raivil closed 11 months ago

raivil commented 12 months ago

Example:

We Citus DB (Postgresql distributed) and basically all keys are composite and will include the partitioning key.

Example: Table "public.orders" has a payment_method_id and an unique index that includes the partitioning key.

"index_orders_on_payment_method_id_and_account_id" UNIQUE, btree (payment_method_id, account_id)

ARDoctor is failing to ignore that column. 

# This code works on version 1.10.0
detector :missing_unique_indexes,
           ignore_columns: %w(payment_method_id)
# This is the error I get on 1.12.0
add a unique index on orders(payment_method_id) - using `has_one` in Orders::PaymentMethod without an index can lead to duplicates
fatkodima commented 12 months ago

Did ignore_columns: %w(payment_method_id) worked 🤔 ? Or you mean ignore_columns: ["Order(payment_method_id)] ?

Can you share a sample code of models, indexes, exactly what worked before and what is not working now?

raivil commented 12 months ago

Works on 1.10.0 but doesn't work on 1.12.0

ignore_columns: %w(payment_method_id)

Doesn't work on 1.12.0 or 1.10.0.

ignore_columns: ["Order(payment_method_id)"]

Models are quite complicated and I can't share them all. Were' using gem "activerecord-multi-tenant", "~> 2.3.0" for the multitenancy, which might be related.

snippet of the table orders:

Indexes:
    "orders_pkey" PRIMARY KEY, btree (account_id, id)
    "index_orders_on_account_id_and_external_id" UNIQUE, btree (account_id, external_id)
    "index_orders_on_payment_method_id_and_account_id" UNIQUE, btree (payment_method_id, account_id)
Foreign-key constraints:
    "fk_orders_account_orders_payment_methods" FOREIGN KEY (payment_method_id, account_id) REFERENCES orders_payment_methods(id, account_id) DEFERRABLE
fatkodima commented 12 months ago

Opened a PR with the fix - https://github.com/gregnavis/active_record_doctor/pull/153

Checks for has_one associations were introduced in https://github.com/gregnavis/active_record_doctor/pull/122. For has_ones, you need to use ModelName(column1,..) format. So with that fix, ignore_columns: ["Order(payment_method_id)"] should work.

raivil commented 12 months ago

@fatkodima tested the branch and it works as you mentioned. Thank you!