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

Unexpected behaviour for CTE joins when CTE has same name as a relation #51249

Closed
dark-panda opened this issue Mar 4, 2024 · 2 comments
Closed

Comments

@dark-panda
Copy link
Contributor

Steps to reproduce

We are big users of CTEs and have been using vlado/activerecord-cte for many years now. While trying to upgrade to Rails 7.1, we came across this situation which previously worked in Vlado's extension (stripped down code follows):

class Task < ApplicationRecord
  has_many :project_resources
  has_many :projects, through: :project_resources
end

class ProjectResource < ApplicationRecord
  belongs_to :resource, polymorphic: true
  belongs_to :project
end

class Project < ApplicationRecord
  has_many :project_resources
  has_many :tasks, through: :project_resources, source: :resource, source_type: 'Task'
end

Project.with(tasks: Task.active).joins(:tasks)

With the extension, this would produce SQL like the following:

WITH tasks AS (
  SELECT tasks.*
  FROM tasks
  WHERE active
)
  SELECT projects.*
  FROM projects
  JOIN project_resources ON project_resources.resource_type = 'Task' AND project_resources.project_id = projects.id
  JOIN tasks ON tasks.id = project_resources.resource_id

With the Rails 7.1 CTE joins, this produces the following:

WITH tasks AS (
  SELECT tasks.*
  FROM tasks
  WHERE active
)
  SELECT projects.*
  FROM projects
  JOIN tasks ON tasks.project_id = projects.id

Which is an invalid SQL query, as project_id doesn't exist on tasks but rather exists on the project_resources relation which sits in between Task and Project. This only happens when the CTE name is the same as an association. For instance, this does not change the join conditions:

Project.with(filtered_tasks: Task.active).joins(:tasks)

Produces

WITH filtered_tasks AS (
  SELECT tasks.*
  FROM tasks
  WHERE active
)
  SELECT projects.*
  FROM projects
  JOIN project_resources ON project_resources.resource_type = 'Task' AND project_resources.project_id = projects.id
  JOIN tasks.id = project_resources.resource_id

Where the join conditions are retained as expected. (The filtered_tasks relation isn't joined here, of course, but the point is is that the :tasks join itself is unaffected by the #with call.)

Masking a table name in a CTE is a very useful capability, but it's not really possible if masking the table impacts the joins.

Expected behavior

I would expect that the joins would not be affected if the relation alias in the WITH query is masking an actual relation.

Actual behavior

The join condition should remain unaffected in the case of the CTE alias being the same as a relation name.

System configuration

Rails version: 7.1.3.2

Ruby version: 3.3.0

Potential Fix

After playing with it a bit, the following appears to satisfy all of our unit tests for our CTEs:

diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 5d63709c3a..f17ed6b562 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1625,7 +1625,7 @@ def build_from
 
       def select_named_joins(join_names, stashed_joins = nil, &block)
         cte_joins, associations = join_names.partition do |join_name|
-          Symbol === join_name && with_values.any? { _1.key?(join_name) }
+          Symbol === join_name && with_values.any? { _1.key?(join_name) } && !reflections[join_name.to_s]
         end
 
         cte_joins.each do |cte_name|

This does not appear to break any existing ActiveRecord tests, so if there is interest in using this I can write up a patch that includes tests.

@fatkodima
Copy link
Member

Please do open a PR.

@rails-bot
Copy link

rails-bot bot commented Jul 5, 2024

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 7-2-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Jul 5, 2024
@rails-bot rails-bot bot closed this as completed Jul 13, 2024
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

2 participants