Skip to content

Fix retrieve_indexes_from_table when indexes is empty and base table does not exist - #849

Merged
ctran merged 1 commit into
ctran:developfrom
jjowdy:develop
Mar 29, 2023
Merged

Fix retrieve_indexes_from_table when indexes is empty and base table does not exist#849
ctran merged 1 commit into
ctran:developfrom
jjowdy:develop

Conversation

@jjowdy

@jjowdyjjowdy commented Jan 27, 2021

Copy link
Copy Markdown
Contributor

Some tables may have a table_name_prefix but no indexes. Previous versions of
the code would strip the prefix and look for indexes on the resulting table name,
which likely would not exist. This causes DB errors, at least in MySQL. So now
check if the table with the derived name exists first before trying to show its indexes.

Example error otherwise:

myservice:development [10] pry(main)> ActiveRecord::Base.connection.indexes('garbage')
Mysql2::Error: Table 'myservice_development.garbage' doesn't exist: SHOW KEYS FROM `garbage`
ActiveRecord::StatementInvalid: Mysql2::Error: Table 'myservice_development.garbage' doesn't exist: SHOW KEYS FROM `garbage`

@ctranctran self-assigned this Mar 24, 2021
@ctranctran added the bug label Mar 24, 2021
@ctranctran added this to the v3.2.0 milestone Mar 24, 2021

@ctranctran left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks!

@ctranctran modified the milestones: v3.2.0, v3.2.1Mar 8, 2022
@rajyan

Copy link
Copy Markdown

Is there any chance that this change being merged?
I've encountered this bug recently, and it's very hard to notice the cause from the error message...

@jjowdy

Copy link
Copy Markdown
ContributorAuthor

Is there any chance that this change being merged?
I've encountered this bug recently, and it's very hard to notice the cause from the error message...

I don't think I have write permissions so I can't seem do that myself. @ctran would you have a chance to do this?

@ctran

Copy link
Copy Markdown
Owner

Something is strange about this PR. I don't see the usual checks being triggered on this one.

@ctran

Copy link
Copy Markdown
Owner

Can you rebate and resolve the conflict?

…does not exist.
Some tables may have a table_name_prefix but no indexes. Previous versions of
the code would strip the prefix and look for indexes on the resulting table
which likely would not exist. This causes DB errors, at least in MySQL. So now
check if the new table exists first before trying to show its indexes.
@jjowdy

Copy link
Copy Markdown
ContributorAuthor

Can you rebate and resolve the conflict?

Sure. (That was a bit of a context switch: I no longer have access to the system I made the fix on and hadn't pushed to github in a while.) I wrapped the new spec context with rubocop:disable RSpec/NestedGroups though because I won't have time to address that.

@ctran

Copy link
Copy Markdown
Owner

Thanks!

@rajyan

Copy link
Copy Markdown

Thank you both so much for taking care of this issue!

jimmynguyc pushed a commit to jimmynguyc/annotate_models that referenced this pull request Oct 29, 2025
…does not exist. (ctran#849)
Some tables may have a table_name_prefix but no indexes. Previous versions of
the code would strip the prefix and look for indexes on the resulting table which likely would not exist. This causes DB errors, at least in MySQL. So now
check if the new table exists first before trying to show its indexes.
thatsanicehat pushed a commit to Home-Chef-Tech/annotate_models that referenced this pull request Jun 12, 2026
…does not exist. (ctran#849)
Some tables may have a table_name_prefix but no indexes. Previous versions of
the code would strip the prefix and look for indexes on the resulting table which likely would not exist. This causes DB errors, at least in MySQL. So now
check if the new table exists first before trying to show its indexes.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jjowdy@rajyan@ctran