PostgreSQL reload virtual columns on update via RETURNING clause by abaldwin88 · Pull Request #48628 · rails/rails

abaldwin88 · 2023-07-02T11:30:36Z

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

Follow-up from PR Assign auto populated columns on Active Record object creation #48241
Related to PR Add ActiveRecord.reload_attributes #48434
Closes Virtual stored columns should return new values on update #48423 when using Postgresql

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 · 2023-07-23T22:30:30Z

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 · 2023-08-10T16:50:31Z

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

abaldwin88 · 2023-08-31T18:31:22Z

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

nvasilevski

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

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

activerecord/lib/active_record/connection_adapters/mysql2/database_statements.rb

abaldwin88 · 2023-09-19T18:18:04Z

Thanks for the feedback @nvasilevski. I've implemented your suggestion for keeping the signature the same except 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.

skcc321 · 2024-07-02T13:58:14Z

Hi guys. Any chance the PR can be merged soon?

rafaelfranca

This PR needs a rebase

rafaelfranca · 2024-09-17T01:30:10Z

activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb

+          return super unless returning_columns&.any?
+
+          returning_columns_statement = returning_columns.map { |c| quote_column_name(c) }.join(", ")
+          sql = "#{sql} RETURNING #{returning_columns_statement}"


Instead of concat sqls, can we just build the right AST in the right place? We probably need to build the AST before passing to exec_update in the update method.

Sorry I'm not entirely clear on the ask here. Is there something different here than sql_for_insert which uses the same code pattern?

I'm planning to move this method from being postgres specific and instead put it under the abstract class. Does that help address your concerns?

My two cents is any further changes around this would be better handled as a separate follow-up PR? A refactor really ought to also include changes to sql_for_insert. However, including changes to that method here would be too broad and make this PR unfocused.

@rafaelfranca I don't think Arel supports Returning nodes yet (implemented in #47161 🙏🏻), also #48241 was merged in 2023 with the same strategy 💡

activerecord/lib/active_record/connection_adapters/sqlite3/database_statements.rb

activerecord/lib/active_record/persistence.rb

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

abaldwin88

Rebased and addressed feedback.

With the latest changes in main I see a pretty clear path to adding this functionality for SQLite and Maria as well. That can be done in a follow-up PR.

abaldwin88 · 2024-11-09T22:14:32Z

activerecord/test/schema/sqlite_specific_schema.rb

  create_table :defaults, force: true do |t|
-      t.integer :random_number, default: -> { "ABS(RANDOM())" }
+      t.virtual :virtual_stored_number, type: :integer, as: "random_number * 10", stored: true if supports_virtual_columns?
+      t.integer :random_number, default: -> { "ABS(RANDOM() % 100)" }


This way it will match the behavior of random_number in the postgresql schema

abaldwin88 · 2024-11-09T22:15:53Z

activerecord/test/schema/sqlite_specific_schema.rb

 ActiveRecord::Schema.define do
  create_table :defaults, force: true do |t|
-      t.integer :random_number, default: -> { "ABS(RANDOM())" }
+      t.virtual :virtual_stored_number, type: :integer, as: "random_number * 10", stored: true if supports_virtual_columns?


Ensures certain code paths / edge cases will be exercised during tests

afn · 2025-04-28T17:24:07Z

Is this PR mergable or is there any outstanding feedback that needs to be addressed? Happy to help with anything to get this moved along.

rails-bot bot added the activerecord label Jul 2, 2023

abaldwin88 mentioned this pull request Aug 26, 2023

Add ActiveRecord.reload_attributes #48434

Open

abaldwin88 changed the title ~~PostgreSQL reload virtual attributes on update~~ PostgreSQL reload virtual attributes on update via RETURNING clause Aug 26, 2023

abaldwin88 force-pushed the virtual_stored_column_returning_on_update branch from 477cf89 to b71ebf1 Compare August 31, 2023 18:30

nvasilevski reviewed Aug 31, 2023

View reviewed changes

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 force-pushed the virtual_stored_column_returning_on_update branch 2 times, most recently from 7549871 to f708a50 Compare September 19, 2023 18:04

MatheusRich added the ready PRs ready to merge label Sep 11, 2024

rafaelfranca reviewed Sep 17, 2024

View reviewed changes

WIP

162afd7

abaldwin88 force-pushed the virtual_stored_column_returning_on_update branch from f708a50 to 162afd7 Compare November 9, 2024 22:11

abaldwin88 commented Nov 9, 2024

View reviewed changes

Conversation

abaldwin88 commented Jul 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Detail

Additional information

Checklist

Uh oh!

ideasasylum commented Jul 23, 2023

Uh oh!

excid3 commented Aug 10, 2023

Uh oh!

abaldwin88 commented Aug 31, 2023

Uh oh!

nvasilevski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abaldwin88 commented Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skcc321 commented Jul 2, 2024

Uh oh!

rafaelfranca left a comment

Choose a reason for hiding this comment

Uh oh!

rafaelfranca Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

abaldwin88 Oct 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abaldwin88 Dec 18, 2024

Choose a reason for hiding this comment

Uh oh!

quentindemetz Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abaldwin88 left a comment

Choose a reason for hiding this comment

Uh oh!

abaldwin88 Nov 9, 2024

Choose a reason for hiding this comment

Uh oh!

abaldwin88 Nov 9, 2024

Choose a reason for hiding this comment

Uh oh!

afn commented Apr 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

abaldwin88 commented Jul 2, 2023 •

edited

Loading

abaldwin88 commented Sep 19, 2023 •

edited

Loading

abaldwin88 Oct 27, 2024 •

edited

Loading