From 7fc0519d1dc66c54f43050c3271c3ffa639de93d Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 12 Oct 2012 22:30:22 +0200 Subject: trailling whitespace cleanup in query_methods.rb --- .../lib/active_record/relation/query_methods.rb | 24 +++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 2b12e66ed0..ad10174adc 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -202,13 +202,13 @@ module ActiveRecord # # User.order('name DESC, email') # => SELECT "users".* FROM "users" ORDER BY name DESC, email - # + # # User.order(:name) # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC - # + # # User.order(email: :desc) # => SELECT "users".* FROM "users" ORDER BY "users"."email" DESC - # + # # User.order(:name, email: :desc) # => SELECT "users".* FROM "users" ORDER BY "users"."name" ASC, "users"."email" DESC def order(*args) @@ -218,7 +218,7 @@ module ActiveRecord # Like #order, but modifies relation in place. def order!(*args) args.flatten! - + validate_order_args args references = args.reject { |arg| Arel::Node === arg } @@ -245,7 +245,7 @@ module ActiveRecord # Like #reorder, but modifies relation in place. def reorder!(*args) args.flatten! - + validate_order_args args self.reordering_value = true @@ -803,9 +803,9 @@ module ActiveRecord s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') end when Symbol - { o => :desc } + { o => :desc } when Hash - o.each_with_object({}) do |(field, dir), memo| + o.each_with_object({}) do |(field, dir), memo| memo[field] = (dir == :asc ? :desc : :asc ) end else @@ -817,25 +817,25 @@ module ActiveRecord def array_of_strings?(o) o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)} end - + def build_order(arel) orders = order_values orders = reverse_sql_order(orders) if reverse_order_value - + orders = orders.uniq.reject(&:blank?).map do |order| case order when Symbol table[order].asc when Hash order.map { |field, dir| table[field].send(dir) } - else + else order end end.flatten - + arel.order(*orders) unless orders.empty? end - + def validate_order_args(args) args.select { |a| Hash === a }.each do |h| unless (h.values - [:asc, :desc]).empty? -- cgit v1.2.3 From 3a6dfca7f5f5bd45cea2f6ac348178e72423e1d5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 12 Oct 2012 14:34:04 -0700 Subject: Speed up relation merging by reducing calls to Array#- MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit before: Calculating ------------------------------------- ar 83 i/100ms ------------------------------------------------- ar 832.1 (±4.0%) i/s - 4233 in 5.096611s after: Calculating ------------------------------------- ar 87 i/100ms ------------------------------------------------- ar 839.0 (±9.3%) i/s - 4176 in 5.032782s Benchmark: require 'config/environment' require 'benchmark/ips' GC.disable unless User.find_by_login('tater') u = User.new u.login = 'tater' u.save! end def active_record user = User.find_by_login('tater') starred = user.starred_items.count end active_record Benchmark.ips do |x| x.report("ar") { active_record } end --- activerecord/lib/active_record/relation/merger.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index e5b50673da..951eabe427 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -39,10 +39,12 @@ module ActiveRecord @values = other.values end + NORMAL_VALUES = Relation::SINGLE_VALUE_METHODS + + Relation::MULTI_VALUE_METHODS - + [:where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] # :nodoc: + def normal_values - Relation::SINGLE_VALUE_METHODS + - Relation::MULTI_VALUE_METHODS - - [:where, :order, :bind, :reverse_order, :lock, :create_with, :reordering, :from] + NORMAL_VALUES end def merge -- cgit v1.2.3 From db8dbe76db514734082af9e8d166c1b33ea2f39f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 12 Oct 2012 16:59:20 -0700 Subject: performance improvements to joins! MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: Calculating ------------------------------------- ar 87 i/100ms ------------------------------------------------- ar 823.4 (±11.8%) i/s - 4089 in 5.070234s After: Calculating ------------------------------------- ar 88 i/100ms ------------------------------------------------- ar 894.1 (±3.9%) i/s - 4488 in 5.028161s Same test as 3a6dfca7f5f5bd45cea2f6ac348178e72423e1d5 --- activerecord/lib/active_record/relation/merger.rb | 14 ++++++++++++-- activerecord/lib/active_record/relation/query_methods.rb | 4 +--- 2 files changed, 13 insertions(+), 5 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 951eabe427..59226d316e 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -22,7 +22,17 @@ module ActiveRecord # the values. def other other = Relation.new(relation.klass, relation.table) - hash.each { |k, v| other.send("#{k}!", v) } + hash.each { |k, v| + if k == :joins + if Hash === v + other.joins!(v) + else + other.joins!(*v) + end + else + other.send("#{k}!", v) + end + } other end end @@ -50,7 +60,7 @@ module ActiveRecord def merge normal_values.each do |name| value = values[name] - relation.send("#{name}!", value) unless value.blank? + relation.send("#{name}!", *value) unless value.blank? end merge_multi_values diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index ad10174adc..a9ace10093 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -258,13 +258,11 @@ module ActiveRecord # User.joins(:posts) # => SELECT "users".* FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" def joins(*args) - args.compact.blank? ? self : spawn.joins!(*args) + args.compact.blank? ? self : spawn.joins!(*args.flatten) end # Like #joins, but modifies relation in place. def joins!(*args) - args.flatten! - self.joins_values += args self end -- cgit v1.2.3 From 2da85edda36692c882ff25ccf7dbd038de2f771c Mon Sep 17 00:00:00 2001 From: Arturo Pie Date: Fri, 12 Oct 2012 18:03:30 -0400 Subject: #7914 get default value when type uses schema name PostgreSQL adapter properly parses default values when using multiple schemas and domains. When using domains across schemas, PostgresSQL prefixes the type of the default value with the name of the schema where that type (or domain) is. For example, this query: ``` SELECT a.attname, d.adsrc FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum WHERE a.attrelid = "defaults"'::regclass AND a.attnum > 0 AND NOT a.attisdropped ORDER BY a.attnum; ``` could return something like "''::pg_catalog.text" or "('''::pg_catalog.text)::text" for the text columns with defaults. I modified the regexp used to parse this value so that it ignores anything between ':: and \b(?:character varying|bpchar|text), and it allows to have optional parens like in the above second example. --- .../connection_adapters/postgresql_adapter.rb | 2 +- .../test/cases/adapters/postgresql/schema_test.rb | 2 +- activerecord/test/cases/defaults_test.rb | 40 ++++++++++++++++++++++ .../test/schema/postgresql_specific_schema.rb | 9 ++++- 4 files changed, 50 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index bd375ad15a..a8c02ef18c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -78,7 +78,7 @@ module ActiveRecord when /\A\(?(-?\d+(\.\d*)?\)?)\z/ $1 # Character types - when /\A'(.*)'::(?:character varying|bpchar|text)\z/m + when /\A\(?'(.*)'::.*\b(?:character varying|bpchar|text)\z/m $1 # Character types (8.1 formatting) when /\AE'(.*)'::(?:character varying|bpchar|text)\z/m diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index ffbf14b528..cd31900d4e 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -72,7 +72,7 @@ class SchemaTest < ActiveRecord::TestCase end def test_schema_names - assert_equal ["public", "test_schema", "test_schema2"], @connection.schema_names + assert_equal ["public", "schema_1", "test_schema", "test_schema2"], @connection.schema_names end def test_create_schema diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb index deaf5252db..72f1c99ca0 100644 --- a/activerecord/test/cases/defaults_test.rb +++ b/activerecord/test/cases/defaults_test.rb @@ -109,3 +109,43 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter) end end end + +if current_adapter?(:PostgreSQLAdapter) + class DefaultsUsingMultipleSchemasAndDomainTest < ActiveSupport::TestCase + def setup + @connection = ActiveRecord::Base.connection + + @old_search_path = @connection.schema_search_path + @connection.schema_search_path = "schema_1, pg_catalog" + @connection.create_table "defaults" do |t| + t.text "text_col", :default => "some value" + t.string "string_col", :default => "some value" + end + Default.reset_column_information + end + + def test_text_defaults_in_new_schema_when_overriding_domain + assert_equal "some value", Default.new.text_col, "Default of text column was not correctly parse" + end + + def test_string_defaults_in_new_schema_when_overriding_domain + assert_equal "some value", Default.new.string_col, "Default of string column was not correctly parse" + end + + def test_bpchar_defaults_in_new_schema_when_overriding_domain + @connection.execute "ALTER TABLE defaults ADD bpchar_col bpchar DEFAULT 'some value'" + Default.reset_column_information + assert_equal "some value", Default.new.bpchar_col, "Default of bpchar column was not correctly parse" + end + + def test_text_defaults_after_updating_column_default + @connection.execute "ALTER TABLE defaults ALTER COLUMN text_col SET DEFAULT 'some text'::schema_1.text" + assert_equal "some text", Default.new.text_col, "Default of text column was not correctly parse after updating default using '::text' since postgreSQL will add parens to the default in db" + end + + def teardown + @connection.schema_search_path = @old_search_path + Default.reset_column_information + end + end +end diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 2cd9f30b59..d0e7338f15 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -12,6 +12,8 @@ ActiveRecord::Schema.define do execute 'DROP FUNCTION IF EXISTS partitioned_insert_trigger()' + execute "DROP SCHEMA IF EXISTS schema_1 CASCADE" + %w(accounts_id_seq developers_id_seq projects_id_seq topics_id_seq customers_id_seq orders_id_seq).each do |seq_name| execute "SELECT setval('#{seq_name}', 100)" end @@ -37,7 +39,12 @@ ActiveRecord::Schema.define do ); _SQL - execute <<_SQL + execute "CREATE SCHEMA schema_1" + execute "CREATE DOMAIN schema_1.text AS text" + execute "CREATE DOMAIN schema_1.varchar AS varchar" + execute "CREATE DOMAIN schema_1.bpchar AS bpchar" + + execute <<_SQL CREATE TABLE geometrics ( id serial primary key, a_point point, -- cgit v1.2.3 From 54b3f417411097f7404d15355858f56ad66e8294 Mon Sep 17 00:00:00 2001 From: Arturo Pie Date: Fri, 12 Oct 2012 18:07:30 -0400 Subject: #7914 Add change of previous commit to CHANGELOG.md --- activerecord/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 7b80598171..f3362f81a3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,9 @@ ## Rails 4.0.0 (unreleased) ## +* PostgreSQL adapter correctly fetches default values when using multiple schemas and domains in a db. Fixes #7914 + + *Arturo Pie* + * Learn ActiveRecord::QueryMethods#order work with hash arguments When symbol or hash passed we convert it to Arel::Nodes::Ordering. -- cgit v1.2.3 From 40475cf3649af4a6d868299b1796b36358f90917 Mon Sep 17 00:00:00 2001 From: Arturo Pie Date: Sat, 13 Oct 2012 18:30:43 -0400 Subject: #7914 Using a better way to get the defaults from db. According to postgreSQL documentation: (http://www.postgresql.org/docs/8.2/static/catalog-pg-attrdef.html) we should not be using 'adsrc' field because this field is unaware of outside changes that could affect the way that default values are represented. Thus, I changed the queries to use "pg_get_expr(adbin, adrelid)" instead of the historical "adsrc" field. --- .../connection_adapters/postgresql/schema_statements.rb | 10 +++++----- .../active_record/connection_adapters/postgresql_adapter.rb | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 2264595751..0f5431ee37 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -286,10 +286,10 @@ module ActiveRecord result = query(<<-end_sql, 'SCHEMA')[0] SELECT attr.attname, CASE - WHEN split_part(def.adsrc, '''', 2) ~ '.' THEN - substr(split_part(def.adsrc, '''', 2), - strpos(split_part(def.adsrc, '''', 2), '.')+1) - ELSE split_part(def.adsrc, '''', 2) + WHEN split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2) ~ '.' THEN + substr(split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2), + strpos(split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2), '.')+1) + ELSE split_part(pg_get_expr(def.adbin, def.adrelid), '''', 2) END FROM pg_class t JOIN pg_attribute attr ON (t.oid = attrelid) @@ -297,7 +297,7 @@ module ActiveRecord JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1]) WHERE t.oid = '#{quote_table_name(table)}'::regclass AND cons.contype = 'p' - AND def.adsrc ~* 'nextval' + AND pg_get_expr(def.adbin, def.adrelid) ~* 'nextval' end_sql end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index a8c02ef18c..0a5b1463c5 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -763,7 +763,8 @@ module ActiveRecord # - ::regclass is a function that gives the id for a table name def column_definitions(table_name) #:nodoc: exec_query(<<-end_sql, 'SCHEMA').rows - SELECT a.attname, format_type(a.atttypid, a.atttypmod), d.adsrc, a.attnotnull, a.atttypid, a.atttypmod + SELECT a.attname, format_type(a.atttypid, a.atttypmod), + pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum WHERE a.attrelid = '#{quote_table_name(table_name)}'::regclass -- cgit v1.2.3 From 8fb841bddefbd2b7ad6b3d7b560111ef0d7fefda Mon Sep 17 00:00:00 2001 From: Arturo Pie Date: Sat, 13 Oct 2012 19:29:23 -0400 Subject: #7914 Remove code for unsupported postgreSQL version. Remove parsing of character type default values for 8.1 formatting since Rails doesn't support postgreSQL 8.1 anymore. Remove misleading comment unrelated to code. --- .../active_record/connection_adapters/postgresql/schema_statements.rb | 3 --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 3 --- 2 files changed, 6 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 0f5431ee37..7cad8f94cf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -280,9 +280,6 @@ module ActiveRecord end_sql if result.nil? or result.empty? - # If that fails, try parsing the primary key's default value. - # Support the 7.x and 8.0 nextval('foo'::text) as well as - # the 8.1+ nextval('foo'::regclass). result = query(<<-end_sql, 'SCHEMA')[0] SELECT attr.attname, CASE diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0a5b1463c5..e18464fa35 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -80,9 +80,6 @@ module ActiveRecord # Character types when /\A\(?'(.*)'::.*\b(?:character varying|bpchar|text)\z/m $1 - # Character types (8.1 formatting) - when /\AE'(.*)'::(?:character varying|bpchar|text)\z/m - $1.gsub(/\\(\d\d\d)/) { $1.oct.chr } # Binary data types when /\A'(.*)'::bytea\z/m $1 -- cgit v1.2.3 From 6f400dabf79faedec258963a6b37b7614f5d4902 Mon Sep 17 00:00:00 2001 From: Miguel Herranz Date: Sun, 14 Oct 2012 15:50:17 +0200 Subject: Fix typo in inet and cidr saving --- .../active_record/connection_adapters/postgresql/quoting.rb | 2 +- activerecord/test/cases/adapters/postgresql/quoting_test.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 37d43d891d..9d3fa18e3a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -90,7 +90,7 @@ module ActiveRecord else super(value, column) end when IPAddr - return super(value, column) unless ['inet','cidr'].includes? column.sql_type + return super(value, column) unless ['inet','cidr'].include? column.sql_type PostgreSQLColumn.cidr_to_string(value) else super(value, column) diff --git a/activerecord/test/cases/adapters/postgresql/quoting_test.rb b/activerecord/test/cases/adapters/postgresql/quoting_test.rb index f8a605b67c..685f0ea74f 100644 --- a/activerecord/test/cases/adapters/postgresql/quoting_test.rb +++ b/activerecord/test/cases/adapters/postgresql/quoting_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'ipaddr' module ActiveRecord module ConnectionAdapters @@ -20,6 +21,18 @@ module ActiveRecord assert_equal 'f', @conn.type_cast(false, c) end + def test_type_cast_cidr + ip = IPAddr.new('255.0.0.0/8') + c = Column.new(nil, ip, 'cidr') + assert_equal ip, @conn.type_cast(ip, c) + end + + def test_type_cast_inet + ip = IPAddr.new('255.1.0.0/8') + c = Column.new(nil, ip, 'inet') + assert_equal ip, @conn.type_cast(ip, c) + end + def test_quote_float_nan nan = 0.0/0 c = Column.new(nil, 1, 'float') -- cgit v1.2.3 From 08927cf1f2ebf7425418f4ee97a3dfb80abe978e Mon Sep 17 00:00:00 2001 From: Angelo Capilleri Date: Sun, 14 Oct 2012 23:01:59 +0200 Subject: refactoring of uniqueness validate_each get scope_value only one time dependig on reflection --- activerecord/lib/active_record/validations/uniqueness.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 5dece1cb36..5fa6a0b892 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -26,11 +26,12 @@ module ActiveRecord relation = relation.and(table[finder_class.primary_key.to_sym].not_eq(record.send(:id))) if record.persisted? Array(options[:scope]).each do |scope_item| - scope_value = record.read_attribute(scope_item) reflection = record.class.reflect_on_association(scope_item) if reflection scope_value = record.send(reflection.foreign_key) scope_item = reflection.foreign_key + else + scope_value = record.read_attribute(scope_item) end relation = relation.and(table[scope_item].eq(scope_value)) end -- cgit v1.2.3