Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 29, 2023
Merged

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

merged 1 commit into from
Mar 29, 2023

Conversation

jjowdy
Copy link
Contributor

@jjowdy jjowdy commented Jan 27, 2021 •
edited
Loading

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`
@ctran ctran self-assigned this Mar 24, 2021
@ctran ctran added the bug label Mar 24, 2021
@ctran ctran added this to the v3.2.0 milestone Mar 24, 2021
ctran
ctran approved these changes Feb 3, 2022
View reviewed changes
Copy link
Owner

@ctran ctran left a comment

Choose a reason for hiding this comment

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

Thanks!
@ctran ctran modified the milestones: v3.2.0, v3.2.1 Mar 8, 2022
@rajyan
Copy link

rajyan commented Mar 27, 2023

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
Contributor Author

jjowdy commented Mar 27, 2023

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
Owner

ctran commented Mar 29, 2023

Something is strange about this PR. I don't see the usual checks being triggered on this one.
@ctran
Copy link
Owner

ctran commented Mar 29, 2023

Can you rebate and resolve the conflict?
@jjowdy
Fix retrieve_indexes_from_table when indexes is empty and base table …
65872e8 
…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.
@ctran ctran merged commit a28fef3 into ctran:develop Mar 29, 2023
@jjowdy
Copy link
Contributor Author

jjowdy commented Mar 29, 2023

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
Owner

ctran commented Mar 29, 2023

Thanks!
@rajyan
Copy link

rajyan commented Mar 29, 2023

Thank you both so much for taking care of this issue!
matteolc pushed a commit to matteolc/annotate_models that referenced this pull request Mar 31, 2023
Squashed commit of the following:
7a15b5c 
commit 22ab676 Author: Cuong Tran <ctran@users.noreply.github.com> Date: Thu Mar 30 15:17:37 2023 -0700 chore: remove broken badges from README.md commit 10a7a76 Author: Takumi KAGIYAMA <694547+kg8m@users.noreply.github.com> Date: Fri Mar 31 07:15:07 2023 +0900 Support `--frozen` option for routing annotations (ctran#979) The `--frozen` option previously deal only model annotations. This change will support route annotations as well. --------- Signed-off-by: kg8m <takumi.kagiyama@gmail.com> Co-authored-by: Cuong Tran <ctran@users.noreply.github.com> commit a28fef3 Author: Jim Jowdy <jjowdy@gmail.com> Date: Wed Mar 29 15:09:11 2023 -0700 Fix retrieve_indexes_from_table when indexes is empty and base table 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. commit 13b532d Author: Cuong Tran <ctran@users.noreply.github.com> Date: Wed Mar 29 01:51:15 2023 -0700 Update codeql-analysis.yml commit ea4cd00 Author: Lovro Bikić <lovro.bikic@infinum.hr> Date: Wed Mar 29 10:31:01 2023 +0200 Add support for annotating check constraints (ctran#868) This adds annotation of check constraints with an option to disable/enable annotation. Most of the work done in this PR is based off of existing implementation for annotating indexes and foreign keys. Signed-off-by: Lovro Bikic <lovro.bikic@infinum.hr> commit 76a1804 Author: Lovro Bikić <lovro.bikic@gmail.com> Date: Wed Mar 29 10:18:24 2023 +0200 Fix flaky specs (ctran#980) Signed-off-by: Lovro Bikic <lovro.bikic@infinum.hr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug

3 participants

@jjowdy @rajyan @ctran