Skip to content

Conversation

@jhawthorn
Copy link
Contributor

These are both private APIs, but _load_from_sql will give us even more standard behaviour (this is what find_by_sql calls) including emitting the instantiation.active_record notification.

This may be faster in some cases since _load_from_sql includes a fast path for when an inheritance column isn't present.

Copy link
Contributor

@zerowidth zerowidth left a comment

Choose a reason for hiding this comment

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

You'd know better than me!

These are both private APIs, but _load_from_sql will give us even more
standard behaviour (this is what find_by_sql calls) including emitting
the instantiation.active_record notification.

This may be faster in some cases since _load_from_sql includes a fast
path for when an inheritance column isn't present.
@jhawthorn jhawthorn changed the title Use _load_from_sql instead of send(:instantiate) Use find_by_sql instead of send(:instantiate) Sep 23, 2021
@jhawthorn
Copy link
Contributor Author

Updated to just call find_by_sql. This is a slight behaviour change in that we'll now print out ex. "User Load" instead of "User Load via GitHub::SQL".

This move us to using a completely standard API 🎉

@jhawthorn jhawthorn changed the title Use find_by_sql instead of send(:instantiate) Use find_by_sql instead of select_all and send(:instantiate) Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants