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

Fix #geo_scope on a model with acts_as_mappable :through (rebased) #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mcmire
Copy link

@mcmire mcmire commented Apr 5, 2012

This branch supercedes #8 and is up to date with master.

Again, I took the liberty of ensuring that the tests pass for all adapters I have available (postgresql, mysql, sqlite).

Unfortunately I don't have a real-world app to test these changes on. Can someone do this for me? These changes could be potentially breaking and I want to make sure all the kinks are ironed out.


Edit: If you are too lazy to read the commit messages, an important change that 1d024ca makes is to modify #geo_scope so that it returns not the usual ActiveRecord::Relation object (which is what a scope object is), but an ActsAsMappable::Relation object. This object is special in that you add a where or order clause and you refer to a "distance" column, it will be replaced with the distance calculation formula (probably the Haversine formula unless you have set it to something else). However, this only works if you add where or order after you say geo_scope. For instance, this will work:

Foo.geo_scope(:origin => ...).where('distance > 5')

but this will not:

scope_from_somewhere = Foo.where('distance > 5')
scope_from_somewhere.geo_scope(:origin => ...)

What I want to know is whether you think this will be a problem. For instance, I could change it so that #geo_scope takes a block:

Foo.geo_scope(:origin => ...) { where('distance > 5') }

This implies that #geo_scope does something special. However, it may be too cumbersome for people.

mcmire added 3 commits April 4, 2012 20:04
Basically, for a model which does this:

  acts_as_mappable :through => :some_column

queries using geo_scope automatically :include :some_column.

In a related vein, calling #geo_scope on a model now returns a special scope
object (ActsAsMappable::Relation instead of ActiveRecord::Relation) such that if
you call another scope method such as `where`, `order`, etc. and attempt to
refer to a "distance" column, that "distance" will automatically be replaced
with the Haversine formula in your query. No more HAVINGs or subqueries!!
@mcmire mcmire mentioned this pull request Apr 5, 2012
@Sabyasachig
Copy link

Hi
I have a query like this.
Event.includes(:user).geo_scope(:origin => address).where("CONCAT(users.name_first,' ', users.name_last) like '%#{search_member}%' AND events.is_published = 1 and events.delete_flag = false").order(" IF(distance IS NULL, '9999999', distance), created_at DESC")
and event model belongs_to :user
But it is not working.

But it is not working.

@mcmire
Copy link
Author

mcmire commented Jul 6, 2012

Okay, in what way does it not work? What's the error that you're getting?

@Sabyasachig
Copy link

actually the back end query is not working.
if I write Event.geo_scope(:origin => @location).includes(:user).where(" user.name_first like '%sab%'")
i am getting the error
activeRecord::StatementInvalid: Mysql::Error: Unknown column 'user.name_first' in 'where clause':
i think i active record association is nor working properly.

@mcmire
Copy link
Author

mcmire commented Jul 9, 2012

Isn't your table named "users", rather than "user"? i.e. it should be

Event.geo_scope(:origin => @location).includes(:user).where("users.name_first like '%sab%'")

@Sabyasachig
Copy link

Yes table name is users by mistake i had type it user . But the problem remains same . If u run the above query then the geo_scope method function will not work.It will run a different query in the back end.

@mcmire
Copy link
Author

mcmire commented Jul 10, 2012

Okay so what is the error that you're getting when you run the first query? Sorry to keep asking questions but I still don't quite know what the issue is here.

@Sabyasachig
Copy link

actually there is no error i am getting but the problem is the actual query running in the back end is different. the geo_scope method is not working there.

@mcmire
Copy link
Author

mcmire commented Jul 12, 2012

Okay, can you give me:

  • the Ruby code that you are writing in order to make the query (the #geo_scope call) - you may have already done this, I want to double check which one you are actually using
  • the generated SQL query that is actually being executed

@Sabyasachig
Copy link

The actual query running in the back end
for this Event.includes(:user).geo_scope(:origin => "kalyani,wb,india").where("users.name_first like '%s%'") is

SELECT events.id AS t0_r0, events.user_id AS t0_r1, events.category_id AS t0_r2, events.title AS t0_r3, events.description AS t0_r4, events.start_date_time AS t0_r5, events.end_date_time AS t0_r6, events.address AS t0_r7, events.lookup_country_id AS t0_r8, events.lookup_state_id AS t0_r9, events.lookup_city_id AS t0_r10, events.lookup_zip_code_id AS t0_r11, events.is_paid AS t0_r12, events.is_active AS t0_r13, events.created_at AS t0_r14, events.updated_at AS t0_r15, events.delete_flag AS t0_r16, events.is_published AS t0_r17, events.photo AS t0_r18, events.lat AS t0_r19, events.long AS t0_r20, events.address_1 AS t0_r21, events.address_2 AS t0_r22, events.additional_information AS t0_r23, events.other_city AS t0_r24, events.other_state AS t0_r25, events.other_country AS t0_r26, events.other_zip AS t0_r27, events.check_location AS t0_r28, events.event_type AS t0_r29, events.is_contact AS t0_r30, events.share_points AS t0_r31, events.number_of_users AS t0_r32, events.is_share AS t0_r33, events.is_complete_share AS t0_r34, events.permalink AS t0_r35, events.event_view_price AS t0_r36, events.event_action_price AS t0_r37, events.event_placement_price AS t0_r38, events.advertisement_share_percentage AS t0_r39, users.id AS t1_r0, users.lookup_user_type_id AS t1_r1, users.name_first AS t1_r2, users.name_last AS t1_r3, users.legal_name AS t1_r4, users.encrypted_password AS t1_r5, users.email AS t1_r6, users.address_1 AS t1_r7, users.address_2 AS t1_r8, users.lookup_city_id AS t1_r9, users.lookup_state_id AS t1_r10, users.lookup_country_id AS t1_r11, users.lookup_zip_code_id AS t1_r12, users.phone_home AS t1_r13, users.phone_business AS t1_r14, users.phone_mobile AS t1_r15, users.facebook AS t1_r16, users.twitter AS t1_r17, users.linkedin AS t1_r18, users.paypal_account AS t1_r19, users.photo AS t1_r20, users.lookup_user_status_id AS t1_r21, users.salt AS t1_r22, users.validation_code AS t1_r23, users.is_active AS t1_r24, users.created_at AS t1_r25, users.updated_at AS t1_r26, users.is_request_for_business AS t1_r27, users.is_apporved_for_business AS t1_r28, users.delete_flag AS t1_r29, users.is_first_login AS t1_r30, users.other_city AS t1_r31, users.other_state AS t1_r32, users.other_country AS t1_r33, users.other_zip AS t1_r34, users.check_location AS t1_r35, users.google_plus AS t1_r36, users.is_post_paid AS t1_r37, users.is_public_email AS t1_r38, users.is_public_address AS t1_r39, users.is_public_phone AS t1_r40, users.is_public_user_status AS t1_r41, users.is_public_social_media AS t1_r42, users.is_public_category_preferences AS t1_r43, users.is_public_point AS t1_r44, users.is_public_event AS t1_r45, users.is_public_connection AS t1_r46, users.rec_code AS t1_r47, users.user_name AS t1_r48, users.agreed_to_terms_and_conditions AS t1_r49 FROM events LEFT OUTER JOIN users ON users.id = events.user_id WHERE (users.name_first like '%s%')

But the query should be .
SELECT events._,
(ACOS(least(1,COS(0.4010023913673075)_COS(1.5436605392515053)_COS(RADIANS(events.lat))_COS(RADIANS(events.long))+
COS(0.4010023913673075)_SIN(1.5436605392515053)_COS(RADIANS(events.lat))_SIN(RADIANS(events.long))+
SIN(0.4010023913673075)_SIN(RADIANS(events.lat))))*3963.19)
AS distance FROM events LEFT OUTER JOIN users on users.id = events.user_id WHERE (users.name_first like '%s%')

Which can be achievable by writing
Event.joins("LEFT OUTER JOIN users on users.id = events.user_id ").geo_scope(:origin => "kalyani,wb,india").where("users.name_first like '%s%'")

But in that case the the effectiveness of "includes" will be lost.

@Sabyasachig
Copy link

And i under stand the basic difference between include and joins but is the any way to write the same query using joins.

@mcmire
Copy link
Author

mcmire commented Jul 16, 2012

Got it. Thanks for that. I'll take a look at why that is not working and get back to you.

@Sabyasachig
Copy link

Thanks Elliot . it will be great if geo_scope will work for both include and joins.

@Sabyasachig
Copy link

Hi Elliot is there any modification in geo_scope. is it working now with include.

@mcmire
Copy link
Author

mcmire commented Aug 1, 2012

No... I didn't make any changes... that's interesting.... and you didn't make any changes either?

@Sabyasachig
Copy link

Yes i have written the code in different way. I am using join instead of include. But it will be really help full if i can use include feature.

@mcmire
Copy link
Author

mcmire commented Aug 3, 2012

Oh I'm sorry I misread, I thought you said it suddenly started working. No, I have yet to fix this. I am a little busy right now with work and whatnot, but I will let you know when I've fixed this.

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.

2 participants