Faveod/arel-extensions

And/Or visitors break adding conditions to an On node

Opened this issue · 2 comments

The visitor for And/Or nodes is overridden in to_sql:

https://github.com/Faveod/arel-extensions/blob/master/lib/arel_extensions/visitors/to_sql.rb#L561-L579

However, this causes broken sql when adding a condition to an On node:

Arel::Nodes::On.new(Post.arel_table[:user_id].eq(User.arel_table[:id])).to_sql
=> "ON `posts`.`user_id` = `users`.`id`"

Arel::Nodes::On.new(Post.arel_table[:user_id].eq(User.arel_table[:id]))
  .and(Post.arel_table[:type].eq('foo')).to_sql
=> "(ON `posts`.`user_id` = `users`.`id`) AND (`posts`.`type` = 'foo')"

As you can see, the entire original ON got parenthesized, which leads to a SQL syntax error.

I'm using Rails 6.1.4.1.

Hi,
you should write:

Arel::Nodes::On.new(
  Post.arel_table[:user_id].eq(User.arel_table[:id]).and(Post.arel_table[:type].eq('foo'))
).to_sql
=> "ON (`posts`.`user_id` = `users`.`id`) AND (`posts`.`type` = 'foo')"

If the AST is wrongly constructed, I am not surprised the result is wrong. Is this from some ActiveRecord code?

Hi @jdelporte thanks for your response.

I'm using https://github.com/camertron/arel-helpers which allows conditions to be added to join conditions: https://github.com/camertron/arel-helpers#joinassociation-helper

Post.joins(
  Post.join_association(:comments, Arel::Nodes::OuterJoin) do |assoc_name, join_conditions|
    join_conditions.and(Post[:author_id].eq(4))
  end
)

join_conditions that they expose here is an On node, which works fine without arel-extensions but I guess is not the correct thing to be exposing given what you said?