rails / arel

A Relational Algebra
2.06k stars 390 forks source link

Generate insert statement without deprecation notices #452

Closed dv closed 7 years ago

dv commented 7 years ago

We're trying to make the POLO gem work with Rails 5. Polo is a gem that generates INSERT statements for the records that already exist in the database.

After a cursory look through the codepaths, I can't figure out how to avoid the following deprecation notice:

DEPRECATION WARNING: Passing a column to `quote` has been deprecated. It is only used for type casting, which should be handled elsewhere. See https://github.com/rails/arel/commit/6160bfbda1d1781c3b08a33ec4955f170e95be11 for more information. (called from raw_sql at /Users/david/poetry/polo/lib/polo/sql_translator.rb:53)
Arel performing automatic type casting is deprecated, and will be removed in Arel 8.0. If you are seeing this, it is because you are manually passing a value to an Arel predicate, and the `Arel::Table` object was constructed manually. The easiest way to remove this warning is to use an `Arel::Table` object returned from calling `arel_table` on an ActiveRecord::Base subclass.

If you're certain the value is already of the right type, change `attribute.eq(value)` to `attribute.eq(Arel::Nodes::Quoted.new(value))` (you will be able to remove that in Arel 8.0, it is only required to silence this deprecation warning).

You can also silence this warning globally by setting `$arel_silence_type_casting_deprecation` to `true`. (Do NOT do this if you are a library author)

If you are passing user input to a predicate, you must either give an appropriate type caster object to the `Arel::Table`, or manually cast the value before passing it to Arel.

After looking through how Rails does it, and some other documentation, I've come up with the following few lines that should generate the INSERT statement, however the deprecation notice still happens (at the last line):

# `record` is an AR model instance
values = record.send(:arel_attributes_with_values_for_create, record.attribute_names)

values.each do |attribute, value|
  values[attribute] = record.class.arel_table.type_cast_for_database(attribute.name, value)
  # Also tried:
  # values[attribute] = record.class.connection.quote(record.class.arel_table.type_cast_for_database(attribute.name, value))
  # values[attribute] = Arel::Nodes::Quoted.new(record.class.arel_table.type_cast_for_database(attribute.name, value)))
end

model = record.class
scope = model.unscoped

im = scope.arel.create_insert
im.into model.arel_table

im.insert values
im.to_sql

It seems from this line that it's actually impossible not to call quote without a column, unless the value is an SqlLiteral. The deprecation notice talks about a Quoted but I'm not sure that's relevant for our usecase (I tried wrapping each value in a Quoted, then we got the error "can't quote quoted").

Is it possible to avoid the deprecation notice when generating an INSERT statement using Arel? I must be overlooking something but can't figure it out. Any help would be appreciated.

dv commented 7 years ago

After rubberducking this here, I figured I should re-research the Rails 5 codepath. I first thought the subs/bindings are used for prepared statements but after further investigation that's not the case. I came up with the following which seems to do the trick. I'll refactor and test this a bit and use that as a solution.

      values = record.send(:arel_attributes_with_values_for_create, record.attribute_names)

      model = record.class
      scope = model.unscoped

      im = scope.arel.create_insert
      im.into model.arel_table

      substitutes, binds = scope.substitute_values(values)
      im.insert substitutes

      record.class.connection.to_sql(im, binds)
dv commented 7 years ago

For posterity's sake: It actually does have something to do with prepared statements (doh), and can be fixed by surrounding the last statement with model.connection.unprepared_statement. There's probably a less roundabout way to do it, but I've used up my time budget for this issue and found a Good Enough (tm) solution.