Skip to content

Fixes #471 - Tolerate absent duplicate indexes - #472

Open
jakduch wants to merge 1 commit into
Dynflow:masterfrom
jakduch:fix/471-missing-duplicate-index
Open

Fixes #471 - Tolerate absent duplicate indexes#472
jakduch wants to merge 1 commit into
Dynflow:masterfrom
jakduch:fix/471-missing-duplicate-index

Conversation

@jakduch

@jakduch jakduch commented Aug 19, 2026

Copy link
Copy Markdown

Summary

Fixes #471.

Related: Foreman Redmine #31736.

Migration 020 unconditionally dropped indexes that duplicate primary-key constraints. When one of those indexes was already absent, the migration aborted instead of accepting the schema as already being in the desired state.

Check the indexes reported by Sequel before dropping each duplicate. This keeps the migration portable across the supported PostgreSQL and SQLite adapters and allows it to continue removing the remaining duplicate indexes.

Add a regression test that migrates through version 19, removes one duplicate index, and verifies that migration 20 succeeds and removes the other two.

Testing

  • test/persistence_migrations_test.rb: 1 run, 6 assertions, 0 failures, 0 errors

AI usage disclosure

Per the community discussion on AI policy, the issue was investigated and the changes, tests, and PR wording were prepared with the assistance of Codex 5.6 Sol High. The resulting changes were reviewed before submitting. The commit also includes an Assisted-By trailer.

Migration 020 now treats an already missing duplicate index as the desired state and continues removing the remaining redundant indexes.

Assisted-By: Codex 5.6 Sol High

@adamruzicka adamruzicka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

471? Now this is some stuff of the legend.

One comment inline

up do
alter_table(:dynflow_actions) do
drop_index [:execution_plan_uuid, :id]
if indexes(:dynflow_actions).key?(:dynflow_actions_execution_plan_uuid_id_index)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the actual dropping, we rely on sequel to derive the index name from the table and the columns. Could we reuse that function rather than using the exact index name?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good point. default_index_name(table, Array(columns)).to_sym will keep the existence check aligned with the name Sequel uses in drop_index. I’ll update it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate index migration fails when an index is already absent

2 participants