From 724a7866774c7847cc79b699ccf7da9fee72b154 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 21 Apr 2011 22:47:13 -0500 Subject: stop using distinct on for the unique id queries. [#6450 state:resolved] --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 2 +- activerecord/lib/active_record/relation/finder_methods.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index acc3e9c5e3..66ca831d6c 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -935,7 +935,7 @@ module ActiveRecord # Return a DISTINCT ON() clause that's distinct on the columns we want but includes # all the required columns for the ORDER BY to work properly. - sql = "DISTINCT ON (#{columns}) #{columns}, " + sql = "DISTINCT #{columns}, " sql << order_columns * ', ' end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index aae257a0e7..a3d4b7f45a 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -246,6 +246,8 @@ module ActiveRecord orders = relation.order_values values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders) + relation = relation.dup + ids_array = relation.select(values).collect {|row| row[primary_key]} ids_array.empty? ? raise(ThrowResult) : table[primary_key].in(ids_array) end -- cgit v1.2.3 From ed2820d6ecef7533970952437b974c7fa10192e1 Mon Sep 17 00:00:00 2001 From: Ken Collins Date: Fri, 22 Apr 2011 20:33:43 +0800 Subject: Move #exec_insert to abstract adapter's database statements. --- .../connection_adapters/abstract/database_statements.rb | 7 +++++++ .../lib/active_record/connection_adapters/mysql_adapter.rb | 4 ---- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 4 ---- .../lib/active_record/connection_adapters/sqlite_adapter.rb | 4 ---- 4 files changed, 7 insertions(+), 12 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 6d9b5c7b32..5ff81aa023 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -55,6 +55,13 @@ module ActiveRecord def exec_query(sql, name = 'SQL', binds = []) end + # Executes insert +sql+ statement in the context of this connection using + # +binds+ as the bind substitutes. +name+ is the logged along with + # the executed +sql+ statement. + def exec_insert(sql, name, binds) + exec_query(sql, name, binds) + end + # Returns the last auto-generated ID from the affected table. # # +id_value+ will be returned unless the value is nil, in diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index c2e75acb9a..0f45565dc9 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -427,10 +427,6 @@ module ActiveRecord end end - def exec_insert(sql, name, binds) - exec_query(sql, name, binds) - end - def last_inserted_id(result) @connection.insert_id end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 66ca831d6c..5adb235835 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -552,10 +552,6 @@ module ActiveRecord end end - def exec_insert(sql, name, binds) - exec_query(sql, name, binds) - end - def sql_for_insert(sql, pk, id_value, sequence_name, binds) unless pk _, table = extract_schema_and_table(sql.split(" ", 4)[2]) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 9e7f874f4b..8bff20fa70 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -173,10 +173,6 @@ module ActiveRecord end end - def exec_insert(sql, name, binds) - exec_query(sql, name, binds) - end - def last_inserted_id(result) @connection.last_insert_row_id end -- cgit v1.2.3 From bc50c1cb02c317ef92e4aa0336d396dcad5bc578 Mon Sep 17 00:00:00 2001 From: Ken Collins Date: Fri, 22 Apr 2011 19:55:42 +0800 Subject: The #substitute_at gets an ActiveRecord::ConnectionAdapters::Column in #insert to match replacement in #exec_query. --- activerecord/lib/active_record/relation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 359f9d8a66..45a7000cce 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -56,7 +56,7 @@ module ActiveRecord end substitutes.each_with_index do |tuple, i| - tuple[1] = conn.substitute_at(tuple.first, i) + tuple[1] = conn.substitute_at(binds[i][0], i) end if values.empty? # empty insert -- cgit v1.2.3 From a37722182fd96190f2317ffd0450c49ed6d406f0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 22 Apr 2011 12:21:59 -0500 Subject: removing incorrect comment and string concatenation --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 5adb235835..203ecd2f03 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -929,10 +929,7 @@ module ActiveRecord order_columns.delete_if { |c| c.blank? } order_columns = order_columns.zip((0...order_columns.size).to_a).map { |s,i| "#{s} AS alias_#{i}" } - # Return a DISTINCT ON() clause that's distinct on the columns we want but includes - # all the required columns for the ORDER BY to work properly. - sql = "DISTINCT #{columns}, " - sql << order_columns * ', ' + "DISTINCT #{columns}, #{order_columns * ', '}" end protected -- cgit v1.2.3 From 3eae734012fadd641a4ec95715480b962861294c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 22 Apr 2011 14:37:53 -0500 Subject: set the backtrace to prevent AR exceptions from lying to us --- .../lib/active_record/connection_adapters/abstract_adapter.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index d24cce0a3c..468a2b106b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -223,7 +223,9 @@ module ActiveRecord rescue Exception => e message = "#{e.class.name}: #{e.message}: #{sql}" @logger.debug message if @logger - raise translate_exception(e, message) + exception = translate_exception(e, message) + exception.set_backtrace e.backtrace + raise exception end def translate_exception(e, message) -- cgit v1.2.3 From 3d19b356d7ccb37b23afcab0b0b698bfd3d668d7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 22 Apr 2011 15:31:03 -0500 Subject: fetch result row arrays from pg in C and return early if there are no money or binary columns. :heart: --- .../connection_adapters/postgresql_adapter.rb | 67 ++++++++++++---------- 1 file changed, 36 insertions(+), 31 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 203ecd2f03..8763074121 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1,6 +1,9 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_support/core_ext/kernel/requires' require 'active_support/core_ext/object/blank' + +# Make sure we're using pg high enough for PGResult#values +gem 'pg', '~> 0.11' require 'pg' module ActiveRecord @@ -460,42 +463,44 @@ module ActiveRecord # create a 2D array representing the result set def result_as_array(res) #:nodoc: # check if we have any binary column and if they need escaping - unescape_col = [] - res.nfields.times do |j| - unescape_col << res.ftype(j) + ftypes = Array.new(res.nfields) do |i| + [i, res.ftype(i)] end - ary = [] - res.ntuples.times do |i| - ary << [] - res.nfields.times do |j| - data = res.getvalue(i,j) - case unescape_col[j] - - # unescape string passed BYTEA field (OID == 17) - when BYTEA_COLUMN_TYPE_OID - data = unescape_bytea(data) if String === data - - # If this is a money type column and there are any currency symbols, - # then strip them off. Indeed it would be prettier to do this in - # PostgreSQLColumn.string_to_decimal but would break form input - # fields that call value_before_type_cast. - when MONEY_COLUMN_TYPE_OID - # Because money output is formatted according to the locale, there are two - # cases to consider (note the decimal separators): - # (1) $12,345,678.12 - # (2) $12.345.678,12 - case data - when /^-?\D+[\d,]+\.\d{2}$/ # (1) - data.gsub!(/[^-\d.]/, '') - when /^-?\D+[\d.]+,\d{2}$/ # (2) - data.gsub!(/[^-\d,]/, '').sub!(/,/, '.') - end + rows = res.values + return rows unless ftypes.any? { |_, x| + x == BYTEA_COLUMN_TYPE_OID || x == MONEY_COLUMN_TYPE_OID + } + + typehash = ftypes.group_by { |_, type| type } + binaries = (typehash[BYTEA_COLUMN_TYPE_OID] || []).map { |x| x.first } + monies = (typehash[MONEY_COLUMN_TYPE_OID] || []).map { |x| x.first } + + rows.each do |row| + # unescape string passed BYTEA field (OID == 17) + binaries.each do |index| + data = row[index] + row[index] = unescape_bytea(data) + end + + # If this is a money type column and there are any currency symbols, + # then strip them off. Indeed it would be prettier to do this in + # PostgreSQLColumn.string_to_decimal but would break form input + # fields that call value_before_type_cast. + monies.each do |index| + data = row[index] + # Because money output is formatted according to the locale, there are two + # cases to consider (note the decimal separators): + # (1) $12,345,678.12 + # (2) $12.345.678,12 + case data + when /^-?\D+[\d,]+\.\d{2}$/ # (1) + data.gsub!(/[^-\d.]/, '') + when /^-?\D+[\d.]+,\d{2}$/ # (2) + data.gsub!(/[^-\d,]/, '').sub!(/,/, '.') end - ary[i] << data end end - return ary end -- cgit v1.2.3 From 5bed6494d49506223dc3b6652858c21487776a02 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 22 Apr 2011 15:35:54 -0500 Subject: split indexes and column types on money / binary iteration --- .../active_record/connection_adapters/postgresql_adapter.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'activerecord/lib/active_record') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 8763074121..fbfccd4ec1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -473,21 +473,20 @@ module ActiveRecord } typehash = ftypes.group_by { |_, type| type } - binaries = (typehash[BYTEA_COLUMN_TYPE_OID] || []).map { |x| x.first } - monies = (typehash[MONEY_COLUMN_TYPE_OID] || []).map { |x| x.first } + binaries = typehash[BYTEA_COLUMN_TYPE_OID] || [] + monies = typehash[MONEY_COLUMN_TYPE_OID] || [] rows.each do |row| # unescape string passed BYTEA field (OID == 17) - binaries.each do |index| - data = row[index] - row[index] = unescape_bytea(data) + binaries.each do |index, _| + row[index] = unescape_bytea(row[index]) end # If this is a money type column and there are any currency symbols, # then strip them off. Indeed it would be prettier to do this in # PostgreSQLColumn.string_to_decimal but would break form input # fields that call value_before_type_cast. - monies.each do |index| + monies.each do |index, _| data = row[index] # Because money output is formatted according to the locale, there are two # cases to consider (note the decimal separators): -- cgit v1.2.3