search

plentz / lol_dba

lol_dba is a small package of rake tasks that scan your application models and displays a list of columns that probably should be indexed. Also, it can generate .sql migration scripts.
1.59k stars 69 forks source link

Exception with indexes that are represented as strings rather than arrays #147

Open timdiggins opened 8 months ago

timdiggins commented 8 months ago

Given you've got a postgresql table using full text search with a gin index on two columns: text and internal_notes

Which would be represented by rails in schema.rb as 

    t.index "((to_tsvector('simple'::regconfig, COALESCE((name)::text, ''::text)) || to_tsvector('simple'::regconfig, COALESCE(internal_notes, ''::text))))", name: "index_clients_for_free_text_search", using: :gin

When you try to use if you use rake lol_dba find_indexes on this rails app - It will fail with an exception.

Full traceback: ``` /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:66:in `block in table_indexes' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:60:in `collect' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:60:in `table_indexes' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:55:in `existing_indexes' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:44:in `block in missing_indexes' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:42:in `each' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:42:in `missing_indexes' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:37:in `check_for_indexes' /data/lol_dba/lib/lol_dba/index_finding/index_finder.rb:4:in `run' /data/lol_dba/lib/tasks/lol_dba.rake:6:in `block (2 levels) in
' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:281:in `block in execute' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:281:in `each' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:281:in `execute' /data/project/Rakefile:12:in `block in execute_with_benchmark' /data/ruby/3.0.6/lib/ruby/3.0.0/benchmark.rb:293:in `measure' /data/project/Rakefile:12:in `execute_with_benchmark' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/bugsnag-6.26.0/lib/bugsnag/integrations/rake.rb:20:in `execute' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:219:in `block in invoke_with_call_chain' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:199:in `synchronize' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:199:in `invoke_with_call_chain' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/task.rb:188:in `invoke' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:182:in `invoke_task' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:138:in `block (2 levels) in top_level' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:138:in `each' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:138:in `block in top_level' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:147:in `run_with_threads' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:132:in `top_level' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:83:in `block in run' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:208:in `standard_exception_handling' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/rake-13.1.0/lib/rake/application.rb:80:in `run' /data/project/bin/rake:5:in `
' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:10:in `block in fork' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:10:in `block in fork' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:8:in `fork' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:8:in `fork' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:27:in `fork' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:8:in `fork' /data/ruby/3.0.6/lib/ruby/gems/3.0.0/gems/activesupport-6.1.7.6/lib/active_support/fork_tracker.rb:27:in `fork' :85:in `require' :85:in `require' ```

This is because in the internal API ActiveRecord::Base.connection.indexes(table_name) will return an index with has an accessor columns that returns this string, NOT an array.

This can be fixed by testing for the type of index.columns before returning:

        if index.columns.is_a?(String)
          # eg. gin, tsvector...
          index.columns
        else
          index.columns.size > 1 ? index.columns.sort : index.columns.first
        end

https://github.com/plentz/lol_dba/blob/master/lib/lol_dba/index_finding/index_finder.rb#L61

I would contribute a PR, but am having trouble running the specs locally

timdiggins commented 8 months ago

draft PR @ https://github.com/plentz/lol_dba/pull/148 - but needs a spec writing for this case