Skip to content

Comments

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

Merged
ctran merged 1 commit intoctran: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 intoctran:developfrom
jjowdy:develop

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 <[email protected]>
Date:   Thu Mar 30 15:17:37 2023 -0700

    chore: remove broken badges from README.md

commit 10a7a76
Author: Takumi KAGIYAMA <[email protected]>
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 <[email protected]>
    Co-authored-by: Cuong Tran <[email protected]>

commit a28fef3
Author: Jim Jowdy <[email protected]>
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 <[email protected]>
Date:   Wed Mar 29 01:51:15 2023 -0700

    Update codeql-analysis.yml

commit ea4cd00
Author: Lovro Bikić <[email protected]>
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 <[email protected]>

commit 76a1804
Author: Lovro Bikić <[email protected]>
Date:   Wed Mar 29 10:18:24 2023 +0200

    Fix flaky specs (ctran#980)

    Signed-off-by: Lovro Bikic <[email protected]>
jimmynguyc pushed a commit to jimmynguyc/annotate_models that referenced this pull request Oct 29, 2025
@jjowdy @jimmynguyc
Fix retrieve_indexes_from_table when indexes is empty and base table …
cb8ad0c 
…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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ctran ctran ctran approved these changes

Assignees

@ctran ctran

Labels

bug

Projects

None yet

Milestone

v3.2.1

Development

Successfully merging this pull request may close these issues.

3 participants

@jjowdy @rajyan @ctran