Permalink
Browse files

Make visitors threadsafe by removing @last_column

The last_column feature of the ToSql visitor and its descendants is what
enabled quoting based on the column last visited -- in other words, if
you have a standard condition like an equality with a string attribute
on the left side and an integer on the right side, then when ARel visits
the node, it'll first visit the left side attribute, setting the
column of the string attribute as the last column, and resulting in the
right side of the condition getting the appropriate quoting.

The downside is that this means that visitors can't be shared between
threads, because of the state mutation. It also makes for some really
weird behavior in the event that the visitor visits a node that happens
to contain an attribute you weren't expecting to be there, since it'll
potentially quote something based on that attribute. So, it prevents
reversing an equality condition. column = value will work, but not value
= column, since the last column wouldn't be the column you're hoping
for.

This is a first pass at fixing this by changing the signature of the
visit methods to accept the currently-relevant attribute, if any.
  • Loading branch information...
1 parent 3ef63ac commit 68a95542e1a7a79d9777223fbffd2b982fed0268 @ernie ernie committed Mar 16, 2013
@@ -13,22 +13,22 @@ def accept node, &block
private
- def visit_Arel_Nodes_Assignment o
+ def visit_Arel_Nodes_Assignment o, a
if o.right.is_a? Arel::Nodes::BindParam
- "#{visit o.left} = #{visit o.right}"
+ "#{visit o.left, a} = #{visit o.right, a}"
else
super
end
end
- def visit_Arel_Nodes_BindParam o
+ def visit_Arel_Nodes_BindParam o, a
if @block
@block.call
else
super
end
end
-
+
end
end
end
@@ -7,13 +7,13 @@ def initialize block = nil
private
- def visit o
+ def visit o, a = nil
super
@block.call o
end
- def unary o
- visit o.expr
+ def unary o, a
+ visit o.expr, a
end
alias :visit_Arel_Nodes_Group :unary
alias :visit_Arel_Nodes_Grouping :unary
@@ -28,38 +28,38 @@ def unary o
alias :visit_Arel_Nodes_Top :unary
alias :visit_Arel_Nodes_UnqualifiedColumn :unary
- def function o
- visit o.expressions
- visit o.alias
- visit o.distinct
+ def function o, a
+ visit o.expressions, a
+ visit o.alias, a
+ visit o.distinct, a
end
alias :visit_Arel_Nodes_Avg :function
alias :visit_Arel_Nodes_Exists :function
alias :visit_Arel_Nodes_Max :function
alias :visit_Arel_Nodes_Min :function
alias :visit_Arel_Nodes_Sum :function
- def visit_Arel_Nodes_NamedFunction o
- visit o.name
- visit o.expressions
- visit o.distinct
- visit o.alias
+ def visit_Arel_Nodes_NamedFunction o, a
+ visit o.name, a
+ visit o.expressions, a
+ visit o.distinct, a
+ visit o.alias, a
end
- def visit_Arel_Nodes_Count o
- visit o.expressions
- visit o.alias
- visit o.distinct
+ def visit_Arel_Nodes_Count o, a
+ visit o.expressions, a
+ visit o.alias, a
+ visit o.distinct, a
end
- def nary o
- o.children.each { |child| visit child }
+ def nary o, a
+ o.children.each { |child| visit child, a }
end
alias :visit_Arel_Nodes_And :nary
- def binary o
- visit o.left
- visit o.right
+ def binary o, a
+ visit o.left, a
+ visit o.right, a
end
alias :visit_Arel_Nodes_As :binary
alias :visit_Arel_Nodes_Assignment :binary
@@ -83,13 +83,13 @@ def binary o
alias :visit_Arel_Nodes_TableAlias :binary
alias :visit_Arel_Nodes_Values :binary
- def visit_Arel_Nodes_StringJoin o
- visit o.left
+ def visit_Arel_Nodes_StringJoin o, a
+ visit o.left, a
end
- def visit_Arel_Attribute o
- visit o.relation
- visit o.name
+ def visit_Arel_Attribute o, a
+ visit o.relation, a
+ visit o.name, a
end
alias :visit_Arel_Attributes_Integer :visit_Arel_Attribute
alias :visit_Arel_Attributes_Float :visit_Arel_Attribute
@@ -99,11 +99,11 @@ def visit_Arel_Attribute o
alias :visit_Arel_Attributes_Attribute :visit_Arel_Attribute
alias :visit_Arel_Attributes_Decimal :visit_Arel_Attribute
- def visit_Arel_Table o
- visit o.name
+ def visit_Arel_Table o, a
+ visit o.name, a
end
- def terminal o
+ def terminal o, a
end
alias :visit_ActiveSupport_Multibyte_Chars :terminal
alias :visit_ActiveSupport_StringInquirer :terminal
@@ -127,43 +127,43 @@ def terminal o
alias :visit_Time :terminal
alias :visit_TrueClass :terminal
- def visit_Arel_Nodes_InsertStatement o
- visit o.relation
- visit o.columns
- visit o.values
+ def visit_Arel_Nodes_InsertStatement o, a
+ visit o.relation, a
+ visit o.columns, a
+ visit o.values, a
end
- def visit_Arel_Nodes_SelectCore o
- visit o.projections
- visit o.source
- visit o.wheres
- visit o.groups
- visit o.windows
- visit o.having
+ def visit_Arel_Nodes_SelectCore o, a
+ visit o.projections, a
+ visit o.source, a
+ visit o.wheres, a
+ visit o.groups, a
+ visit o.windows, a
+ visit o.having, a
end
- def visit_Arel_Nodes_SelectStatement o
- visit o.cores
- visit o.orders
- visit o.limit
- visit o.lock
- visit o.offset
+ def visit_Arel_Nodes_SelectStatement o, a
+ visit o.cores, a
+ visit o.orders, a
+ visit o.limit, a
+ visit o.lock, a
+ visit o.offset, a
end
- def visit_Arel_Nodes_UpdateStatement o
- visit o.relation
- visit o.values
- visit o.wheres
- visit o.orders
- visit o.limit
+ def visit_Arel_Nodes_UpdateStatement o, a
+ visit o.relation, a
+ visit o.values, a
+ visit o.wheres, a
+ visit o.orders, a
+ visit o.limit, a
end
- def visit_Array o
- o.each { |i| visit i }
+ def visit_Array o, a
+ o.each { |i| visit i, a }
end
- def visit_Hash o
- o.each { |k,v| visit(k); visit(v) }
+ def visit_Hash o, a
+ o.each { |k,v| visit(k, a); visit(v, a) }
end
end
end
Oops, something went wrong.

1 comment on commit 68a9554

@elia
elia commented on 68a9554 Jul 30, 2015

This fixes #153 cc @gama

Please sign in to comment.