Permalink
Browse files

BREAKING CHANGE: Removed autojoin.

Improved merge (no join necessary).
Added tests for merge with alternate association name.
Refactored build_arel for readability.
  • Loading branch information...
1 parent 82598e1 commit e5a6659dc73766ffbed6817060addd6c874179f2 @ernie ernie committed Jul 29, 2010
Showing with 107 additions and 133 deletions.
  1. +13 −0 CHANGELOG
  2. +18 −22 README.rdoc
  3. +13 −14 lib/meta_where/builder.rb
  4. +0 −20 lib/meta_where/join_dependency.rb
  5. +42 −45 lib/meta_where/relation.rb
  6. +21 −32 test/test_relations.rb
View
@@ -1,3 +1,16 @@
+Changes since 0.5.2 (2010-07-09):
+ * Removed autojoin. Inner joins have side-effects. Newbies are the ones who aremost
+ likely to use autojoin, and they're also the ones least likely to understand why
+ certain rows stop being returned. Better to force a small learning curve. I've
+ decided against leaving it in with deprecation since Rails 3 isn't final yet.
+ Decided to get it out before it's "too late" to do so without impacting code running
+ on a stable version of Rails.
+ * Refactored build_arel to more closely mirror the refactoring that's been going on
+ to the method in Rails edge.
+ * Improved merge functonality. It shouldn't have ever required someone to do an
+ autojoin to begin with. Now it just works. If you're merging two relations, you
+ should expect to only get results that have a match on both sides.
+
Changes since 0.5.1 (2010-06-22):
* Added debug_sql method to Relations. Lets you see the actual SQL that
will be run against the database without having to resort to the
View
@@ -150,18 +150,6 @@ parentheses around everything. So MetaWhere also supports a substitution-inspire
=== But wait, there's more!
-== Autojoin
-Normally, you have to be sure to join (or include, which will join if conditions warrant)
-any associations that you're including in your wheres. With MetaWhere, you can just build
-up your relation's conditions, and tack an <tt>autojoin</tt> anywhere in the chain. MetaWhere
-will check out the associations you're using in your conditions and join them automatically
-(if they aren't already joined).
-
- Article.where(:comments => [:body.like % '%FIRST POST%']).autojoin
-
-Remember that joins will return duplicate rows if your conditions don't prevent it, so
-you might want to tack on a <tt>uniq</tt> as well.
-
== Intelligent hash condition mapping
This is one of those things I hope you find so intuitive that you forget it wasn't
built in already.
@@ -194,18 +182,20 @@ With MetaWhere:
=> SELECT "articles".* FROM "articles" INNER JOIN "comments"
ON "comments"."article_id" = "articles"."id" WHERE (("comments"."body" = 'hey'))
-Of course, it's even simpler with <tt>autojoin</tt>, but the general idea is that if an
-association with the name provided exists, MetaWhere::Builder will build the
-conditions against that association's table, before falling back to a standard table name
-scheme. It also handles nested associations:
+The general idea is that if an association with the name provided exists, MetaWhere::Builder
+will build the conditions against that association's table as it's been aliased, before falling
+back to assuming you're specifying a table by name. It also handles nested associations:
Article.where(
:comments => {
:body => 'yo',
:moderations => [:value < 0]
},
:other_comments => {:body => 'hey'}
- ).autojoin.to_sql
+ ).joins(
+ {:comments => :moderations},
+ :other_comments
+ ).to_sql
=> SELECT "articles".* FROM "articles"
INNER JOIN "comments" ON "comments"."article_id" = "articles"."id"
INNER JOIN "moderations" ON "moderations"."comment_id" = "comments"."id"
@@ -234,13 +224,21 @@ Additionally, to cover times when detection is impossible, or the first detected
association isn't the one you wanted, you can call merge with a second parameter,
specifying the association to be used during the merge.
-This merge functionality allows you to do:
+This merge functionality allows you to do this...
- (Comment.where(:id < 7) & Article.where(:title =~ '%blah%')).autojoin.to_sql
+ (Comment.where(:id < 7) & Article.where(:title =~ '%blah%')).to_sql
=> SELECT "comments".* FROM "comments" INNER JOIN "articles"
ON "articles"."id" = "comments"."article_id"
WHERE ("comments"."id" < 7) AND ("articles"."title" LIKE '%blah%')"
+...or this...
+
+ Article.where(:id < 2).merge(Comment.where(:id < 7), :lame_comments).to_sql
+ => "SELECT "articles".* FROM "articles"
+ INNER JOIN "comments" ON "comments"."article_id" = "articles"."id"
+ AND "comments"."body" = 'first post!'
+ WHERE ("articles"."id" < 2) AND ("comments"."id" < 7)"
+
== Enhanced order clauses
If you are used to doing stuff like <tt>Article.order('title asc')</tt>, that will still
@@ -251,14 +249,12 @@ mapping," above) and also some convenience methods for ascending and descending
Article.order(
:title.desc,
:comments => [:created_at.asc, :updated_at]
- ).autojoin.to_sql
+ ).joins(:comments).to_sql
=> SELECT "articles".* FROM "articles"
INNER JOIN "comments" ON "comments"."article_id" = "articles"."id"
ORDER BY "articles"."title" DESC,
"comments"."created_at" ASC, "comments"."updated_at"
-As you can see, autojoin will work with these associations as well.
-
== Thanks
A huge thank you goes to Pratik Naik (lifo) for a dicussion on #rails-contrib about a patch
I'd submitted, and his take on a DSL for query conditions, which was the inspiration for this
View
@@ -4,14 +4,13 @@ module MetaWhere
class Builder
include MetaWhere::Utility
attr_reader :join_dependency
-
- def initialize(join_dependency, autojoin = false)
+
+ def initialize(join_dependency)
@join_dependency = join_dependency
@engine = join_dependency.join_base.arel_engine
@default_table = Arel::Table.new(join_dependency.join_base.table_name, :engine => @engine)
- @association_finder = autojoin ? :find_or_build_join_association : :find_join_association
end
-
+
def build_table(parent_or_table_name = nil)
if parent_or_table_name.is_a?(Symbol)
Arel::Table.new(parent_or_table_name, :engine => @engine)
@@ -21,18 +20,18 @@ def build_table(parent_or_table_name = nil)
@default_table
end
end
-
+
def build_predicates_from_hash(attributes, parent = nil)
table = build_table(parent)
predicates = attributes.map do |column, value|
if value.is_a?(Hash)
- association = parent.is_a?(Symbol) ? nil : @join_dependency.send(@association_finder, column, parent)
+ association = parent.is_a?(Symbol) ? nil : @join_dependency.find_join_association(column, parent)
build_predicates_from_hash(value, association || column)
elsif value.is_a?(MetaWhere::Condition)
- association = parent.is_a?(Symbol) ? nil : @join_dependency.send(@association_finder, column, parent)
+ association = parent.is_a?(Symbol) ? nil : @join_dependency.find_join_association(column, parent)
value.to_predicate(self, association || column)
elsif value.is_a?(Array) && value.all? {|v| v.respond_to?(:to_predicate)}
- association = parent.is_a?(Symbol) ? nil : @join_dependency.send(@association_finder, column, parent)
+ association = parent.is_a?(Symbol) ? nil : @join_dependency.find_join_association(column, parent)
value.map {|val| val.to_predicate(self, association || column)}
else
if column.is_a?(MetaWhere::Column)
@@ -51,7 +50,7 @@ def build_predicates_from_hash(attributes, parent = nil)
unless attribute = table[column]
raise ::ActiveRecord::StatementInvalid, "No attribute named `#{column}` exists for table `#{table.name}`"
end
-
+
unless valid_comparison_method?(method)
raise ::ActiveRecord::StatementInvalid, "No comparison method named `#{method}` exists for column `#{column}`"
end
@@ -62,24 +61,24 @@ def build_predicates_from_hash(attributes, parent = nil)
predicates.flatten
end
-
+
def build_attributes_from_hash(attributes, parent = nil)
table = build_table(parent)
built_attributes = attributes.map do |column, value|
if value.is_a?(Hash)
- association = parent.is_a?(Symbol) ? nil : @join_dependency.send(@association_finder, column, parent)
+ association = parent.is_a?(Symbol) ? nil : @join_dependency.find_join_association(column, parent)
build_attributes_from_hash(value, association || column)
elsif value.is_a?(Array) && value.all? {|v| v.respond_to?(:to_attribute)}
- association = parent.is_a?(Symbol) ? nil : @join_dependency.send(@association_finder, column, parent)
+ association = parent.is_a?(Symbol) ? nil : @join_dependency.find_join_association(column, parent)
value.map {|val| val.to_attribute(self, association || column)}
else
- association = parent.is_a?(Symbol) ? nil : @join_dependency.send(@association_finder, column, parent)
+ association = parent.is_a?(Symbol) ? nil : @join_dependency.find_join_association(column, parent)
value.respond_to?(:to_attribute) ? value.to_attribute(self, association || column) : value
end
end
built_attributes.flatten
end
-
+
end
end
@@ -35,18 +35,6 @@ def build_with_metawhere(associations, parent = nil, join_class = Arel::InnerJoi
end
end
- def find_or_build_join_association(name, parent)
- unless parent.respond_to?(:reflections)
- raise ArgumentError, "Parent ('#{parent.class}') is not reflectable (must be JoinBase or JoinAssociation)"
- end
-
- raise ArgumentError, "#{name} is not a Symbol" unless name.is_a?(Symbol)
-
- build(name, parent)
- rescue AssociationNotFoundError
- nil
- end
-
def find_join_association(name_or_reflection, parent)
case name_or_reflection
when Symbol, String
@@ -55,13 +43,5 @@ def find_join_association(name_or_reflection, parent)
join_associations.detect {|j| (j.reflection == name_or_reflection) && (j.parent == parent)}
end
end
-
- def merge(other_join_dependency)
- if self.join_base == other_join_dependency.join_base
- self.graft(*other_join_dependency.join_associations)
- else
- raise BaseMismatchError, "Can't merge a join dependency with a different join base."
- end
- end
end
end
View
@@ -10,16 +10,6 @@ module Relation
alias_method_chain :merge, :metawhere
alias_method :&, :merge_with_metawhere
-
- const_get("SINGLE_VALUE_METHODS").push(:autojoin) # I'm evil.
- attr_accessor :autojoin_value
- end
-
- def autojoin(value = true, &block)
- new_relation = clone
- new_relation.send(:apply_modules, Module.new(&block)) if block_given?
- new_relation.autojoin_value = value
- new_relation
end
def merge_with_metawhere(r, association_name = nil)
@@ -31,7 +21,7 @@ def merge_with_metawhere(r, association_name = nil)
r.joins_values.map! {|j| [Symbol, Hash].include?(j.class) ? {association_name => j} : j}
end
- merge_without_metawhere(r)
+ joins(association_name).merge_without_metawhere(r)
end
def reset_with_metawhere
@@ -120,7 +110,7 @@ def custom_join_sql(*joins)
def predicate_wheres
join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, association_joins, custom_joins)
- builder = MetaWhere::Builder.new(join_dependency, @autojoin_value)
+ builder = MetaWhere::Builder.new(join_dependency)
remove_conflicting_equality_predicates(flatten_predicates(@where_values, builder))
end
@@ -143,42 +133,13 @@ def debug_sql
def build_arel_with_metawhere
arel = table
- joined_associations = []
-
join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, association_joins, custom_joins)
- # Build wheres now to take advantage of autojoin if needed
- builder = MetaWhere::Builder.new(join_dependency, @autojoin_value)
- predicate_wheres = remove_conflicting_equality_predicates(flatten_predicates(@where_values, builder))
-
- order_attributes = @order_values.map {|o|
- o.respond_to?(:to_attribute) ? o.to_attribute(builder, join_dependency.join_base) : o
- }.flatten.uniq.select {|o| o.present?}
- order_attributes.map! {|a| Arel::SqlLiteral.new(a.is_a?(String) ? a : a.to_sql)}
-
- join_dependency.graft(*stashed_association_joins)
-
- @implicit_readonly = true unless association_joins.empty? && stashed_association_joins.empty?
-
- to_join = []
-
- join_dependency.join_associations.each do |association|
- if (association_relation = association.relation).is_a?(Array)
- to_join << [association_relation.first, association.join_class, association.association_join.first]
- to_join << [association_relation.last, association.join_class, association.association_join.last]
- else
- to_join << [association_relation, association.join_class, association.association_join]
- end
- end
+ builder = MetaWhere::Builder.new(join_dependency)
- to_join.each do |tj|
- unless joined_associations.detect {|ja| ja[0] == tj[0] && ja[1] == tj[1] && ja[2] == tj[2] }
- joined_associations << tj
- arel = arel.join(tj[0], tj[1]).on(*tj[2])
- end
- end
+ arel = build_intelligent_joins(arel, builder) if @joins_values.present?
- arel = arel.join(custom_joins)
+ predicate_wheres = remove_conflicting_equality_predicates(flatten_predicates(@where_values, builder))
predicate_wheres.each do |where|
next if where.blank?
@@ -199,7 +160,7 @@ def build_arel_with_metawhere
arel = arel.group(*@group_values.uniq.select{|g| g.present?}) if @group_values.present?
- arel = arel.order(*order_attributes) if order_attributes.present?
+ arel = build_order(arel, builder, @order_values) if @order_values.present?
arel = build_select(arel, @select_values.uniq)
@@ -217,6 +178,42 @@ def build_arel_with_metawhere
private
+ def build_intelligent_joins(arel, builder)
+ joined_associations = []
+
+ builder.join_dependency.graft(*stashed_association_joins)
+
+ @implicit_readonly = true unless association_joins.empty? && stashed_association_joins.empty?
+
+ to_join = []
+
+ builder.join_dependency.join_associations.each do |association|
+ if (association_relation = association.relation).is_a?(Array)
+ to_join << [association_relation.first, association.join_class, association.association_join.first]
+ to_join << [association_relation.last, association.join_class, association.association_join.last]
+ else
+ to_join << [association_relation, association.join_class, association.association_join]
+ end
+ end
+
+ to_join.each do |tj|
+ unless joined_associations.detect {|ja| ja[0] == tj[0] && ja[1] == tj[1] && ja[2] == tj[2] }
+ joined_associations << tj
+ arel = arel.join(tj[0], tj[1]).on(*tj[2])
+ end
+ end
+
+ arel = arel.join(custom_joins)
+ end
+
+ def build_order(arel, builder, orders)
+ order_attributes = orders.map {|o|
+ o.respond_to?(:to_attribute) ? o.to_attribute(builder, builder.join_dependency.join_base) : o
+ }.flatten.uniq.select {|o| o.present?}
+ order_attributes.map! {|a| Arel::SqlLiteral.new(a.is_a?(String) ? a : a.to_sql)}
+ order_attributes.present? ? arel.order(*order_attributes) : arel
+ end
+
def remove_conflicting_equality_predicates(predicates)
predicates.reverse.inject([]) { |ary, w|
unless w.is_a?(Arel::Predicates::Equality) && ary.any? {|p| p.is_a?(Arel::Predicates::Equality) && p.operand1.name == w.operand1.name}
Oops, something went wrong.

0 comments on commit e5a6659

Please sign in to comment.