Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2017-07-14 14:19:00 +0300
committerYorick Peterse <yorickpeterse@gmail.com>2017-07-14 14:19:00 +0300
commita004f9ec8c66ea9dcce0e76e28b0a343437c0b16 (patch)
treec45a2964428e5eefa09be4c907deab3a5f59eb6f
parent7324d86fa9034e9f9da720075212cc13ac92094a (diff)
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.
-rw-r--r--rubocop/cop/migration/hash_index.rb51
-rw-r--r--rubocop/rubocop.rb1
-rw-r--r--spec/rubocop/cop/migration/hash_index_spec.rb53
3 files changed, 105 insertions, 0 deletions
diff --git a/rubocop/cop/migration/hash_index.rb b/rubocop/cop/migration/hash_index.rb
new file mode 100644
index 00000000000..2cc59691d84
--- /dev/null
+++ b/rubocop/cop/migration/hash_index.rb
@@ -0,0 +1,51 @@
+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
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index f76144275c9..3fbd5b0163c 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -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'
diff --git a/spec/rubocop/cop/migration/hash_index_spec.rb b/spec/rubocop/cop/migration/hash_index_spec.rb
new file mode 100644
index 00000000000..9a8576a19e5
--- /dev/null
+++ b/spec/rubocop/cop/migration/hash_index_spec.rb
@@ -0,0 +1,53 @@
+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