New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't merge order relation containing `?` #33664

Closed
NaturalHokke opened this Issue Aug 20, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@NaturalHokke
Copy link

NaturalHokke commented Aug 20, 2018

Steps to reproduce

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "5.2.1"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Ensure backward compatibility with Minitest 4
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do
  end

  create_table :search_posts, force: true do |t|
    t.integer :post_id
    t.string :name
  end
end

class Post < ActiveRecord::Base
  has_one :search_post
end

class SearchPost < ActiveRecord::Base
  belongs_to :post
end

class BugTest < Minitest::Test
  def test_association_stuff
    post = Post.create!
    post.create_search_post(name: '!')
    post = Post.create!
    post.create_search_post(name: '?')

    assert_equal 2, Post.count
    assert_equal 2, SearchPost.count
    assert_equal [2, 1], SearchPost.order(Arel.sql "CASE name WHEN '?' THEN 1 ELSE 0 END DESC").ids
    assert_equal [2, 1], Post.joins(:search_post).merge(SearchPost.order(Arel.sql "CASE name WHEN '?' THEN 1 ELSE 0 DESC")).ids
  end
end

Expected behavior

return records

Actual behavior

raise ActiveRecord::PreparedStatementInvalid: wrong number of bind variables (0 for 1) in: CASE name WHEN '?' THEN 1 ELSE 0 DESC

System configuration

ActiveRecord version: any

@kamipo kamipo closed this in 96cd16b Aug 20, 2018

@kamipo

This comment has been minimized.

Copy link
Member

kamipo commented Aug 20, 2018

Thanks for the report.
It is a bug for Relation::Merger then I've fixed it in 96cd16b.

kamipo added a commit that referenced this issue Aug 21, 2018

Fix merging relation that order including `?`
The `Relation::Merger` has a problem that order values would be merged
as nested array.

That was caused an issue #33664 since if array value is passed to
`order` and first element in the array includes `?`, the array is
regarded as a prepared statement and bind variables.

https://api.rubyonrails.org/classes/ActiveRecord/Sanitization/ClassMethods.html#method-i-sanitize_sql_for_order

Just merging that as splat args like other values would fix the issue.

Fixes #33664.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment