djezzzl/database_consistency

EnumValueChecker doesn't work with ActiveRecord below 7

Closed this issue · 4 comments

I noticed that the EnumValueChecker doesn't work with Rails versions below 7 because enum_types doesn't exist yet in those versions.

database_consistency/lib/database_consistency/checkers/column_checkers/enum_value_checker.rb:40:in 
`enum_column_values': undefined method `enum_types' for 
#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x000000010839ef50 
...

I suggest that the gem should either not run the EnumValueChecker for ActiveRecord versions below 7, or explicitly mention this limitation in the documentation.

Additionally, I'm using the activerecord-postgres_enum gem in my Rails 6 app, and it redefines enum_types. This also causes the EnumValueChecker to break in the DatabaseConsistency gem. I'm not sure if there's anything that can or should be done about this, but it might be worth mentioning in the documentation as well.

ActiveRecord 7:

3.1.3 :001 > model.connection.enum_types
 => [["article_status", "draft,published"]] 

activerecord-postgres_enum

3.1.3 :001 > model.connection.enum_types
 => {:article_status=>["draft", "published"]} 

Hey @toydestroyer,

Very good catch! Thank you for fixing it! I'm merging and releasing it right away.

Please keep it up!

BTW, you might find my research on B-Tree vs Hash interesting. I think I would like to create a checker based on it, what do you think?

Released in 1.7.1. Please try it out and feel free to reopen the issue if needed! Well done again!

I think I would like to create a checker based on it, what do you think?

Sounds pretty cool! How can you determine that only the equality operator is used? 🤔

P. S. Can move it to another issue/discussion

Sounds pretty cool! How can you determine that only the equality operator is used? thinking

I would love the tool to access production data or its gathered statistics. Unfortunately, we can't count on this.

So my idea was to politely suggest with the provided rule of thumb and let engineers decide whether this is the right choice. They can ignore the suggestion, right? But maybe, it would help to identify this one rare case when the hash index is appropriate, saving them a lot of memory (and some on performance).