2 posts tagged with "Software Engineers"

This is my personal journey on becoming a Rails Contributor. This is a less technical blog. The technical version can be found here.

One day at work I found a problem with a Rails query while working on our internal filtering gem.

I quickly found an existing issue on the rails repository. I thought, "Great! The Rails maintainers know about it so it's likely being fixed soon". But then I looked at when the issue was created. 2021. It was three years old. So much for it being fixed soon 🤷🏻‍♂️. I looked into the issue a bit more and learned that it was introduced after a "caching" layer was added to ActiveRecord query generation logic. There were multiple comments on the issue but only one comment from @oneiros had a suggested solution.

That was it. I went back to work and didn't think any more of the issue. If it was an issue for three years, it likely wasn't an easy fix and wasn't a high priority for any maintainer. I couldn't fix it. I was just a normal dev.

During a weekly sync with my manager, he suggested that I should attempt to fix the issue. We have personal development time at Fleetio and he thought this would be a good use of that time. Even if I couldn't fix it, I would still be learning and growing my dev skills. I agreed, so the following Friday I decided to try and fix this bug.

My first step was to figure out how to contribute to Rails. Luckily there is great documentation for that. I followed the steps to use the VS Code remote containers plugin. This was my first time working with the new rails .devcontainer setup but it was a lot easier than I expected. Once I had the Rails environment set up, I ran the ActiveRecord specs. (Rails is split up into multiple gems, and this bug was within the active_record). The specs passed. I successfully had the dev environment set up.

All of this set up took around 15 minutes. A lot faster than it could have if I tried to set up everything without the dev container.

Now that I had the environment set up, I needed to generate a failing spec. This proved a lot harder than I expected. The bug was tough to reproduce. Eventually, after about an hour, I was able to create the failing spec.

Now I could actually start to work on the bug!

I used the comment from the issue as a starting point. I just copied the fix and my test case passed! 🎉 I quickly created a PR to Rails. This was a mistake. I got excited and rushed the implementation. But I made the mistake of not running the rest of the test suite. While my problem was fixed, the original change spec was failing. All I did was "revert" the original change.

Since the easy fix didn't work, now I actually had to try. I spent a few hours trying different approaches that I won't get into in this post. See this blog if you want more. Finally, I found a solution that had my spec and the original change spec both passing! 🎉 I created a second PR to Rails and called it a day.

The following Monday I checked on the PR and the build specs were failing. I thought "How could this happen? It ran fine locally!". It turns out, I wrote my spec in a way that failed if MySql was used as the database. MySql and Postgres/SqlLite use different quotation marks/styles. I quickly updated my tests, ran the entire test suite with each database option, and submitted yet another PR to Rails.

After three days of no feedback, my PR was merged! Two months later, it was released as part of Rails v7.1.5.

Key takeaways​

• Being a contributor to an open source project can be intimidating. Don't let that stop you!
• Understand the problem completely before attempting to fix it.
• Be patient and run all tests before creating a PR.

This is a more technical dive into an issue I fixed in Rails. If you want a less technical/more personal blog, check out that blog.

Background:​

All code examples and links are from rails Rails 7.2.1. This was resolved in Rails 7.2.2.

Glossary:

• Association - A high level declaration of how two models are related in Rails.
• Reflection - A detailed representation of how an association is generated in Rails.
• Reflection Chain - A list of reflections that connect two models. Used by Rails to generate SQL joins.

ActiveRecord builds a reflection chain that connects the parent model to it's joined association.

Given the following class:

class Parent < ActiveRecord::Base
  has_one :child
  has_one :grandchild, through: :child
end

Parent.joins(:child) has a reflection chain of Parent -> Child and Parent.joins(:grandchild) has a reflection chain of Parent -> Child -> Grandchild.

Problem:​

.left_outer_joins() resulted in incorrect SQL joins when a child association shared the same parent association but not entire association ancestry.

I found this problem while working on our internal record filtering gem. A simplified overview of our set up is below.

class PurchaseOrder < ActiveRecord::Base
  belongs_to :created_by, class_name: "User"
  has_one :created_by_contact, class_name: "Contact", through: :created_by, source: :scoped_contact

  belongs_to :approved_by, class_name: "User"
  has_one :approved_by_contact, class_name: "Contact", through: :approved_by, source: :scoped_contact
end

class User < ActiveRecord::Base
  has_many :account_memberships, inverse_of: :user, dependent: :destroy
  has_one :scoped_contact, class_name: "Contact", through: :scoped_account_membership, source: :contact
end

class AccountMembership < ActiveRecord::Base
  belongs_to :contact
end

When we perform an inner join we get:

-- PurchaseOrder.joins(:created_by_contact, :approved_by_contact).to_sql

SELECT
    "purchase_orders".*
FROM
    "purchase_orders"
    INNER JOIN "users" ON "users"."id" = "purchase_orders"."created_by_id"
    INNER JOIN "account_memberships" ON "account_memberships"."user_id" = "users"."id"
    INNER JOIN "contacts" ON "contacts"."id" = "account_memberships"."contact_id"
    INNER JOIN "users" "approved_bies_purchase_orders_join" ON "approved_bies_purchase_orders_join"."id" = "purchase_orders"."approved_by_id"
    INNER JOIN "account_memberships" "scoped_account_memberships_purchase_orders_join" ON "scoped_account_memberships_purchase_orders_join"."user_id" = "approved_bies_purchase_orders_join"."id"
    INNER JOIN "contacts" "approved_by_contacts_purchase_orders" ON "approved_by_contacts_purchase_orders"."id" = "scoped_account_memberships_purchase_orders_join"."contact_id"

We see that we correctly join "purchase_orders"."created_by_id" to contacts and "purchase_orders"."approved_by_id" to approved_by_contacts_purchase_orders (an alias for contacts).

However, if we want to use .left_joins we see a problem.

-- PurchaseOrder.left_joins(:created_by_contact, :approved_by_contact)

SELECT
    "purchase_orders".*
FROM
    "purchase_orders"
    LEFT OUTER JOIN "users" ON "users"."id" = "purchase_orders"."created_by_id"
    LEFT OUTER JOIN "account_memberships" ON "account_memberships"."user_id" = "users"."id"
    LEFT OUTER JOIN "contacts" ON "contacts"."id" = "account_memberships"."contact_id"
    LEFT OUTER JOIN "contacts" "approved_by_contacts_purchase_orders" ON "approved_by_contacts_purchase_orders"."id" = "account_memberships"."contact_id"

We correctly join "purchase_orders"."created_by_id" to contacts but "purchase_orders"."created_by_id" is also joined to approved_by_contacts_purchase_orders.

We are missing the join to the approved_by_contact 😱

Explanation of Problem:​

Given the following code:

PurchaseOrder.left_joins(:created_by_contact, :approved_by_contact)

The created_by_contact association with the PurchaseOrder model has a reflection chain of PurchaseOrder(created_by) -> User -> AccountMembership -> Contact.

When left joining on created_by_contact, the make_constraints method caches the reflection chain at each level. The cache key is the last reflection item of the reflection chain.

The following cache is generated for the created_by_contact reflection chain.

{
  Contact: [PurchaseOrder(created_by), User, AccountMembership, Contact],
  AccountMembership: [PurchaseOrder(created_by), User, AccountMembership],
  User: [PurchaseOrder(created_by), User],
  PurchaseOrder(created_by): [PurchaseOrder(created_by)]
}

Since created_by_contact is the first association joined, every reflection chain item is added to the cache.

The approved_by_contact association with the PurchaseOrder model has a reflection chain of PurchaseOrder(approved_by) -> User -> AccountMembership -> Contact.

When we add :approved_by_contact to the .left_joins() method, the make_constraints method checks the cache and finds a match on the Contact reflection key for the value of [PurchaseOrder(created_by), User, AccountMembership, Contact] and stops building any further join constraints.

Solution:​

Now that I know the problem, I can fix it. The make_constraints method didn't have enough information to discern if the reflection chain was a full or partial match.

I updated the code within the make_constraints method and passed the entire reflection chain to use as the cache key.

This allowed make_constraints to successfully match on when the reflection chain was the same but not match in my case when the reflection chain was different.

I created a PR to Rails with my fix and it was merged after three days! Two months later, it was released as part of Rails v7.2.2.