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

activerecord: Duplicate aliases in generated query when combining `#joins()` and `#left_outer_joins()` with common join dependencies #30504

Closed
zapo opened this Issue Sep 1, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@zapo

zapo commented Sep 1, 2017

It's possible to generate queries with duplicate aliases when combining Relation#joins() and Relaction#left_outer_joins() that have common join dependencies.

Steps to reproduce

Considering the following schema:

class User < ActiveRecord::Base
  has_many :posts
end

class Post < ActiveRecord::Base
  has_many :comments
end

Any of the following query will fail with ambiguous column (sqlite) or duplicate alias (postgresql):

User.joins(:posts).left_outer_joins(:posts).count
User.joins(:posts).left_outer_joins(:posts => :comments).count

# Both end up generating duplicate aliases for posts, eg:
# SELECT COUNT(*) FROM "users" INNER JOIN "posts" ON "posts"."user_id" = "users"."id" 
# LEFT OUTER JOIN "posts" ON "posts"."user_id" = "users"."id"

I have a PR that fixes this on master #30410. I can backport for 5.1, 5.0 if needed (cc @eileencodes)

Expected behavior

The LEFT OUTER JOIN fragment of that query should alias the "posts" table and the resulting query should be executable.

Actual behavior

Both joins are generated with their own alias tracker and end up either not using aliases (in this case) or could be using colliding aliases (if more joins where added by chaining additional associations in either the inner or left joins depending on "posts").

System configuration

Rails version: 5.0.x, 5.1.x, 5.2.0-alpha

Ruby version: 2.3, 2.4

@ckritzinger

This comment has been minimized.

Show comment
Hide comment
@ckritzinger

ckritzinger Sep 4, 2017

Nice! In the meantime, I'm using this workaround:

  scope :from_groups, ->(groups) { # No space between "->" and "("
    # See https://github.com/rails/rails/issues/30504
    joins("LEFT OUTER JOIN user_groups AS u_g ON u_g.user_id = users.id").where(u_g: { group_id: groups }).distinct if groups.present?
  }

(basically, just hacking the alias manually)

ckritzinger commented Sep 4, 2017

Nice! In the meantime, I'm using this workaround:

  scope :from_groups, ->(groups) { # No space between "->" and "("
    # See https://github.com/rails/rails/issues/30504
    joins("LEFT OUTER JOIN user_groups AS u_g ON u_g.user_id = users.id").where(u_g: { group_id: groups }).distinct if groups.present?
  }

(basically, just hacking the alias manually)

@mjy

This comment has been minimized.

Show comment
Hide comment
@mjy

mjy Sep 5, 2017

Contributor

Perhaps related. I have a scope that works in 4.2 and 5.0. It fails now very subtly. Very similar setup, the join references a has_many through the same table:

 scope :with_sequence_name, ->(name) { joins(derived_extracts: [:derived_sequences]).where(sequences: {name: name}) }

See the SQL bellow, identical but for the isolated line:

Join SQL in 4.2 and 5.0: (passes tests)

 %Q{INNER JOIN  "origin_relationships"
           ON  "origin_relationships"."old_object_id" = "collection_objects"."id"
              AND  "origin_relationships"."new_object_type" = 'Extract'
              AND  "origin_relationships"."old_object_type" = 'CollectionObject'
   INNER JOIN  "extracts"
           ON  "extracts"."id" =  "origin_relationships"."new_object_id"
   INNER JOIN  "origin_relationships" "origin_relationships_extracts_join"
           ON  "origin_relationships_extracts_join"."old_object_id" = "extracts"."id"

              AND  "origin_relationships_extracts_join"."new_object_type" = 'Sequence'

              AND  "origin_relationships_extracts_join"."old_object_type" = 'Extract'
   INNER JOIN  "sequences"
           ON  "sequences"."id" = "origin_relationships_extracts_join"."new_object_id"}

Join SQL in 5.1.3 (fails tests)

%Q{INNER JOIN  "origin_relationships"
           ON  "origin_relationships"."old_object_id" = "collection_objects"."id"
              AND  "origin_relationships"."new_object_type" = 'Extract'
              AND  "origin_relationships"."old_object_type" = 'CollectionObject'
   INNER JOIN  "extracts"
           ON  "extracts"."id" =  "origin_relationships"."new_object_id"
   INNER JOIN  "origin_relationships"  "origin_relationships_extracts_join"
           ON  "origin_relationships_extracts_join"."old_object_id" = "extracts"."id"

              AND  "origin_relationships"."new_object_type" = 'Sequence'  

              AND  "origin_relationships_extracts_join"."old_object_type" = 'Extract'
   INNER JOIN  "sequences"
           ON  "sequences"."id" =  "origin_relationships_extracts_join"."new_object_id"}

System configuration

Rails version: 5.1.3
Ruby version: 2.4.1
Postgres 9.6.3

Contributor

mjy commented Sep 5, 2017

Perhaps related. I have a scope that works in 4.2 and 5.0. It fails now very subtly. Very similar setup, the join references a has_many through the same table:

 scope :with_sequence_name, ->(name) { joins(derived_extracts: [:derived_sequences]).where(sequences: {name: name}) }

See the SQL bellow, identical but for the isolated line:

Join SQL in 4.2 and 5.0: (passes tests)

 %Q{INNER JOIN  "origin_relationships"
           ON  "origin_relationships"."old_object_id" = "collection_objects"."id"
              AND  "origin_relationships"."new_object_type" = 'Extract'
              AND  "origin_relationships"."old_object_type" = 'CollectionObject'
   INNER JOIN  "extracts"
           ON  "extracts"."id" =  "origin_relationships"."new_object_id"
   INNER JOIN  "origin_relationships" "origin_relationships_extracts_join"
           ON  "origin_relationships_extracts_join"."old_object_id" = "extracts"."id"

              AND  "origin_relationships_extracts_join"."new_object_type" = 'Sequence'

              AND  "origin_relationships_extracts_join"."old_object_type" = 'Extract'
   INNER JOIN  "sequences"
           ON  "sequences"."id" = "origin_relationships_extracts_join"."new_object_id"}

Join SQL in 5.1.3 (fails tests)

%Q{INNER JOIN  "origin_relationships"
           ON  "origin_relationships"."old_object_id" = "collection_objects"."id"
              AND  "origin_relationships"."new_object_type" = 'Extract'
              AND  "origin_relationships"."old_object_type" = 'CollectionObject'
   INNER JOIN  "extracts"
           ON  "extracts"."id" =  "origin_relationships"."new_object_id"
   INNER JOIN  "origin_relationships"  "origin_relationships_extracts_join"
           ON  "origin_relationships_extracts_join"."old_object_id" = "extracts"."id"

              AND  "origin_relationships"."new_object_type" = 'Sequence'  

              AND  "origin_relationships_extracts_join"."old_object_type" = 'Extract'
   INNER JOIN  "sequences"
           ON  "sequences"."id" =  "origin_relationships_extracts_join"."new_object_id"}

System configuration

Rails version: 5.1.3
Ruby version: 2.4.1
Postgres 9.6.3

@zapo

This comment has been minimized.

Show comment
Hide comment
@zapo

zapo Sep 5, 2017

@mjy Perhaps. I don't think my patch as is would solve this though since this seems more related to STI join conditions computation. Not sure if worth opening a new ticket or not.

zapo commented Sep 5, 2017

@mjy Perhaps. I don't think my patch as is would solve this though since this seems more related to STI join conditions computation. Not sure if worth opening a new ticket or not.

@mjy

This comment has been minimized.

Show comment
Hide comment
@mjy

mjy Sep 5, 2017

Contributor

@zapo Got it. For what it's worth none of the models in my issue I'm using are STI.

Might be worth mentioning in my case that my example is using a double polymorphic join table (origin_relationships), and through relationships.

I can open a new ticket if folks think its worthwhile, just let me know.

Contributor

mjy commented Sep 5, 2017

@zapo Got it. For what it's worth none of the models in my issue I'm using are STI.

Might be worth mentioning in my case that my example is using a double polymorphic join table (origin_relationships), and through relationships.

I can open a new ticket if folks think its worthwhile, just let me know.

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