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

PostgreSQL reload virtual columns on update via RETURNING clause #48628

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

abaldwin88
Copy link
Contributor

@abaldwin88 abaldwin88 commented Jul 2, 2023

Extends Active Record to automatically reload virtual columns on update when using PostgreSQL. This is done by issuing a single UPDATE query that includes a RETURNING clause.

Motivation

Saves an extra round trip to the database for reload and removes a developer pitfall around stale values.

Detail

Given a Post model represented by the following schema:

create_table :posts do |t|
  t.integer :upvotes_count
  t.integer :downvotes_count
  t.virtual :total_votes_count, type: :integer, as: "upvotes_count + downvotes_count", stored: true
end

total_votes_count will reflect the sum of upvotes and downvotes after update is successfully called. Prior to this change calling reload would have been necessary to obtain the value calculated by the database.

post = Post.find(1)
post.update(upvotes_count: 2, downvotes_count: 2)
# Calling `post.reload` no longer necessary
post.total_votes => 4
  • At this moment MySQL and SQLite adapters do not support this behavior

Additional information

CC: @nvasilevski

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@ideasasylum
Copy link

I just checked out Rails main to see how the generated columns work with previous_changes. It worked on create but doesn't on update 😞

Then I tried this branch and it works as expected 🥳

Given a virtual column like:

add_column :products, :premium, :virtual, type: :boolean, as: "price >= 1000", stored: true

I could do:

product = Product.create! user: u, price: 100
product.premium
=> false
product.update price: 2000
=> true
product.premium
=> true
product.previous_changes
=> 
{"price"=>[100, 2000],
 "updated_at"=>[Sun, 23 Jul 2023 22:22:35.416328000 UTC +00:00, Sun, 23 Jul 2023 22:22:51.945466000 UTC +00:00],
 "premium"=>[false, true]}

Yay! I can see the changes caused by the update of the premium virtual column

I hope this makes it into 7.1 as the virtual columns will solve a gnarly problem for me if the dirty tracking works for them

@excid3
Copy link
Contributor

excid3 commented Aug 10, 2023

Ran into this one today as well. Would love to see this fixed.

@abaldwin88 abaldwin88 changed the title PostgreSQL reload virtual attributes on update PostgreSQL reload virtual attributes on update via RETURNING clause Aug 26, 2023
@abaldwin88 abaldwin88 force-pushed the virtual_stored_column_returning_on_update branch from 477cf89 to b71ebf1 Compare August 31, 2023 18:30
@abaldwin88
Copy link
Contributor Author

Rebased onto the latest from main. Added a test for dirty tracking behavior based on the example provided by @ideasasylum

Copy link
Contributor

@nvasilevski nvasilevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the biggest issue with the current implementation is changing return value of public methods which is a breaking change and normally requires a deprecation cycle

Other than that I think it's a very valuable feature to be merged and also a safe one since most of the changes happen behind the scenes and don't require any public APIs to be exposed

@abaldwin88 abaldwin88 changed the title PostgreSQL reload virtual attributes on update via RETURNING clause PostgreSQL reload virtual columns on update via RETURNING clause Sep 18, 2023
@abaldwin88 abaldwin88 force-pushed the virtual_stored_column_returning_on_update branch from b71ebf1 to 7549871 Compare September 19, 2023 17:33
Extends Active Record to reload virtual columns on update when using
PostgreSQL. This is done by issuing a single UPDATE query that includes
a RETURNING clause.

Given a `Post` model represented by the following schema:
```ruby
create_table :posts, id: false do |t|
  t.integer :upvotes_count
  t.integer :downvotes_count
  t.virtual :total_votes_count, type: :integer, as: "upvotes_count + downvotes_count", stored: true
end
```
`total_votes_count` will reflect the sum of upvotes and downvotes after
`update` is successfully called. Prior to this change calling `reload`
would have been necessary to obtain the value calculated by the database.
```ruby
post = Post.find(1)
post.update(upvotes_count: 2, downvotes_count: 2)
post.total_votes => 4
```

* At this moment MySQL and SQLite adapters do not support this behavior
@abaldwin88 abaldwin88 force-pushed the virtual_stored_column_returning_on_update branch from 7549871 to f708a50 Compare September 19, 2023 18:04
@abaldwin88
Copy link
Contributor Author

Thanks for the feedback @nvasilevski. I've implemented your suggestion for keeping the signature the same expect when returning is specified. I also made a couple other small revisions based on your input, added docs, and rebased onto latest from main.

Build error from Rubocop appears to be from main. This PR does not modify action_mailer_basics.md.

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

Successfully merging this pull request may close these issues.

Virtual stored columns should return new values on update
4 participants