Skip to content
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

.eager_load when selecting another column sets the ID incorrectly to a record being eager loaded. #51340

Open
PhilCoggins opened this issue Mar 17, 2024 · 1 comment

Comments

@PhilCoggins
Copy link
Contributor

Steps to reproduce

# frozen_string_literal: true

require "bundler/inline"

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

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

  gem "rails", github: "rails/rails", branch: "main"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"

  gem "sqlite3"
end

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

# 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 |t|
    t.integer :first_comment_id
    t.integer :last_comment_id
  end

  create_table :comments, force: true do |t|
    t.integer :post_id
  end
end

class Post < ActiveRecord::Base
  has_many :comments

  belongs_to :first_comment, class_name: "Comment"
  belongs_to :last_comment, class_name: "Comment"

  scope :select_another_column, -> { select("*").select("TIME('now') as now") }
end

class Comment < ActiveRecord::Base
  belongs_to :post
end

class BugTest < Minitest::Test
  def test_association_stuff
    post = Post.create!
    post.first_comment = Comment.create!
    post.last_comment = Comment.create!

    post.save!

    post = Post.find(post.id)
    post_with_eager_loads = Post.eager_load(:first_comment, :last_comment).select_another_column.find(post.id)

    assert_equal post.id, post_with_eager_loads.id
  end
end

Expected behavior

Expect that IDs are consistent when using .eager_load

Actual behavior

ID gets set to one of the other tables joined in

System configuration

Rails version: main

Ruby version: 3.3.0

@Jay0921
Copy link

Jay0921 commented Mar 18, 2024

@PhilCoggins When using select("*") in a joins query, SQL will select all columns from all tables involved in the join. This can lead to unexpected results, especially in cases where tables share column names (the id column, in this case). The ambiguity of identical column names can cause data from one table to overwrite data from another.

A more precise way to handle this situation is to specify the table name in the query, using select("posts.*").

Change select("*").select("TIME('now') as now") to select("posts.*, TIME('now') as now") should solve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants