From 28847b642e7cc0d4b836f6a1ae7d36ef6c89388b Mon Sep 17 00:00:00 2001 From: Alexey Volosenko Date: Fri, 5 Jul 2019 16:38:13 +0300 Subject: [PATCH] Updated logic to correctly dumping single column indexes with an alternate order. diff --git a/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb b/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb index fd91fdb..7055acc 100644 --- a/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb +++ b/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb @@ -9,6 +9,12 @@ module ActiveRecord # :nodoc: # Regex to find where clause in index statements INDEX_WHERE_EXPRESSION = /WHERE (.+)$/ + # Taken from https://github.com/postgres/postgres/blob/master/src/include/catalog/pg_index.h#L75 + # Values are in reverse order + INDOPTION_DESC = 1 + # NULLs are first instead of last + INDOPTION_NULLS_FIRST = 2 + # Returns the list of all tables in the schema search path or a specified schema. # # == Patch: @@ -71,7 +77,14 @@ module ActiveRecord # :nodoc: schemas = schema ? "ARRAY['#{schema}']" : 'current_schemas(false)' result = query(<<-SQL, name) - SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid, am.amname, d.indclass + SELECT distinct i.relname, + d.indisunique, + d.indkey, + pg_get_indexdef(d.indexrelid), + t.oid, + am.amname, + d.indclass, + d.indoption FROM pg_class t INNER JOIN pg_index d ON t.oid = d.indrelid INNER JOIN pg_class i ON d.indexrelid = i.oid @@ -91,7 +104,8 @@ module ActiveRecord # :nodoc: :definition => row[3], :id => row[4], :access_method => row[5], - :operators => row[6].split(" ") + :operators => row[6].split(" "), + :options => row[7].split(" ").map(&:to_i) } column_names = find_column_names(table_name, index) @@ -140,6 +154,25 @@ module ActiveRecord # :nodoc: remove_type(functional_name) end end + else + # In case if column_names if not empty it contains list of column name taken from pg_attribute table. + # So we need to check indoption column and add DESC and NULLS LAST based on its value. + # https://stackoverflow.com/questions/18121103/how-to-get-the-index-column-orderasc-desc-nulls-first-from-postgresql/18128457#18128457 + column_names = column_names.map.with_index do |column_name, column_index| + option = index[:options][column_index] + + if option != 0 + column_name << " DESC" if option & INDOPTION_DESC > 0 + + if option & INDOPTION_NULLS_FIRST > 0 + column_name << " NULLS FIRST" + else + column_name << " NULLS LAST" + end + end + + column_name + end end column_names diff --git a/pg_saurus.iml b/pg_saurus.iml new file mode 100644 index 0000000..9cfe971 --- /dev/null +++ b/pg_saurus.iml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/spec/active_record/schema_dumper_spec.rb b/spec/active_record/schema_dumper_spec.rb index b0b5aca..d713ae0 100644 --- a/spec/active_record/schema_dumper_spec.rb +++ b/spec/active_record/schema_dumper_spec.rb @@ -102,6 +102,10 @@ describe ActiveRecord::SchemaDumper do @dump.should =~ /add_index "books", \["title varchar_pattern_ops"\]/ end + it "dumps indexes with simple columns with an alternate order" do + @dump.should =~ /add_index "books", \["author_id DESC NULLS FIRST", "publisher_id DESC NULLS LAST"\]/ + end + it "dumps functional indexes with longer operator strings" do @dump.should =~ /add_index "pets", \["lower\(name\) DESC NULLS LAST"\]/ end diff --git a/spec/dummy/.generators b/spec/dummy/.generators new file mode 100644 index 0000000..1618976 --- /dev/null +++ b/spec/dummy/.generators @@ -0,0 +1,8 @@ + + diff --git a/spec/dummy/db/migrate/20190213151821_create_books.rb b/spec/dummy/db/migrate/20190213151821_create_books.rb index 50a0da2..839b792 100644 --- a/spec/dummy/db/migrate/20190213151821_create_books.rb +++ b/spec/dummy/db/migrate/20190213151821_create_books.rb @@ -1,12 +1,17 @@ class CreateBooks < ActiveRecord::Migration def change create_table :books do |t| + t.integer :author_id + t.integer :publisher_id t.string :title t.json :tags t.timestamps end + add_index :books, ["author_id DESC NULLS FIRST", "publisher_id DESC NULLS LAST"], + name: "books_author_id_and_publisher_id" + add_index :books, "title varchar_pattern_ops" add_index :books, "((tags->'attrs'->>'edition')::int)", name: "books_tags_json_index", skip_column_quoting: true diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 30bcb04..7c9bc32 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -43,6 +43,8 @@ ActiveRecord::Schema.define(version: 20190320025645) do FUNCTION_DEFINITION create_table "books", force: :cascade do |t| + t.integer "author_id" + t.integer "publisher_id" t.string "title" t.json "tags" t.datetime "created_at" @@ -50,6 +52,7 @@ ActiveRecord::Schema.define(version: 20190320025645) do end add_index "books", "((((tags -> 'attrs'::text) ->> 'edition'::text))::integer)", :name => "books_tags_json_index", :skip_column_quoting => true + add_index "books", ["author_id DESC NULLS FIRST", "publisher_id DESC NULLS LAST"], :name => "books_author_id_and_publisher_id" add_index "books", ["title varchar_pattern_ops"], :name => "index_books_on_title_varchar_pattern_ops" create_table "breeds", force: :cascade do |t| --- .../connection_adapters/postgresql_adapter.rb | 37 ++++++++++++++++++- spec/active_record/schema_dumper_spec.rb | 4 ++ .../db/migrate/20190213151821_create_books.rb | 5 +++ spec/dummy/db/schema.rb | 3 ++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb b/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb index fd91fdb..061c75e 100644 --- a/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb +++ b/lib/core_ext/active_record/connection_adapters/postgresql_adapter.rb @@ -9,6 +9,12 @@ class PostgreSQLAdapter # Regex to find where clause in index statements INDEX_WHERE_EXPRESSION = /WHERE (.+)$/ + # Taken from https://github.com/postgres/postgres/blob/master/src/include/catalog/pg_index.h#L75 + # Values are in reverse order + INDOPTION_DESC = 1 + # NULLs are first instead of last + INDOPTION_NULLS_FIRST = 2 + # Returns the list of all tables in the schema search path or a specified schema. # # == Patch: @@ -71,7 +77,14 @@ def indexes(table_name, name = nil) schemas = schema ? "ARRAY['#{schema}']" : 'current_schemas(false)' result = query(<<-SQL, name) - SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), t.oid, am.amname, d.indclass + SELECT distinct i.relname, + d.indisunique, + d.indkey, + pg_get_indexdef(d.indexrelid), + t.oid, + am.amname, + d.indclass, + d.indoption FROM pg_class t INNER JOIN pg_index d ON t.oid = d.indrelid INNER JOIN pg_class i ON d.indexrelid = i.oid @@ -91,7 +104,8 @@ def indexes(table_name, name = nil) :definition => row[3], :id => row[4], :access_method => row[5], - :operators => row[6].split(" ") + :operators => row[6].split(" "), + :options => row[7].split(" ").map(&:to_i) } column_names = find_column_names(table_name, index) @@ -140,6 +154,25 @@ def find_column_names(table_name, index) remove_type(functional_name) end end + else + # In case if column_names if not empty it contains list of column name taken from pg_attribute table. + # So we need to check indoption column and add DESC and NULLS LAST based on its value. + # https://stackoverflow.com/questions/18121103/how-to-get-the-index-column-orderasc-desc-nulls-first-from-postgresql/18128457#18128457 + column_names = column_names.map.with_index do |column_name, column_index| + option = index[:options][column_index] + + if option != 0 + column_name << " DESC" if option & INDOPTION_DESC > 0 + + if option & INDOPTION_NULLS_FIRST > 0 + column_name << " NULLS FIRST" + else + column_name << " NULLS LAST" + end + end + + column_name + end end column_names diff --git a/spec/active_record/schema_dumper_spec.rb b/spec/active_record/schema_dumper_spec.rb index b0b5aca..d713ae0 100644 --- a/spec/active_record/schema_dumper_spec.rb +++ b/spec/active_record/schema_dumper_spec.rb @@ -102,6 +102,10 @@ @dump.should =~ /add_index "books", \["title varchar_pattern_ops"\]/ end + it "dumps indexes with simple columns with an alternate order" do + @dump.should =~ /add_index "books", \["author_id DESC NULLS FIRST", "publisher_id DESC NULLS LAST"\]/ + end + it "dumps functional indexes with longer operator strings" do @dump.should =~ /add_index "pets", \["lower\(name\) DESC NULLS LAST"\]/ end diff --git a/spec/dummy/db/migrate/20190213151821_create_books.rb b/spec/dummy/db/migrate/20190213151821_create_books.rb index 50a0da2..839b792 100644 --- a/spec/dummy/db/migrate/20190213151821_create_books.rb +++ b/spec/dummy/db/migrate/20190213151821_create_books.rb @@ -1,12 +1,17 @@ class CreateBooks < ActiveRecord::Migration def change create_table :books do |t| + t.integer :author_id + t.integer :publisher_id t.string :title t.json :tags t.timestamps end + add_index :books, ["author_id DESC NULLS FIRST", "publisher_id DESC NULLS LAST"], + name: "books_author_id_and_publisher_id" + add_index :books, "title varchar_pattern_ops" add_index :books, "((tags->'attrs'->>'edition')::int)", name: "books_tags_json_index", skip_column_quoting: true diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index 30bcb04..7c9bc32 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -43,6 +43,8 @@ FUNCTION_DEFINITION create_table "books", force: :cascade do |t| + t.integer "author_id" + t.integer "publisher_id" t.string "title" t.json "tags" t.datetime "created_at" @@ -50,6 +52,7 @@ end add_index "books", "((((tags -> 'attrs'::text) ->> 'edition'::text))::integer)", :name => "books_tags_json_index", :skip_column_quoting => true + add_index "books", ["author_id DESC NULLS FIRST", "publisher_id DESC NULLS LAST"], :name => "books_author_id_and_publisher_id" add_index "books", ["title varchar_pattern_ops"], :name => "index_books_on_title_varchar_pattern_ops" create_table "breeds", force: :cascade do |t|