New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to preload the roles to avoid N+1? #359

Open
gabrielhilal opened this Issue Sep 15, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@gabrielhilal
Contributor

gabrielhilal commented Sep 15, 2015

I'm looking for a way to preload the roles in order to avoid the N+1 issue.

Something like:

users = User.includes(:roles, :users_roles) # run big query
users.first.has_role?(:admin) # do not run query

The main reason is the fact that I'm using active_model_serializers to return some attributes depending on the user's role.

I really appreciate any suggestion ;)

@EppO

This comment has been minimized.

Show comment
Hide comment
@EppO

EppO Oct 15, 2015

Member

You can preload all your roles but using has_role? method will trigger a SQL query you can't avoid.
Use the association .roles on your User instance instead, like:

User.includes(:roles).map { |u| u.roles.each { |r| r.name } }

That will produce 3 queries:

  • the users fetch query
  • the user_roles (join table) fetch based on the result of the first query
  • the roles based on the result of the second query
  User Load (2.0ms)  SELECT "users".* FROM "users"
  HABTM_Roles Load (0.7ms)  SELECT "users_roles".* FROM "users_roles" WHERE "users_roles"."user_id" IN (2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, 300, 301, 302, 303, 304, 305, 306, 307, 308, 309, 310, 311, 312, 313, 314, 315, 316, 317, 318, 319, 320, 321, 322, 323, 324, 325, 326, 327, 328, 329, 330, 331, 332, 333, 334, 335, 336, 337, 338, 339, 340, 341, 342, 343, 344, 345, 346, 347, 348, 349, 350, 351, 352, 353, 354, 355, 356, 357, 358, 359, 360, 361, 362, 363, 364, 365, 366, 367, 368, 369, 370, 371, 372, 373, 374, 375, 376, 377, 378, 379, 380, 381, 382, 383, 384, 385, 386, 387, 388, 389, 390, 391, 392, 393, 394, 395, 396, 397, 398, 399, 400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410, 411, 412, 413, 414, 415, 416, 417, 418, 419, 420, 421, 422, 423, 424, 425, 426, 427, 428, 429, 430, 431, 432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442, 443, 444, 445, 446, 447, 448, 449, 450, 451, 452, 453, 454, 455, 456, 457, 458, 459, 460, 461, 462, 463, 464, 465, 466, 467, 468, 469, 470, 471, 472, 473, 474, 475, 476, 477, 478, 479, 480, 481, 482, 483, 484, 485, 486, 487, 488, 489, 490, 491, 492, 493, 494, 495, 496, 497, 498, 499, 500, 501, 502, 503)
  Role Load (0.3ms)  SELECT "roles".* FROM "roles" WHERE "roles"."id" IN (1, 2)

Depending on the number of users and roles in your DB (in my example I had a ton of users but only 2 roles), that could be a very bad idea though. You'd better use find_in_batches if you process a ton of users.

Member

EppO commented Oct 15, 2015

You can preload all your roles but using has_role? method will trigger a SQL query you can't avoid.
Use the association .roles on your User instance instead, like:

User.includes(:roles).map { |u| u.roles.each { |r| r.name } }

That will produce 3 queries:

  • the users fetch query
  • the user_roles (join table) fetch based on the result of the first query
  • the roles based on the result of the second query
  User Load (2.0ms)  SELECT "users".* FROM "users"
  HABTM_Roles Load (0.7ms)  SELECT "users_roles".* FROM "users_roles" WHERE "users_roles"."user_id" IN (2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88, 89, 90, 91, 92, 93, 94, 95, 96, 97, 98, 99, 100, 101, 102, 103, 104, 105, 106, 107, 108, 109, 110, 111, 112, 113, 114, 115, 116, 117, 118, 119, 120, 121, 122, 123, 124, 125, 126, 127, 128, 129, 130, 131, 132, 133, 134, 135, 136, 137, 138, 139, 140, 141, 142, 143, 144, 145, 146, 147, 148, 149, 150, 151, 152, 153, 154, 155, 156, 157, 158, 159, 160, 161, 162, 163, 164, 165, 166, 167, 168, 169, 170, 171, 172, 173, 174, 175, 176, 177, 178, 179, 180, 181, 182, 183, 184, 185, 186, 187, 188, 189, 190, 191, 192, 193, 194, 195, 196, 197, 198, 199, 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210, 211, 212, 213, 214, 215, 216, 217, 218, 219, 220, 221, 222, 223, 224, 225, 226, 227, 228, 229, 230, 231, 232, 233, 234, 235, 236, 237, 238, 239, 240, 241, 242, 243, 244, 245, 246, 247, 248, 249, 250, 251, 252, 253, 254, 255, 256, 257, 258, 259, 260, 261, 262, 263, 264, 265, 266, 267, 268, 269, 270, 271, 272, 273, 274, 275, 276, 277, 278, 279, 280, 281, 282, 283, 284, 285, 286, 287, 288, 289, 290, 291, 292, 293, 294, 295, 296, 297, 298, 299, 300, 301, 302, 303, 304, 305, 306, 307, 308, 309, 310, 311, 312, 313, 314, 315, 316, 317, 318, 319, 320, 321, 322, 323, 324, 325, 326, 327, 328, 329, 330, 331, 332, 333, 334, 335, 336, 337, 338, 339, 340, 341, 342, 343, 344, 345, 346, 347, 348, 349, 350, 351, 352, 353, 354, 355, 356, 357, 358, 359, 360, 361, 362, 363, 364, 365, 366, 367, 368, 369, 370, 371, 372, 373, 374, 375, 376, 377, 378, 379, 380, 381, 382, 383, 384, 385, 386, 387, 388, 389, 390, 391, 392, 393, 394, 395, 396, 397, 398, 399, 400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410, 411, 412, 413, 414, 415, 416, 417, 418, 419, 420, 421, 422, 423, 424, 425, 426, 427, 428, 429, 430, 431, 432, 433, 434, 435, 436, 437, 438, 439, 440, 441, 442, 443, 444, 445, 446, 447, 448, 449, 450, 451, 452, 453, 454, 455, 456, 457, 458, 459, 460, 461, 462, 463, 464, 465, 466, 467, 468, 469, 470, 471, 472, 473, 474, 475, 476, 477, 478, 479, 480, 481, 482, 483, 484, 485, 486, 487, 488, 489, 490, 491, 492, 493, 494, 495, 496, 497, 498, 499, 500, 501, 502, 503)
  Role Load (0.3ms)  SELECT "roles".* FROM "roles" WHERE "roles"."id" IN (1, 2)

Depending on the number of users and roles in your DB (in my example I had a ton of users but only 2 roles), that could be a very bad idea though. You'd better use find_in_batches if you process a ton of users.

@gabrielhilal

This comment has been minimized.

Show comment
Hide comment
@gabrielhilal

gabrielhilal Oct 21, 2015

Contributor

Many thanks @EppO

Since has_role? always trigger a SQL query (where), I decided to implement a new method that uses ruby enumerable based on the roles association.
In case someone else is facing the same problem, you can add the following to your user model:

  def check_role?(name, instance = nil)
    roles.find do |role|
      role.resource_id == (instance ? instance.id : nil) &&
        role.resource_type == (instance ? instance.class.name : nil) &&
        role.name == name.to_s
    end.present?
  end

In this way, the following won't generate any extra queries:

@user.check_role?(:admin, @instance)
@user.check_role?(:admin, @another_instance)

Also, if you are looping a bunch of users you can do the following to avoid extra queries:

User.includes(:roles).each do |user|
  user.check_role?(:admin, @instance)
  user.check_role?(:admin, @another_instance)
end

I hope it helps someone else ;)

Contributor

gabrielhilal commented Oct 21, 2015

Many thanks @EppO

Since has_role? always trigger a SQL query (where), I decided to implement a new method that uses ruby enumerable based on the roles association.
In case someone else is facing the same problem, you can add the following to your user model:

  def check_role?(name, instance = nil)
    roles.find do |role|
      role.resource_id == (instance ? instance.id : nil) &&
        role.resource_type == (instance ? instance.class.name : nil) &&
        role.name == name.to_s
    end.present?
  end

In this way, the following won't generate any extra queries:

@user.check_role?(:admin, @instance)
@user.check_role?(:admin, @another_instance)

Also, if you are looping a bunch of users you can do the following to avoid extra queries:

User.includes(:roles).each do |user|
  user.check_role?(:admin, @instance)
  user.check_role?(:admin, @another_instance)
end

I hope it helps someone else ;)

@wldcordeiro

This comment has been minimized.

Show comment
Hide comment
@wldcordeiro

wldcordeiro Oct 21, 2015

Member

@gabrielhilal If that's something you think could be helpful a PR is always welcome. 😄

Member

wldcordeiro commented Oct 21, 2015

@gabrielhilal If that's something you think could be helpful a PR is always welcome. 😄

@gabrielhilal

This comment has been minimized.

Show comment
Hide comment
@gabrielhilal

gabrielhilal Oct 30, 2015

Contributor

@wldcordeiro: I would love to help :)
I believe the current method has_role? must always hit the database as it already does.
I think a nice feature would be something like has_preloaded_role? - difficult to find the right name for that.
The idea is to not hit the db, using only ruby enumerable. So, we could preload the roles once and then call the method for all users.
e.g:

users = User.with_role(:admin, @resource).preload(:roles)
users.each do |user|
  user.has_preloaded_role(:member, @resource) # no queries
end

If you guys think it is useful, I would be happy to implement this feature ;)

Contributor

gabrielhilal commented Oct 30, 2015

@wldcordeiro: I would love to help :)
I believe the current method has_role? must always hit the database as it already does.
I think a nice feature would be something like has_preloaded_role? - difficult to find the right name for that.
The idea is to not hit the db, using only ruby enumerable. So, we could preload the roles once and then call the method for all users.
e.g:

users = User.with_role(:admin, @resource).preload(:roles)
users.each do |user|
  user.has_preloaded_role(:member, @resource) # no queries
end

If you guys think it is useful, I would be happy to implement this feature ;)

@wldcordeiro

This comment has been minimized.

Show comment
Hide comment
@wldcordeiro

wldcordeiro Oct 30, 2015

Member

I'd say either a method like has_cached_role? or as an argument to has_role?(:admin, cached=true)

Member

wldcordeiro commented Oct 30, 2015

I'd say either a method like has_cached_role? or as an argument to has_role?(:admin, cached=true)

@cesc1989

This comment has been minimized.

Show comment
Hide comment
@cesc1989

cesc1989 Nov 6, 2015

@gabrielhilal I got some conditionals to hide/show chunks of views depending on the user's role:

<% if (current_user.has_role? :super_admin) || (current_user.has_role? :doctor) %>
  #show view
<% end %>

And because there are like 5+ roles in this app, it(the conditionals group) ends up returning things like this:
second-user-load

Do you think the method you PR-ested( #369 ) would help me improve this situation?

I'd like to optimize it as much as I can because these validations occurs for many users in many different views.

Thanks.

cesc1989 commented Nov 6, 2015

@gabrielhilal I got some conditionals to hide/show chunks of views depending on the user's role:

<% if (current_user.has_role? :super_admin) || (current_user.has_role? :doctor) %>
  #show view
<% end %>

And because there are like 5+ roles in this app, it(the conditionals group) ends up returning things like this:
second-user-load

Do you think the method you PR-ested( #369 ) would help me improve this situation?

I'd like to optimize it as much as I can because these validations occurs for many users in many different views.

Thanks.

@gabrielhilal

This comment has been minimized.

Show comment
Hide comment
@gabrielhilal

gabrielhilal Nov 6, 2015

Contributor

@cesc1989: The has_cached_role? method (just merged into master 😄 ) should avoid these queries... in your case it will generate only one query to load the association current_user.roles.

Contributor

gabrielhilal commented Nov 6, 2015

@cesc1989: The has_cached_role? method (just merged into master 😄 ) should avoid these queries... in your case it will generate only one query to load the association current_user.roles.

@cesc1989

This comment has been minimized.

Show comment
Hide comment
@cesc1989

cesc1989 Nov 12, 2015

Indeed, @gabrielhilal your PR works just great. Thanks a lot!

cesc1989 commented Nov 12, 2015

Indeed, @gabrielhilal your PR works just great. Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment