Commit a004f9ec authored by Yorick Peterse's avatar Yorick Peterse

Added cop to blacklist the use of hash indexes

These indexes are not recorded in the WAL (at least until PostgreSQL
10) and this isn't worth the minor performance improvement over btree
indexes.
parent 7324d86f
require 'set'
require_relative '../../migration_helpers'
module RuboCop
module Cop
module Migration
# Cop that prevents the use of hash indexes in database migrations
class HashIndex < RuboCop::Cop::Cop
include MigrationHelpers
MSG = 'hash indexes should be avoided at all costs since they are not ' \
'recorded in the PostgreSQL WAL, you should use a btree index instead'.freeze
NAMES = Set.new([:add_index, :index, :add_concurrent_index]).freeze
def on_send(node)
return unless in_migration?(node)
name = node.children[1]
return unless NAMES.include?(name)
opts = node.children.last
return unless opts && opts.type == :hash
opts.each_node(:pair) do |pair|
next unless hash_key_type(pair) == :sym &&
hash_key_name(pair) == :using
if hash_key_value(pair).to_s == 'hash'
add_offense(pair, :expression)
end
end
end
def hash_key_type(pair)
pair.children[0].type
end
def hash_key_name(pair)
pair.children[0].children[0]
end
def hash_key_value(pair)
pair.children[1].children[0]
end
end
end
end
end
......@@ -13,6 +13,7 @@ require_relative 'cop/migration/add_concurrent_index'
require_relative 'cop/migration/add_index'
require_relative 'cop/migration/add_timestamps'
require_relative 'cop/migration/datetime'
require_relative 'cop/migration/hash_index'
require_relative 'cop/migration/remove_concurrent_index'
require_relative 'cop/migration/remove_index'
require_relative 'cop/migration/reversible_add_column_with_default'
......
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../../rubocop/cop/migration/hash_index'
describe RuboCop::Cop::Migration::HashIndex do
include CopHelper
subject(:cop) { described_class.new }
context 'in migration' do
before do
allow(cop).to receive(:in_migration?).and_return(true)
end
it 'registers an offense when creating a hash index' do
inspect_source(cop, 'def change; add_index :table, :column, using: :hash; end')
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
end
end
it 'registers an offense when creating a concurrent hash index' do
inspect_source(cop, 'def change; add_concurrent_index :table, :column, using: :hash; end')
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
end
end
it 'registers an offense when creating a hash index using t.index' do
inspect_source(cop, 'def change; t.index :table, :column, using: :hash; end')
aggregate_failures do
expect(cop.offenses.size).to eq(1)
expect(cop.offenses.map(&:line)).to eq([1])
end
end
end
context 'outside of migration' do
it 'registers no offense' do
inspect_source(cop, 'def change; index :table, :column, using: :hash; end')
expect(cop.offenses.size).to eq(0)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment