Skip to content

Conversation

simi
Copy link
Contributor

@simi simi commented Mar 15, 2019

mainly extracted from #35494

If same key is used in data hash and also in defaults, data hash one is used.

I'll update documentation once approved.

@boblail
Copy link
Contributor

boblail commented Mar 16, 2019

LGTM! 🙂

👍I like how you did this without a second loop through all the inserts

@simi simi force-pushed the support_defaults_in_insert_all branch from 0ee01ea to 3a3d15a Compare March 16, 2019 15:56
@simi
Copy link
Contributor Author

simi commented Mar 16, 2019

@boblail 👀 changes addressed.

@simi simi force-pushed the support_defaults_in_insert_all branch from 3a3d15a to 05a8990 Compare March 16, 2019 20:55
@boblail
Copy link
Contributor

boblail commented Mar 21, 2019

🤔 What if we didn't add :defaults at all? What if — instead of

🅰️

Book.insert_all(data, defaults: { created_at: Time.now, updated_at: Time.now })

we used create_with according to your PR, #35638, to do:

🅱️

Book.create_with(created_at: Time.now, updated_at: Time.now).insert_all(data)

The behavior will be slightly different. 🅰️ would allow a record in data to override the merged timestamps; 🅱️ will give precedence to the values given in create_with.

However, I think it accomplishes the same goal, adds fewer arguments to insert_all, and it'd be my personal preference to use create_with over defaults: anyway.

@simi
Copy link
Contributor Author

simi commented Mar 21, 2019

good idea @boblail

@kaspth
Copy link
Contributor

kaspth commented Mar 31, 2019

After thinking about it more, I came to the same conclusion as @boblail that we shouldn't add defaults, but go with where.

E.g. Post.where(created_at: Time.now).insert_all(posts).

Yes, we should also support create_with and it should only be merged in for inserts (and not updates) when using upsert_all.

Thanks!

@kaspth kaspth closed this Mar 31, 2019
@alex-benoit
Copy link

Can the conclusion of the discussion/what is planned for Rails 6 be posted on the open issue here as well 🙏 @kaspth
#35493

@kaspth
Copy link
Contributor

kaspth commented May 5, 2019

Rails 6 has hit its feature freeze, so there’s no further feature development scheduled for 6.0

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.

4 participants