Permalink
Browse files

expliticly make STI column a bind value

  • Loading branch information...
1 parent 2fff094 commit 03118bc5afe99aaaf463a834d8a1ae16c918f6e8 @tenderlove tenderlove committed Jan 15, 2014
Showing with 9 additions and 4 deletions.
  1. +9 −4 activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -61,15 +61,20 @@ def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scop
left.merge right
end
- if reflection.type
- constraint = constraint.and table[reflection.type].eq foreign_klass.base_class.name
- end
-
if rel && !rel.arel.constraints.empty?
bind_values.concat rel.bind_values
constraint = constraint.and rel.arel.constraints
end
+ if reflection.type
+ value = foreign_klass.base_class.name
+ column = klass.columns_hash[column.to_s]
+
+ substitute = klass.connection.substitute_at(column, bind_values.length)
+ bind_values.push [column, value]
+ constraint = constraint.and table[reflection.type].eq substitute
+ end
+
joins << table.create_join(table, table.create_on(constraint), join_type)
# The current table in this iteration becomes the foreign table in the next

6 comments on commit 03118bc

@tenderlove This commit goes way back but still not in 4.1.x. The previous code does not respect sti_name and I was going to issue a PR which would conflict with this. Thoughts?

I have:

constraint = constraint.and table[reflection.type].eq foreign_klass.base_class.sti_name 
Owner

tenderlove replied Aug 21, 2014

@aceofspades ah, makes sense. Just send a PR for master. We can backport to older releases.

@tenderlove I wasn't sure how this applies to your changes in master, does a PR to 4.1.x make sense?

Owner

tenderlove replied Aug 21, 2014

@aceofspades it looks like line 70 in this patch needs to change. But if you want to send a PR to 4.1.x as well, I can forward port it to master. Just make sure it has a test please!

I've created a PR for 4-1-stable here #16624. Thanks!

@tenderlove Can we revive this? The binds in association_scope.rb seem to also neglect sti_name but I'd hope to know whether there's interest in merging before spending time on the fix and tests.

Please sign in to comment.