Skip to content
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

Problem with custom select / PG function #7

Closed
mbajur opened this issue Oct 10, 2014 · 16 comments
Closed

Problem with custom select / PG function #7

mbajur opened this issue Oct 10, 2014 · 16 comments

Comments

@mbajur
Copy link

mbajur commented Oct 10, 2014

Hello!
i'm trying to use search_cop with my quite complex model code. Basically, i have a custom postgres function and i'm using it like that:

@users = User.all.select('CUSTOM_FUNCTION(users.id) AS custom_function')
@users.last.id  #=> 1
@users.last.custom_function  #=> "result"

However, when i apply a search_cop method search() to the code above, i'm not able to access my custom_function result row:

@users = User.all.search('query').select('CUSTOM_FUNCTION(users.id) AS custom_function')
@users.last.id  #=> 1
@users.last.custom_function  #=> nil

What's more interesting - i can see that db query produced by the chain above looks quite proper and i'm able to use it in pgAdminII which gives me a custom_function column with the proper value.

Have you any idea what can be wrong in here or what can i do about it?

Thanks in advance.

@mrkamel
Copy link
Owner

mrkamel commented Oct 10, 2014

#eager_load doesn't seem to work well with #select:

Product.select("products.id as xyz").last.xyz
=> 1

Product.eager_load(:comments).select("products.id as xyz").last.xyz
=> NoMethodError: undefined method `xyz' for ...

I don't consider this to be a bug in SearchCop as the use of eager_load is clearly stated.

However, to fix it you currently have to do the join-logic yourself via a scope block within your search_scope block and avoid eager_load. Check out https://github.com/mrkamel/search_cop#associations

In case you need help with the join logic, just call.

@mbajur
Copy link
Author

mbajur commented Oct 11, 2014

Thank you for reply, mrkamel!
I'm affraid i'm not quite comfortable with defining that by myself. Can you please guide me a bit on how to make that? It should later go to the README as an example cause i think it might be quite common issue.

@mrkamel
Copy link
Owner

mrkamel commented Oct 11, 2014

Ok, please post your current search_scope and association specifications to let me help you. If your model is the one you already mentioned in issue #5, you should be able to use/switch to my proposed final version of option A) from there.

BTW, there already are examples regarding custom scope's in the README. Moreover, this IMO is more an ActiveRecord issue than a SearchCop issue.

@mbajur
Copy link
Author

mbajur commented Oct 12, 2014

Yes, that's exactly what i have mentioned in #5 :) Yes, i'm definatelly not denying that it's AR issue rather than SearchCop, i'm just using SearchCop in my production commercial app and this one thing stops me from making a final deploy before a deadline which has passed on friday. That's why i'm kind of desperate and not able to deal with that on my own.

Therefore, i'm 100 times more thankfull for your will to help me with that thing :)

@mrkamel
Copy link
Owner

mrkamel commented Oct 13, 2014

So, is #5 sufficient to get the thing done? If not, please post your current search_scope block plus your associations (has_one, belongs_to, ...)

@mbajur
Copy link
Author

mbajur commented Oct 14, 2014

Oh, sorry, i've made a horrible mistake. I meaned completely different model and the #5 task is completely not relevant. Here is my code:

class User < ActiveModel::Base
  belongs_to :dealership

  search_scope :search do
    attributes :email, :name, :phone, :role
  end
end

Thanks!

@mrkamel
Copy link
Owner

mrkamel commented Oct 14, 2014

Hm, your search_scope doesn't search within attributes of an associated model at all. Thus, SearchCop won't eager_load anything. Are you sure your snippet is complete? Maybe post the generated sql for User.all.select('CUSTOM_FUNCTION(users.id) AS custom_function') VS User.all.search('query').select('CUSTOM_FUNCTION(users.id) AS custom_function')

@mbajur
Copy link
Author

mbajur commented Oct 14, 2014

Sure, here it is:

Without search() method
# User.with_sums.last.total_ro_value

SELECT USER_JOBS_TOTALS('ro_value' :: text, users.*, '1914-10-14' :: text, '2114-10-14' :: text, 'false' :: boolean)       AS total_ro_value, 
       USER_JOBS_TOTALS('final_ro_value' :: text, users.*, '1914-10-14' :: text, '2114-10-14' :: text, 'false' :: boolean) AS total_final_ro_value, 
       USER_JOBS_TOTALS('upsell_value' :: text, users.*, '1914-10-14' :: text, '2114-10-14' :: text, 'false' :: boolean)   AS total_upsell_value 
FROM   "users" 
WHERE  "users"."role" = 'service_technician' 
ORDER  BY "users"."id" DESC 
LIMIT  1 
With search()
# User.with_sums.search('rodrigo').last.total_ro_value

SELECT   USER_JOBS_TOTALS( 'ro_value'::text, users.*, '1914-10-14'::text, '2114-10-14'::text, 'false'::      boolean ) AS total_ro_value, 
        USER_JOBS_TOTALS( 'final_ro_value'::text, users.*, '1914-10-14'::text, '2114-10-14'::text, 'false'::boolean ) AS total_final_ro_value, 
         USER_JOBS_TOTALS( 'upsell_value'::text, users.*, '1914-10-14'::text, '2114-10-14'::text, 'false'::  boolean ) AS total_upsell_value, 
         "users"."id"                                                                                                  AS t0_r0,
         "users"."email"                                                                                               AS t0_r1,
         "users"."encrypted_password"                                                                                  AS t0_r2,
         "users"."reset_password_token"                                                                                AS t0_r3,
         "users"."reset_password_sent_at"                                                                              AS t0_r4,
         "users"."remember_created_at"                                                                                 AS t0_r5,
         "users"."sign_in_count"                                                                                       AS t0_r6,
         "users"."current_sign_in_at"                                                                                  AS t0_r7,
         "users"."last_sign_in_at"                                                                                     AS t0_r8,
         "users"."current_sign_in_ip"                                                                                  AS t0_r9,
         "users"."last_sign_in_ip"                                                                                     AS t0_r10,
         "users"."confirmation_token"                                                                                  AS t0_r11,
         "users"."confirmed_at"                                                                                        AS t0_r12,
         "users"."confirmation_sent_at"                                                                                AS t0_r13,
         "users"."unconfirmed_email"                                                                                   AS t0_r14,
         "users"."failed_attempts"                                                                                     AS t0_r15,
         "users"."unlock_token"                                                                                        AS t0_r16,
         "users"."locked_at"                                                                                           AS t0_r17,
         "users"."authentication_token"                                                                                AS t0_r18,
         "users"."name"                                                                                                AS t0_r19,
         "users"."phone"                                                                                               AS t0_r20,
         "users"."role"                                                                                                AS t0_r21,
         "users"."dealership_id"                                                                                       AS t0_r22,
         "users"."jobs_count"                                                                                          AS t0_r23,
         "users"."videos_count"                                                                                        AS t0_r24,
         "users"."is_admin"                                                                                            AS t0_r25,
         "users"."created_at"                                                                                          AS t0_r26,
         "users"."updated_at"                                                                                          AS t0_r27,
         "users"."username"                                                                                            AS t0_r28
FROM     "users" 
WHERE    "users"."role" = 'service_technician' 
AND      (( 
                           "users"."email" ilike '%rodrigo%' 
                  OR       "users"."name" ilike '%rodrigo%' 
                  OR       "users"."phone" ilike '%rodrigo%' 
                  OR       "users"."role" ilike '%rodrigo%')) 
ORDER BY "users"."id" DESC limit 1

with_sums() is a method which adds the custom functions to query and it works more or less that (it's kind of long and complex so here is just an example of how it works):

def self.with_stats
  self.select('custom_function(*) AS total_ro_value, custom_function(*) AS total_final_ro_value, ...')
end

@mrkamel
Copy link
Owner

mrkamel commented Oct 14, 2014

I don't fully understand. You're using User.with_sums.search('rodrigo').total_ro_value, but User.with_sums.search('rodrigo') returns an ActiveRecord::Relation Object. Maybe your pseudo-code of with_stats/with_sums is incomplete. However, what about using #first , such that User.with_sums.search('rodrigo').first.total_ro_value ?

Otherwise, please tell me what the rails console tells you about User.with_sums.class VS User.with_sums.search('rodrigo').class

@mbajur
Copy link
Author

mbajur commented Oct 14, 2014

Oh crap, sorry, you're right, there was a typo in the first lines of provided code samples, they should be User.with_sums.last.total_ro_value and User.with_sums.search('rodrigo').last.total_ro_value. Previous comment fixed.

@mrkamel
Copy link
Owner

mrkamel commented Oct 14, 2014

What happens when you run the sql generated for User.with_sums.search('rodrigo').last.total_ro_value from the postgres console? Does it return the desired values for total_ro_value, etc? Maybe paste the postgres result as well.

@mbajur
Copy link
Author

mbajur commented Oct 14, 2014

Yes, running generated query in pgadmin returns desired columns. i'm not
able to submit the results right now cause i'm out of home but they look
as expected

@mrkamel
Copy link
Owner

mrkamel commented Oct 14, 2014

Ok, the problem is SearchCop calls eager_load even when no assocations are referenced, such that eager_load([]) is called. I'll push a patch. The patch should fix your issue as long as you don't reference any associations. If, however, in the future, you reference any associations, you'll have to do the joining yourself via a scope block, as initially stated. Thanks.

@mbajur
Copy link
Author

mbajur commented Oct 14, 2014

Great to hear it's tracked down. Thank you for your help and time mrkamel! I'm looking forward to the patch.

@mrkamel
Copy link
Owner

mrkamel commented Oct 14, 2014

Could you confirm, that this fixes your issue, before i bump a new gem version?
To install it, update your Gemfile:

gem "search_cop", :git => "https://github.com/mrkamel/search_cop.git"
$ bundle update search_cop

@mrkamel mrkamel reopened this Oct 14, 2014
@mbajur
Copy link
Author

mbajur commented Oct 14, 2014

Yes, now it works. Thank you so much!

@mbajur mbajur closed this as completed Oct 14, 2014
mrkamel pushed a commit that referenced this issue Oct 15, 2014
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

No branches or pull requests

2 participants