Virtual stored columns are always nil for newly created records

[camiloforero]

Steps to reproduce

Using postgresql:

Generate a new model with one or more columns with default values, and a virtual stored column based on those other columns Create a new record of that model Access the value of the virtual stored column from the newly created ActiveRecord object

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails", branch: "main"
  gem 'pg', '~> 1.4'
end

require "active_record"
require "minitest/autorun"
require "logger"

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection "postgresql:///test"
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.integer :counter1, null: false, default: 0
    t.integer :counter2, null: false, default: 0
    t.virtual :counter_sum, type: :integer, as: "counter1 + counter2", stored: true
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_virtual_column_value_populated
    post = Post.create!

    assert_equal 0, post.counter_sum
  end
end

Expected behavior

The value of the virtual column should be something, in this particular case, 0

Actual behavior

The value of the virtual column is nil. The test fails with the following output

# Running:

D, [2022-08-02T13:05:17.402730 #13548] DEBUG -- :   TRANSACTION (0.1ms)  BEGIN
D, [2022-08-02T13:05:17.403136 #13548] DEBUG -- :   Post Create (0.6ms)  INSERT INTO "posts" DEFAULT VALUES RETURNING "id"
D, [2022-08-02T13:05:17.406144 #13548] DEBUG -- :   TRANSACTION (2.6ms)  COMMIT
F

Failure:
BugTest#test_virtual_column_value_populated [stored_bug.rb:37]:
Expected: 0
  Actual: nil


rails test stored_bug.rb:34



Finished in 0.011047s, 90.5240 runs/s, 90.5240 assertions/s.
1 runs, 1 assertions, 1 failures, 0 errors, 0 skips

System configuration

Rails version: edge Ruby version: 3.1.2 PostgreSQL version: 14.4

[ghiculescu]

Hmm, I couldn't get your tests to run locally. But I can replicate by adding equivalent tests to the framework:

https://github.com/rails/rails/blob/main/activerecord/test/cases/adapters/postgresql/virtual_column_test.rb

diff --git a/activerecord/test/cases/adapters/postgresql/virtual_column_test.rb b/activerecord/test/cases/adapters/postgresql/virtual_column_test.rb
index 1ef9c846d1..fa1332d69a 100644
--- a/activerecord/test/cases/adapters/postgresql/virtual_column_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/virtual_column_test.rb
@@ -21,6 +21,9 @@ def setup
         t.virtual :name_octet_length, type: :integer, as: "OCTET_LENGTH(name)", stored: true
         t.integer :column1
         t.virtual :column2, type: :integer, as: "column1 + 1", stored: true
+        t.integer :counter1, default: 0
+        t.integer :counter2, default: 0
+        t.virtual :counter_sum, type: :integer, as: "counter1 + counter2", stored: true
       end
       VirtualColumn.create(name: "Rails")
     end
@@ -52,6 +55,23 @@ def test_stored_column
       assert_equal 5, VirtualColumn.take.name_length
     end

+    def test_virtual_column_create_with_defaults
+      vc = VirtualColumn.create
+      assert_equal 0, vc.counter_sum # fail; this is nil
+    end
+
+    def test_virtual_column_update_with_defaults
+      vc = VirtualColumn.take
+      vc.update(counter1: 1)
+      assert_equal 1, vc.counter_sum # fail; this is 0
+    end

Then ARCONN=postgresql bundle exec ruby -Itest test/cases/adapters/postgresql/virtual_column_test.rb to run the test.

vc.reload.counter_sum works as expected, so I guess the issue is that the attribute isn't updated during the save.

[ghiculescu]

Looking at https://github.com/rails/rails/blob/3661d0d8a46a2725b3e7bacbc230e4f183fb67dc/activerecord/lib/active_record/persistence.rb#L1117 I wouldn't say this is "expected", but it seems like the only column that gets updated based on a create is id.

[camiloforero]

That's probably it, I assume PostgreSQL updates the virtual column's value on its end, but because it isn't included in the RETURNING clause, ActiveRecord keeps the old value. Or nil when the record is new.

I assume the most efficient fix could be adding all virtual columns to the RETURNING clauses of INSERT and UPDATE statements

[ghiculescu]

I assume the most efficient fix could be adding all virtual columns to the RETURNING clauses of INSERT and UPDATE statements

I think so, would you like to try and make a PR for this?

[camiloforero]

I could try?

I assume I'd have to work on this particular snippet,

https://github.com/rails/rails/blob/3661d0d8a46a2725b3e7bacbc230e4f183fb67dc/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb#L91-L93

I'd need to find a way to get the table's columns based on the table name, and add the virtual ones to the RETURNING clause

And then make sure those values are added to the ActiveRecord object.

And then do something similar for UPDATE statements

Looks complex but not impossible

[matthewd]

I think this overlaps with #39968, which sits near the very top of my shame pile of super-valuable PRs I need to take the time to review and merge.

It's been [more than] a while since I've read through that PR, but from my recollection, I think you're describing a behaviour that I would like to have automatically supported, but which is not directly implemented in that change.

If you're still keen, I suspect that PR might provide a good base for you to build onto, where it allows a model to manually identify extra columns to be included in RETURNING, and you'd be extending that to automatically include some based on the schema.

[rails-bot[bot]]

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 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[simi]

Can we keep this open :pray: @rails-bot?

[rails-bot[bot]]

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 7-0-stable branch or on main, 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[bot]]

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 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

[nvasilevski]

Hey folks, the reproduction script is passing since https://github.com/rails/rails/pull/48241 has been merged. We were not targeting this issue specifically but I'm glad the solution end up being generic enough to cover this corner case. If you can think if any other way of define an "auto-populated" column in postgresql and it doesn't get populated after creation it should be easy to fix by extending the Column#auto_populated? definition: https://github.com/rails/rails/blob/f2a9767c785fadfef483e0863debe9b224a9ed70/activerecord/lib/active_record/connection_adapters/column.rb#L71-L73

⚠️ However, as was mentioned here, most likely record.update will not update value of a virtual column. So perhaps we should create a new issue as a follow-up to this one with a reproduction script that targets the .update scenario so we can think of a solution here.

I would appreciate if someone can double-check the reproduction script against main once again so we can close this issue. I'll wait for some time before closing it but anyway I'll make sure to create a new one to describe the record.update scenario

[abaldwin88]

@nvasilevski A database trigger could cause a column to be auto-populated. e.g. https://stackoverflow.com/a/16105517 But that's probably not a use case worth handling directly in the rails codebase? The Active Record pattern largely discourages triggers.

FWIW I also reran the reproduction script and it passed.

[nvasilevski]

A database trigger could cause a column to be auto-populated.

Good example, I think in this case it would be harder to extend the current implementation as trigger is not a property of a particular column. However, technically an application could patch Model._returning_columns_for_insert method to include more columns to be requested in RETURNING statement for insertion.

_returning_columns_for_insert is private to Rails so applications should avoid patching it but if there is a strong case and need for applications to customize the list it can become public or we could add a way to manually append/override the array