My company's Rails app (using Postgres) has quite a few columns that are managed by database triggers. A RETURNING statement to populate them on insert/update sounds amazing! Right now, we have to reload the record after save, to get the new values. It's tedious and inefficient.

just a gentle fyi - the failing checks should probably be tackled

I just "requested changes" to make Rubocop happy.

Will take a look tomorrow @natematykiewicz :)

@00dav00 this is amazing! My company would appreciate this PR a lot, is there anyway I can help? Is it okay if I try to fix the breaking tests?

Hi @gmaliar , the biggest problem this PR has is that the feature is not officially approved, that's why I haven't continue working on it. Here is the rails forum issue, if you know someone that can get admins attention on this it would be greatly appreciated.

@matthewd suggested in #34237 that they thought some functionality in this area did make sense as part of AR core, perhaps they could provide some feedback or advice on this PR to try to move it along.

Specifically they said:

To my eye, the ideal behaviour would be @tbuehlmann's explicit self.returning_attributes = (for manual additions reflecting things like triggers), plus automatic detection of unparseable default expressions [to always be returned on insert] and computed columns [to always be returned on insert or update].

Loving the idea and the PR. My company is also having issues with gen_random_uuid() from PostgreSQL. You should really follow @jrochkind suggestion. I'd like to help with any you may need 👍

Hi @jorge-precich , I do started this PR based on that issue that @jrochkind mentioned and would definitely love any feedback or advice from @matthewd . I guess that maybe if the rails forrum issue gets more interaction the rails core team may be open to consider the addition.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

I still would really like this to get merged

I am also very interested in this functionality. We have a PR. Is there anything we can do to help? Is there any way to get Rails committer attention?

@natematykiewicz @jrochkind @deepj I will rebase against the last changes in master to prevent it from being closed. I'm not sure what we can do get the attention of a committer. Do you have any suggestions?

Trying to take a quick stab at summarizing this PR:

• 👀 Need a review on this implementation (similar to insert_all)
• 📈 Current implementation could maybe be optimized further
• It seems that test coverage could improve

That said, I think the first point is by far the most important.

We can also add more tests and refactor after that.

Hi @zzak thanks so much for checking this!
Is there something I have to/can do about the Need a review on this implementation part or is this an rails team task? Please let me know :)

@00dav00 For review, it helps to have someone with additional context and in this particular case my best guess is @matthewd -- so we may just have to wait for them on this.

Understood @zzak , thanks for clearing this up. I guess nothing to do but wait for this to reach @matthewd 's radar.

Kasper is doing some PR review this week. I suggested reviewing this PR.

He has some feedback:
https://twitter.com/natematykiewicz/status/1421342172796882944

Thanks @natematykiewicz . The feedback "try closing it and then reopening a new PR doing it in a different way" -- without having actually reviewed the code here, and thus with absolutely no tips on what sort of different way might be good -- seems to me profoundly discouraging and unhelpful. I don't know why anyone would put their time into another PR based on that, I sure wouldn't.

My conclusion is basically just that nobody on Rails core team is interested in this, and there is no way to get a PR reviewed in that case, so it's unlikely to be a good use of time to try an alternate PR.

I wonder if there's any way to get this feature with a relatively light monkey-patch that seems possibly likely to survive future releases? That would seem to be the only hope.

Is there a way to support more than just Postgres in this? A PR that's just Postgres-specific might not gain as much traction as something that's more widely supported.

@natematykiewicz Thanks for checking with Kasper, I think I will just let this be closed by GH bots cause it looks like there is no way of getting this reviewed.

@jrochkind I'm not sure about monkey patching for this case may because it may be to fragile given that the changes sit at the core of active record.

Monkey patching it is a really bad idea. The Composite Primary Keys gem does a similar monkey patch to return your composite primary key columns back, and the gem is very unstable between patch versions of ActiveRecord. Having something like this in Rails would be a huge help to a lot of things. I see 2 things that I'm guessing would help this PR along:

1. It's currently Postgres only
2. It uses the word returning, which is a Postgres term (I think)

I guess a big question is, does MySQL have a similar feature available that this could also support? Basecamp uses MySQL, so making this be a feature for that would probably help this gain traction.

Googling, it's unfortuantely looking to me like MySQL offers no equivalent. :(

This feature is supported by PostgreSQL and SQLite (more limited than in PostgreSQL). MySQL doesn't support it yet.

• https://www.postgresql.org/docs/12/dml-returning.html
• https://www.sqlite.org/lang_returning.html

Here's a PR that just get merged for something supported by Postgres but not MySQL. It may provide a model of how to structure code in Rails for features only supported by some adapters and not others (as well as evidence that such features can be merged sometimes!).

#41487

However, without interest from Rails committers, it may be that this PR isn't going to go anywhere no matter how much time is put into it. :(

Here's another PR for returning on SQLite. #42955

Maybe if they can be reconciled to use similar code, and the attention of any rails committers can be obtained (that's the hard part), there'd be a chance of merge?

And here's yet another merged PR for an unrelated feature that supports only postgres and SQLite, demonstrating that Rails is in some cases willing to merge AR features only supported on postgres and sqlite, not mysql. #40491

Just to chime in on how to get attention: I had a read of the CONTRIBUTING guide. As this PR is clearly labeled as a feature request this section applies:

Do you intend to add a new feature or change an existing one?

• Suggest your change in the rubyonrails-core mailing list and start writing code.

• Do not open an issue on GitHub until you have collected positive feedback about the change. GitHub issues are primarily intended for bug reports and fixes.

So I guess it is best to drop by in their forums and reference this. I do not have the time to do so myself but am grateful if anybody else would take a chance. In my opinion, the discussions on here and the 17 👍 as of today should surely count as plenty of positive feedback as required by the guide.

@davegson this is the rails forum issue

my bad - bummer to see it overlooked there too...

Here's another PR for returning on SQLite. #42955

Maybe if they can be reconciled to use similar code, and the attention of any rails committers can be obtained (that's the hard part), there'd be a chance of merge?

And here's yet another merged PR for an unrelated feature that supports only postgres and SQLite, demonstrating that Rails is in some cases willing to merge AR features only supported on postgres and sqlite, not mysql. #40491

Actually. MariaDB support returning clause too. MariaDB and MySql use same adapter. So I think returing function should be more widely used, not just limited to postgresql. I have also proposed a feature in rails forum: https://discuss.rubyonrails.org/t/feature-proposal-activerecord-update-all-delete-all-support-use-sqls-returning-clause-statements/78729

Hi @OuYangJinTing , I think this has more to do with getting the right person to review and agree on the changes than been able to make the feature broader, as @zzak commented before.

I think, this PR should be also extended to automatically support virtual columns, because currently there is a need to do a reload to get the value.

Would be great too for updates, specially with increment_counter method, with that we can have atomic counter for unique name generation based on a count, for example.

@winnieoursbrun FYI I used this psql code in Rails production for the same reason:

  # foobars_id is a field used in unique name generation, e.g. with concatinated version of foobars
  def generate_number
    self.version_number = 1
    query = "select setval('foobars_id_seq'::regclass, nextval('foobars_id_seq'::regclass), false)"
    doc_num = ActiveRecord::Base.connection.select_values(query)[0]
    self.document.id = doc_num
    self.number = "#{doc_num.to_s.rjust(5, '0')}-#{self.version_number}"
end

My company is using https://github.com/pennylane-hq/active_record-returning_attributes/tree/qdm/rails7 for the moment

It looks like this has finally been handled, separately by separate development, here? #48241

this one can probably be closed, with that one (hooray!) merged?