Skip to content
Browse files

Oracle: fix limited id selection for eager loading. Closes #6515.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5480 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
1 parent 63df6eb commit c0bce43e903fe42645c16a8062ce1cd970217f40 @jeremy jeremy committed
View
2 activerecord/CHANGELOG
@@ -27,7 +27,7 @@
* has_one :dependent => :nullify ignores nil associates. #6528 [janovetz, Jeremy Kemper]
-* Oracle: resolve test failures, use prefetched primary key for inserts, check for null defaults. Factor out some common methods from all adapters. #6515 [Michael Schoen]
+* Oracle: resolve test failures, use prefetched primary key for inserts, check for null defaults, fix limited id selection for eager loading. Factor out some common methods from all adapters. #6515 [Michael Schoen]
* Make add_column use the options hash with the Sqlite Adapter. Closes #6464 [obrie]
View
10 activerecord/lib/active_record/associations.rb
@@ -1190,25 +1190,23 @@ def select_limited_ids_list(options, join_dependency)
"#{name} Load IDs For Limited Eager Loading"
).collect { |row| connection.quote(row[primary_key]) }.join(", ")
end
-
+
def construct_finder_sql_for_association_limiting(options, join_dependency)
scope = scope(:find)
is_distinct = include_eager_conditions?(options) || include_eager_order?(options)
sql = "SELECT "
if is_distinct
- ordered_columns = options[:order].to_s.split(',').collect! { |s| s.split.first }
- options[:order] = "#{table_name}.#{primary_key}, #{options[:order]}" if options[:order] && connection.requires_order_columns_in_distinct_clause?
- sql << connection.distinct("#{table_name}.#{primary_key}", ordered_columns)
+ sql << connection.distinct("#{table_name}.#{primary_key}", options[:order])
else
sql << primary_key
end
sql << " FROM #{table_name} "
-
+
if is_distinct
sql << join_dependency.join_associations.collect(&:association_join).join
add_joins!(sql, options, scope)
end
-
+
add_conditions!(sql, options[:conditions], scope)
sql << "ORDER BY #{options[:order]} " if options[:order]
add_limit!(sql, options, scope)
View
7 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -280,8 +280,11 @@ def add_column_options!(sql, options) #:nodoc:
sql << " NOT NULL" if options[:null] == false
end
- # SELECT DISTINCT clause for a given set of columns. PostgreSQL overrides this for custom DISTINCT syntax.
- def distinct(columns, *order_columns)
+ # SELECT DISTINCT clause for a given set of columns and a given ORDER BY clause.
+ # Both PostgreSQL and Oracle overrides this for custom DISTINCT syntax.
+ #
+ # distinct("posts.id", "posts.created_at desc")
+ def distinct(columns, order_by)
"DISTINCT #{columns}"
end
end
View
6 activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -56,12 +56,6 @@ def prefetch_primary_key?(table_name = nil)
false
end
- # Does this adapter require the order columns to be in the select clause
- # of a DISTINCT query? This is +false+ in all adapters except postgresql.
- def requires_order_columns_in_distinct_clause?
- false
- end
-
def reset_runtime #:nodoc:
rt, @runtime = @runtime, 0
rt
View
30 activerecord/lib/active_record/connection_adapters/oracle_adapter.rb
@@ -314,7 +314,7 @@ def indexes(table_name, name = nil) #:nodoc:
def columns(table_name, name = nil) #:nodoc:
(owner, table_name) = @connection.describe(table_name)
- raise "Couldn't describe #{table_name}. Does it exist?" unless owner and table_name
+ raise "Could not describe #{table_name}. Does it exist?" unless owner and table_name
table_cols = %Q{
select column_name as name, data_type as sql_type, data_default, nullable,
@@ -427,6 +427,34 @@ def structure_drop #:nodoc:
end
end
+ # SELECT DISTINCT clause for a given set of columns and a given ORDER BY clause.
+ #
+ # Oracle requires the ORDER BY columns to be in the SELECT list for DISTINCT
+ # queries. However, with those columns included in the SELECT DISTINCT list, you
+ # won't actually get a distinct list of the column you want (presuming the column
+ # has duplicates with multiple values for the ordered-by columns. So we use the
+ # FIRST_VALUE function to get a single (first) value for each column, effectively
+ # making every row the same.
+ #
+ # distinct("posts.id", "posts.created_at desc")
+ def distinct(columns, order_by)
+ return "DISTINCT #{columns}" if order_by.blank?
+
+ # construct a clean list of column names from the ORDER BY clause, removing
+ # any asc/desc modifiers
+ order_columns = order_by.split(',').collect! { |s| s.split.first }
+ order_columns.delete_if &:blank?
+
+ # simplify the ORDER BY to just use positional syntax, to avoid the complexity of
+ # having to create valid column aliases for the FIRST_VALUE columns
+ order_by.replace(((offset=columns.count(',')+2) .. offset+order_by.count(',')).to_a * ", ")
+
+ # construct a valid DISTINCT clause, ie. one that includes the ORDER BY columns, using
+ # FIRST_VALUE such that the inclusion of these columns doesn't invalidate the DISTINCT
+ order_columns.map! { |c| "FIRST_VALUE(#{c}) OVER (PARTITION BY #{columns} ORDER BY #{c})" }
+ sql = "DISTINCT #{columns}, "
+ sql << order_columns * ", "
+ end
private
View
29 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -111,10 +111,6 @@ def table_alias_length
63
end
- def requires_order_columns_in_distinct_clause?
- true
- end
-
# QUOTING ==================================================
def quote(value, column = nil)
@@ -376,14 +372,27 @@ def type_to_sql(type, limit = nil, precision = nil, scale = nil) #:nodoc:
end
end
- # PostgreSQL requires the ORDER BY columns in the select list for distinct queries.
- # If you select distinct by a column though, you must pass that column in the order by clause too:
+ # SELECT DISTINCT clause for a given set of columns and a given ORDER BY clause.
+ #
+ # PostgreSQL requires the ORDER BY columns in the select list for distinct queries, and
+ # requires that the ORDER BY include the distinct column.
#
- # distinct("posts.id", 'posts.id', 'posts.created_at')
- def distinct(columns, *order_columns)
+ # distinct("posts.id", "posts.created_at desc")
+ def distinct(columns, order_by)
+ return "DISTINCT #{columns}" if order_by.blank?
+
+ # construct a clean list of column names from the ORDER BY clause, removing
+ # any asc/desc modifiers
+ order_columns = order_by.split(',').collect! { |s| s.split.first }
order_columns.delete_if &:blank?
- sql = "DISTINCT ON (#{columns}) #{columns}"
- sql << (order_columns.any? ? ", #{order_columns * ', '}" : '')
+
+ # add the DISTINCT columns to the start of the ORDER BY clause
+ order_by.replace "#{columns}, #{order_by}"
+
+ # 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 << order_columns * ', '
end
private

0 comments on commit c0bce43

Please sign in to comment.
Something went wrong with that request. Please try again.