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

Complex PG query yields different results under Rails 6 #37374

Closed
portwood opened this issue Oct 4, 2019 · 4 comments
Closed

Complex PG query yields different results under Rails 6 #37374

portwood opened this issue Oct 4, 2019 · 4 comments
Assignees

Comments

@portwood
Copy link

portwood commented Oct 4, 2019

Steps to reproduce

ActiveRecord produces a different and incorrect PG query under Rails 6.

Expected behavior

Rails 5.2.3:

domains = @company.company_domains.includes(:people).where(people: { hr_invalidated: false }).where.not(id: @company.blocked_domains.ids)

tp = domains.where.not(people: { plus_member_at: nil })
 
tp.where(people: {disabled: false, blocked: false}).where.not(people: { confirmed_at: nil, special: true }).explain

EXPLAIN for: SELECT "domains"."id" AS t0_r0, "domains"."name" AS t0_r1, "domains"."created_at" AS t0_r2, "domains"."updated_at" AS t0_r3, "people"."id" AS t1_r0, "people"."domain_id" AS t1_r1, "people"."blocked" AS t1_r2, "people"."opt_out" AS t1_r3, "people"."hr_admin" AS t1_r4, "people"."weekly_email_digest" AS t1_r5, "people"."terms_and_conditions" AS t1_r6, "people"."encrypted_first_name" AS t1_r7, "people"."encrypted_first_name_iv" AS t1_r8, "people"."encrypted_last_name" AS t1_r9, "people"."encrypted_last_name_iv" AS t1_r10, "people"."encrypted_name" AS t1_r11, "people"."encrypted_name_iv" AS t1_r12, "people"."encrypted_nickname" AS t1_r13, "people"."encrypted_nickname_iv" AS t1_r14, "people"."email" AS t1_r15, "people"."encrypted_title" AS t1_r16, "people"."encrypted_title_iv" AS t1_r17, "people"."encrypted_location" AS t1_r18, "people"."encrypted_location_iv" AS t1_r19, "people"."encrypted_quote" AS t1_r20, "people"."encrypted_quote_iv" AS t1_r21, "people"."avatar_file_name" AS t1_r22, "people"."avatar_content_type" AS t1_r23, "people"."avatar_file_size" AS t1_r24, "people"."avatar_updated_at" AS t1_r25, "people"."created_at" AS t1_r26, "people"."updated_at" AS t1_r27, "people"."disabled" AS t1_r28, "people"."encrypted_password" AS t1_r29, "people"."reset_password_token" AS t1_r30, "people"."reset_password_sent_at" AS t1_r31, "people"."remember_created_at" AS t1_r32, "people"."sign_in_count" AS t1_r33, "people"."current_sign_in_at" AS t1_r34, "people"."last_sign_in_at" AS t1_r35, "people"."current_sign_in_ip" AS t1_r36, "people"."last_sign_in_ip" AS t1_r37, "people"."confirmation_token" AS t1_r38, "people"."confirmed_at" AS t1_r39, "people"."confirmation_sent_at" AS t1_r40, "people"."unconfirmed_email" AS t1_r41, "people"."failed_attempts" AS t1_r42, "people"."unlock_token" AS t1_r43, "people"."locked_at" AS t1_r44, "people"."address_id" AS t1_r45, "people"."company_id" AS t1_r46, "people"."timezone" AS t1_r47, "people"."encrypted_manager_id" AS t1_r48, "people"."encrypted_manager_id_iv" AS t1_r49, "people"."encrypted_employee_id" AS t1_r50, "people"."encrypted_employee_id_iv" AS t1_r51, "people"."payment_token" AS t1_r52, "people"."last_import" AS t1_r53, "people"."discretionary_gift_approved" AS t1_r54, "people"."hr_super_admin" AS t1_r55, "people"."hr_admin_digest_reports" AS t1_r56, "people"."enhanced_experience" AS t1_r57, "people"."dob" AS t1_r58, "people"."service_date" AS t1_r59, "people"."plus_member_at" AS t1_r60, "people"."department_id" AS t1_r61, "people"."eea_countries_id" AS t1_r62, "people"."eea_resident" AS t1_r63, "people"."send_scheduled_reminder" AS t1_r64, "people"."hr_invalidated" AS t1_r65, "people"."special" AS t1_r66, "people"."role" AS t1_r67, "people"."weekly_notes_goal" AS t1_r68, "people"."session_validity_token" AS t1_r69, "people"."sk" AS t1_r70, "people"."first_name_bidx" AS t1_r71, "people"."last_name_bidx" AS t1_r72, "people"."employee_id_bidx" AS t1_r73, "people"."manager_id_bidx" AS t1_r74, "people"."location_bidx" AS t1_r75, "people"."include_with_department_id" AS t1_r76 FROM "domains" LEFT OUTER JOIN "people" ON "people"."domain_id" = "domains"."id" INNER JOIN "associated_memberships" ON "domains"."id" = "associated_memberships"."domain_id" WHERE "associated_memberships"."company_id" = $1 AND "people"."hr_invalidated" = $2 AND 1=1 AND "people"."plus_member_at" IS NOT NULL AND "people"."disabled" = $3 AND "people"."blocked" = $4 AND "people"."confirmed_at" IS NOT NULL AND "people"."special" != $5 [["company_id", 2], ["hr_invalidated", false], ["disabled", false], ["blocked", false], ["special", true]]

                                                                          QUERY PLAN
---------------------------------------------------------------------------------------------------------------------------------------------------------------
 Nested Loop  (cost=5.72..52.93 rows=8 width=1054)
   ->  Hash Join  (cost=5.44..11.90 rows=2 width=37)
         Hash Cond: (domains.id = associated_memberships.domain_id)
         ->  Seq Scan on domains  (cost=0.00..5.73 rows=273 width=33)
         ->  Hash  (cost=5.41..5.41 rows=2 width=4)
               ->  Seq Scan on associated_memberships  (cost=0.00..5.41 rows=2 width=4)
                     Filter: (company_id = 2)
   ->  Index Scan using index_people_on_domain_id on people  (cost=0.29..20.48 rows=4 width=1021)
         Index Cond: (domain_id = domains.id)
         Filter: ((NOT hr_invalidated) AND (plus_member_at IS NOT NULL) AND (NOT disabled) AND (NOT blocked) AND (confirmed_at IS NOT NULL) AND (NOT special))
(10 rows)

This query yields the correct results.

Actual behavior

The same query generated a different SQL query.

domains = @company.company_domains.includes(:people).where(people: { hr_invalidated: false }).where.not(id: @company.blocked_domains.ids)

tp = domains.where.not(people: { plus_member_at: nil })

tp.where(people: {disabled: false, blocked: false}).where.not(people: {plus_member_at: nil, special: true}).explain

EXPLAIN for: SELECT "domains"."id" AS t0_r0, "domains"."name" AS t0_r1, "domains"."created_at" AS t0_r2, "domains"."updated_at" AS t0_r3, "people"."id" AS t1_r0, "people"."domain_id" AS t1_r1, "people"."blocked" AS t1_r2, "people"."opt_out" AS t1_r3, "people"."hr_admin" AS t1_r4, "people"."weekly_email_digest" AS t1_r5, "people"."terms_and_conditions" AS t1_r6, "people"."encrypted_first_name" AS t1_r7, "people"."encrypted_first_name_iv" AS t1_r8, "people"."encrypted_last_name" AS t1_r9, "people"."encrypted_last_name_iv" AS t1_r10, "people"."encrypted_name" AS t1_r11, "people"."encrypted_name_iv" AS t1_r12, "people"."encrypted_nickname" AS t1_r13, "people"."encrypted_nickname_iv" AS t1_r14, "people"."email" AS t1_r15, "people"."encrypted_title" AS t1_r16, "people"."encrypted_title_iv" AS t1_r17, "people"."encrypted_location" AS t1_r18, "people"."encrypted_location_iv" AS t1_r19, "people"."encrypted_quote" AS t1_r20, "people"."encrypted_quote_iv" AS t1_r21, "people"."avatar_file_name" AS t1_r22, "people"."avatar_content_type" AS t1_r23, "people"."avatar_file_size" AS t1_r24, "people"."avatar_updated_at" AS t1_r25, "people"."created_at" AS t1_r26, "people"."updated_at" AS t1_r27, "people"."disabled" AS t1_r28, "people"."encrypted_password" AS t1_r29, "people"."reset_password_token" AS t1_r30, "people"."reset_password_sent_at" AS t1_r31, "people"."remember_created_at" AS t1_r32, "people"."sign_in_count" AS t1_r33, "people"."current_sign_in_at" AS t1_r34, "people"."last_sign_in_at" AS t1_r35, "people"."current_sign_in_ip" AS t1_r36, "people"."last_sign_in_ip" AS t1_r37, "people"."confirmation_token" AS t1_r38, "people"."confirmed_at" AS t1_r39, "people"."confirmation_sent_at" AS t1_r40, "people"."unconfirmed_email" AS t1_r41, "people"."failed_attempts" AS t1_r42, "people"."unlock_token" AS t1_r43, "people"."locked_at" AS t1_r44, "people"."address_id" AS t1_r45, "people"."company_id" AS t1_r46, "people"."timezone" AS t1_r47, "people"."encrypted_manager_id" AS t1_r48, "people"."encrypted_manager_id_iv" AS t1_r49, "people"."encrypted_employee_id" AS t1_r50, "people"."encrypted_employee_id_iv" AS t1_r51, "people"."payment_token" AS t1_r52, "people"."last_import" AS t1_r53, "people"."discretionary_gift_approved" AS t1_r54, "people"."hr_super_admin" AS t1_r55, "people"."hr_admin_digest_reports" AS t1_r56, "people"."enhanced_experience" AS t1_r57, "people"."dob" AS t1_r58, "people"."service_date" AS t1_r59, "people"."plus_member_at" AS t1_r60, "people"."department_id" AS t1_r61, "people"."eea_countries_id" AS t1_r62, "people"."eea_resident" AS t1_r63, "people"."send_scheduled_reminder" AS t1_r64, "people"."hr_invalidated" AS t1_r65, "people"."special" AS t1_r66, "people"."role" AS t1_r67, "people"."weekly_notes_goal" AS t1_r68, "people"."session_validity_token" AS t1_r69, "people"."sk" AS t1_r70, "people"."first_name_bidx" AS t1_r71, "people"."last_name_bidx" AS t1_r72, "people"."employee_id_bidx" AS t1_r73, "people"."manager_id_bidx" AS t1_r74, "people"."location_bidx" AS t1_r75, "people"."include_with_department_id" AS t1_r76 FROM "domains" INNER JOIN "associated_memberships" ON "domains"."id" = "associated_memberships"."domain_id" LEFT OUTER JOIN "people" ON "people"."domain_id" = "domains"."id" WHERE "associated_memberships"."company_id" = $1 AND "people"."hr_invalidated" = $2 AND 1=1 AND "people"."plus_member_at" IS NOT NULL AND "people"."disabled" = $3 AND "people"."blocked" = $4 AND NOT ("people"."plus_member_at" IS NULL AND "people"."special" = $5) [["company_id", 2], ["hr_invalidated", false], ["disabled", false], ["blocked", false], ["special", true]]

                                                                            QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Nested Loop  (cost=5.72..53.61 rows=75 width=1054)
   ->  Hash Join  (cost=5.44..11.90 rows=2 width=37)
         Hash Cond: (domains.id = associated_memberships.domain_id)
         ->  Seq Scan on domains  (cost=0.00..5.73 rows=273 width=33)
         ->  Hash  (cost=5.41..5.41 rows=2 width=4)
               ->  Seq Scan on associated_memberships  (cost=0.00..5.41 rows=2 width=4)
                     Filter: (company_id = 2)
   ->  Index Scan using index_people_on_domain_id on people  (cost=0.29..20.48 rows=38 width=1021)
         Index Cond: (domain_id = domains.id)
         Filter: ((NOT hr_invalidated) AND (plus_member_at IS NOT NULL) AND (NOT disabled) AND (NOT blocked) AND ((plus_member_at IS NOT NULL) OR (NOT special)))
(10 rows)

The differences from 5.2.3 are at the end of the plan. "((plus_member_at IS NOT NULL) OR (NOT special))"

System configuration

pg (1.1.4)
Rails version:
6.0
Ruby version:
2.6.5

@matthewd
Copy link
Member

matthewd commented Oct 5, 2019

Thanks for reporting this -- it does indeed sound like a bug in #36029 (cc @kamipo).

6.0 should be behaving as 5.2 did, but showing a deprecation warning that this behaviour will change in 6.1.

To work around the problem (and also become ready for 6.1), you can separate out the not-conditions: where.not(people: { confirmed_at: nil }).where.not(people: { special: true })

Sorry that's a bit wordier for your use case.. the intent is to make where.not more like the opposite of where -- so here, it's aiming for the opposite set of rows from what would be matched by where(people: { confirmed_at: nil, special: true })

@kamipo kamipo self-assigned this Oct 8, 2019
@sebastian-palma
Copy link
Contributor

sebastian-palma commented Oct 9, 2019

Those aren't the same queries.

You're using where.not(people: { confirmed_at: nil, ...}) in the same query (Rails 5.2.3) and where.not(people: { plus_member_at: nil, ...}) in the second example (Rails 6.0.0).

That confirms the difference you added "The differences from 5.2.3 are at the end of the plan. "((plus_member_at IS NOT NULL) OR (NOT special))"".

Can you clarify, was it a typo?

@portwood
Copy link
Author

The previous comment from @matthewd clarified the changes, and I was able to resolve my immediate issues. My assumption was #where.not with compound arguments acted like a NOR in Rails 6 (as it did in prior versions of Rails). I received no deprecation notice when running my application under Rails 6, but received incorrect results. I've updated the code and now avoid compound arguments to #where.not.

I still believe that with this behavior change, there should be a deprecation warning on statements containing compound arguments to the method.

@rails-bot
Copy link

rails-bot bot commented Jan 19, 2020

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 6-0-stable branch or on master, 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 Jan 19, 2020
@rails-bot rails-bot bot closed this as completed Jan 26, 2020
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

4 participants