camertron / scuttle-rb

A library for transforming raw SQL statements into ActiveRecord/Arel queries. Ruby wrapper and tests for scuttle-java.
86 stars 2 forks source link

Wrong code produced, syntax error, too #4

Open camertron opened 8 years ago

camertron commented 8 years ago

(Originally filed by @martinstreicher against arel-helpers)

My solution for this SQL:

SELECT weights.* 
FROM weights 
WHERE captured_at IN (
SELECT MIN(captured_at) 
FROM weights 
GROUP BY DATE(captured_at))

Was (in my code)…

class Weight < ActiveRecord::Base
  def self.by_created_at_asc
    order created_at: :asc
  end

  def self.by_captured_at_asc
    order captured_at: :asc
  end

  def self.earliest_captured_at
    table
      .project(table[:captured_at].minimum)
      .group(Arel::Nodes::NamedFunction.new('DATE', [table[:captured_at]]))
  end

  def self.first_of_each_day
    where(table[:captured_at].in(earliest_captured_at)).by_captured_at_asc
  end

  def self.table
    self.arel_table
  end
end

Scuttle had emitted this:

Weight.select(Weight.arel_table[Arel.star]).where(
  Weight.arel_table[:captured_at].in(
    Weight.select(Weight.arel_table[:captured_at].minimum).group(
      Arel::Nodes::NamedFunction.new('DATE', [:captured_at])
    ).ast
  )
)

The syntax for NamedFunction is wrong, for one. And the .ast should no longer be passed in. I traced thru Arel to debug this.

camertron commented 8 years ago

Ok, so it looks like the NamedFunction syntax changed in Rails 4.2. Symbols and strings are no longer accepted in the array of function arguments, you have to pass an instance of Arel::Attributes::Attribute instead. Fortunately doing so is backwards-compatible with previous versions, so I should be able to roll out a universal fix for that pretty easily.

Composing multiple select statements via in also changed in Rails 4.2. I tried using the technique @martinstreicher described (i.e. removing the .ast call in the inner select), but that didn't work for me. I'll need to play around more in the console to figure this one out.

martinstreicher commented 8 years ago

When I was debugging in in Arel, it looked like it called .ast on an argument. Thus, you cannot pass in an AST. If I recall, it was the difference between calling in with a SelectManager vs a SelectStatement, where the latter is the AST structure. The Ruby code I posted does work in 4.2.5.1.

On Feb 29, 2016, at 1:24 PM, Cameron Dutro notifications@github.com wrote:

Ok, so it looks like the NamedFunction syntax changed in Rails 4.2. Symbols and strings are no longer accepted in the array of function arguments, you have to pass an instance of Arel::Attributes::Attribute instead. Fortunately doing so is backwards-compatible with previous versions, so I should be able to roll out a universal fix for that pretty easily.

Composing multiple select statements via in also changed in Rails 4.2. I tried using the technique @martinstreicher https://github.com/martinstreicher described (i.e. removing the .ast call in the inner select), but that didn't work for me. I'll need to play around more in the console to figure this one out.

— Reply to this email directly or view it on GitHub https://github.com/camertron/scuttle-rb/issues/4#issuecomment-190320954.

camertron commented 4 years ago

I just shipped a dropdown rails version selector to scuttle.io that removes the errant .ast call for Rails >= 6.0, but more work is still necessary here to support Rails 4.2. Hopefully I'll be able to more easily tackle more of these version-specific issues now that scuttle has an input for it.