Improve peopleLocations endpoint: people_locations method #3217

milaaraujo · 2018-08-08T02:31:00Z

Hey everybody!

We are opening this request to fix the issue #3196. Here we implemented a new version of recentPeople method.

Now we first select the users that have lat/lon and then we join the users table with the revisions table to order them by most recent activities. We also change the name of the method from recentPeople to people_locations.

We would like some feedback, thanks! :)

fixes #3196
@publiclab/reviewers @publiclab/soc @jywarren

plotsbot · 2018-08-08T02:42:26Z

	1 Warning
⚠️	It looks like you merged from master in this pull request. Please rebase to get rid of the merge commits – you may want to rewind the master branch and rebase instead of merging in from master, which can cause problems when accepting new code!

	1 Message
📖	@milaaraujo Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.

Generated by 🚫 Danger

jywarren · 2018-08-08T22:56:04Z

This looks great. I'd love a review from one other person maybe? @publiclab/reviewers

jywarren · 2018-08-08T23:00:41Z

app/services/search_service.rb

+    user_scope = User.where('rusers.status <> 0')\
+                     .joins(:user_tags)\
+                     .where('value LIKE "lat:%"')\
+                     .joins(:revisions)\


This might be a bit heavy if all you're wanting is the timestamp. I wonder if you could select just the timestamp, but you might have to do .select('node_revisions.timestamp', plus every other column (Not sure if you can wildcard * multiple columns...) so then the way to get past that could be to get the absolute minimum number of columns with a select(), collect the nids with nids = query.collect(&:nid) and then run a second query, like:

User.where(nid: nids)

This way the original query is very narrowly restricted, then a second query collects the full User records. This is a bit messy looking but you can see how it might result in a more efficient initial query followed by a narrower second query which gets all the User table bits and pieces you're looking for, without the messy joins. Does this make sense?

To be honest I don't know if it makes sense! 😝

Actually, under rails, the query being executed is: SELECT DISTINCT "rusers".* FROM "rusers" INNER JOIN "user_tags" ON "user_tags"."uid" = "rusers"."id" INNER JOIN "node" ON "node"."uid" = "rusers"."id" INNER JOIN "node_revisions" ON "node_revisions"."nid" = "node"."nid" WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT ?. We are only selecting the data from the user table, which is what we need at the end of the day, and we use the timestamp just to sort the results. Of course, I do not know exactly how mysql runs this query, but it is unlikely that it actually returns the entire secondary table to only then select what the query explicitly asks for...

jywarren · 2018-08-09T14:52:26Z

Hmm, well I guess I meant not the whole table meaning all the rows, but I meant every column, and the revisions table has some bigger columns like the node body content. So I think it would be a relatively big amount of data to return. Maybe this optimization is more than we need, and I'm just being paranoid... @namangupta01 what do you think, you were recently thinking about a query optimization?

…

On Thu, Aug 9, 2018, 1:08 AM Camila Araújo ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/services/search_service.rb <#3217 (comment)>: > sresult = DocList.new - nodes = Node.all.order("changed DESC").limit(srchString).distinct - users = [] - nodes.each do |node| - if node.author.status != 0 - if tagName.blank? - users << node.author.user - else - users << node.author.user if node.author.user.has_tag(tagName) - end - end + user_scope = User.where('rusers.status <> 0')\ + .joins(:user_tags)\ + .where('value LIKE "lat:%"')\ + .joins(:revisions)\ To be honest I don't know if it makes sense! 😝 Actually, under rails, the query being executed is: SELECT DISTINCT "rusers".* FROM "rusers" INNER JOIN "user_tags" ON "user_tags"."uid" = "rusers"."id" INNER JOIN "node" ON "node"."uid" = "rusers"."id" INNER JOIN "node_revisions" ON "node_revisions"."nid" = "node"."nid" WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT ?. We are only selecting the data from the user table, which is what we need at the end of the day, and we use the timestamp just to sort the results. Of course, I do not know exactly how mysql runs this query, but it is unlikely that it actually returns the entire secondary table to only then select what the query explicitly asks for... — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#3217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ6LlqpcIAYfU6Xn_f2YqmLFXsEQWks5uO8OjgaJpZM4VzLMP> .

milaaraujo · 2018-08-09T17:17:56Z

But what I'm saying is that we are not returning every column of the revisions table. We are returning -> SELECT DISTINCT "rusers".*. And I really don't know how mysql runs the joins, but I believe it has its own optimization.

However, @namangupta01 what do you think, you were recently thinking about a query optimization? [2] 😄

jywarren · 2018-08-09T18:55:53Z

I think I'm at the limit of my knowledge of mysql too! I thought that joining essentially adds new columns for each of the joined tables columns. But I'm trying to test this to see what the raw msyql result is, and whether we're getting revision body content in the results... hang on a sec and apologies!

jywarren · 2018-08-09T18:57:18Z

Hmm:

> user_scope = User.where('rusers.status <> 0').joins(:user_tags).where('value LIKE "lat:%"').joins(:revisions).order("node_revisions.timestamp DESC")
# ActiveRecord::ConfigurationError: Can't join 'User' to association named 'revisions'; perhaps you misspelled it?

jywarren · 2018-08-09T18:58:37Z

Oh wait, duh - has_many :revisions, through: :node -- adding this...

jywarren · 2018-08-09T19:00:02Z

I see the query as:

  User Load (17.8ms)  SELECT  `rusers`.* FROM `rusers` INNER JOIN `user_tags` ON `user_tags`.`uid` = `rusers`.`id` INNER JOIN `node` ON `node`.`uid` = `rusers`.`id` INNER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT 11

let me think...

jywarren · 2018-08-09T19:01:24Z

OK, full SQL output (just running the generated query in the command-line mysql) is:

+----+----------+-------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+-------------+--------------------+---------------------+------------------+---------------+------------------+---------------+---------------------+---------------------+-------------------+-------+-----------+-----------------+--------------------+-----------------+------+--------------+--------+
| id | username | email             | crypted_password                                                                                                                 | password_salt                                                                                                                    | persistence_token                                                                                                                | login_count | failed_login_count | last_request_at     | current_login_at | last_login_at | current_login_ip | last_login_ip | created_at          | updated_at          | openid_identifier | role  | reset_key | photo_file_name | photo_content_type | photo_file_size | bio  | token        | status |
+----+----------+-------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+-------------+--------------------+---------------------+------------------+---------------+------------------+---------------+---------------------+---------------------+-------------------+-------+-----------+-----------------+--------------------+-----------------+------+--------------+--------+
|  1 | Bob      | bob@publiclab.org | 4d7b3086073fdc9d7647f598d9e036e0e6e210b680bf6ea402631ae28afc1a92d8c9640923ed9d8d738c341b264ae147aa06bbf4024fc319ab76554fbd3d0b52 | a517e2d46393a2a1ae1fd29cc5153abee965973ed572f72bbeca977d5b81be51fdbb8a115b6d4ace6c6cefa05fecd2f5dbf57669092613cfa734b4b41c66ee82 | 0f4d6908dc76f59302b57fd35b77661a2938d7481727f611086e1ab347669337bb122e32b4b3f905f1547fb29725c10cf7133455cf8f2492888e67e64187b2f0 |           0 |                  0 | 2018-01-26 01:24:55 | NULL             | NULL          | NULL             | NULL          | 2018-01-26 01:24:55 | 2018-01-26 01:24:55 | NULL              | basic | NULL      | NULL            | NULL               | NULL            |      | abcdefg12345 |      1 |
+----+----------+-------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+-------------+--------------------+---------------------+------------------+---------------+------------------+---------------+---------------------+---------------------+-------------------+-------+-----------+-----------------+--------------------+-----------------+------+--------------+--------+
1 row in set (0.00 sec)

jywarren · 2018-08-09T19:06:33Z

Oddly, it seems different when I do this:

User.where('rusers.status <> 0').joins(:user_tags).where('value LIKE "lat:%"').joins(:revisions).order("node_revisions.timestamp DESC").select('rusers.*, node_revisions.timestamp').limit(2)

  User Load (12.4ms)  SELECT  rusers.*, node_revisions.timestamp FROM `rusers` INNER JOIN `user_tags` ON `user_tags`.`uid` = `rusers`.`id` INNER JOIN `node` ON `node`.`uid` = `rusers`.`id` INNER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT 2

That outputs:

+----+----------+-------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+-------------+--------------------+---------------------+------------------+---------------+------------------+---------------+---------------------+---------------------+-------------------+-------+-----------+-----------------+--------------------+-----------------+------+--------------+--------+------------+
| id | username | email             | crypted_password                                                                                                                 | password_salt                                                                                                                    | persistence_token                                                                                                                | login_count | failed_login_count | last_request_at     | current_login_at | last_login_at | current_login_ip | last_login_ip | created_at          | updated_at          | openid_identifier | role  | reset_key | photo_file_name | photo_content_type | photo_file_size | bio  | token        | status | timestamp  |
+----+----------+-------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+-------------+--------------------+---------------------+------------------+---------------+------------------+---------------+---------------------+---------------------+-------------------+-------+-----------+-----------------+--------------------+-----------------+------+--------------+--------+------------+
|  1 | Bob      | bob@publiclab.org | 4d7b3086073fdc9d7647f598d9e036e0e6e210b680bf6ea402631ae28afc1a92d8c9640923ed9d8d738c341b264ae147aa06bbf4024fc319ab76554fbd3d0b52 | a517e2d46393a2a1ae1fd29cc5153abee965973ed572f72bbeca977d5b81be51fdbb8a115b6d4ace6c6cefa05fecd2f5dbf57669092613cfa734b4b41c66ee82 | 0f4d6908dc76f59302b57fd35b77661a2938d7481727f611086e1ab347669337bb122e32b4b3f905f1547fb29725c10cf7133455cf8f2492888e67e64187b2f0 |           0 |                  0 | 2018-01-26 01:24:55 | NULL             | NULL          | NULL             | NULL          | 2018-01-26 01:24:55 | 2018-01-26 01:24:55 | NULL              | basic | NULL      | NULL            | NULL               | NULL            |      | abcdefg12345 |      1 | 1522444233 |
|  1 | Bob      | bob@publiclab.org | 4d7b3086073fdc9d7647f598d9e036e0e6e210b680bf6ea402631ae28afc1a92d8c9640923ed9d8d738c341b264ae147aa06bbf4024fc319ab76554fbd3d0b52 | a517e2d46393a2a1ae1fd29cc5153abee965973ed572f72bbeca977d5b81be51fdbb8a115b6d4ace6c6cefa05fecd2f5dbf57669092613cfa734b4b41c66ee82 | 0f4d6908dc76f59302b57fd35b77661a2938d7481727f611086e1ab347669337bb122e32b4b3f905f1547fb29725c10cf7133455cf8f2492888e67e64187b2f0 |           0 |                  0 | 2018-01-26 01:24:55 | NULL             | NULL          | NULL             | NULL          | 2018-01-26 01:24:55 | 2018-01-26 01:24:55 | NULL              | basic | NULL      | NULL            | NULL               | NULL            |      | abcdefg12345 |      1 | 1522444233 |
+----+----------+-------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------+-------------+--------------------+---------------------+------------------+---------------+------------------+---------------+---------------------+---------------------+-------------------+-------+-----------+-----------------+--------------------+-----------------+------+--------------+--------+------------+

jywarren · 2018-08-09T19:13:34Z

hmm, things get stranger but maybe clearer?...


2.4.1 :004 > User.group('rusers.id').where('rusers.status <> 0').joins(:user_tags).where('value LIKE "
lat:%"').joins(:revisions).order("node_revisions.timestamp DESC").select('rusers.*, node_revisions.timestamp').limit(2)
  User Load (6.6ms)  SELECT  rusers.*, node_revisions.timestamp FROM `rusers` INNER JOIN `user_tags` ON `user_tags`.`uid` = `rusers`.`id` INNER JOIN `node` ON `node`.`uid` = `rusers`.`id` INNER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` WHERE (rusers.status <> 0) AND (value LIKE "lat:%") GROUP BY rusers.id ORDER BY node_revisions.timestamp DESC LIMIT 2                                  
ActiveRecord::StatementInvalid: Mysql2::Error: Expression #25 of SELECT list is not in GROUP BY clause and contains nonaggregated column 'plots.node_revisions.timestamp' which is not functionally dependent on columns in GROUP BY clause; this is incompatible with sql_mode=only_full_group_by: SELECT  rusers.*, node_revisions.timestamp FROM `rusers` INNER JOIN `user_tags` ON `user_tags`.`uid` = `rusers`.`id` INNER JOIN `node` ON `node`.`uid` = `rusers`.`id` INNER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` WHERE (rusers.status <> 0) AND (value LIKE "lat:%") GROUP BY rusers.id ORDER BY node_revisions.timestamp DESC LIMIT 2

jywarren · 2018-08-09T19:17:33Z

Hmm, so I think i'm now stuck. In response to the error in the last one, i think it's saying we need to add node_revisions to our group_by, but that's silly because then it wouldn't group by User and we get duplicate records (which I can confirm from running this locally).

jywarren · 2018-08-09T19:26:23Z

I guess I suspect the original version without the extra select() may not have really been sorting by timestamp. Looking at this attempt to get a distinct response:

2.4.1 :022 > User.where('rusers.status <> 0').joins(:user_tags).where('value LIKE "lat:%"').joins(:revisions).order("node_revisions.timestamp DESC").limit(2).distinct

  User Load (2.3ms)  SELECT  DISTINCT `rusers`.* FROM `rusers` INNER JOIN `user_tags` ON `user_tags`.`uid` = `rusers`.`id` INNER JOIN `node` ON `node`.`uid` = `rusers`.`id` INNER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT 2                                                                    
ActiveRecord::StatementInvalid: Mysql2::Error: Expression #1 of ORDER BY clause is not in SELECT list, references column 'plots.node_revisions.timestamp' which is not in SELECT list; this is incompatible with DISTINCT: SELECT  DISTINCT `rusers`.* FROM `rusers` INNER JOIN `user_tags` ON `user_tags`.`uid` = `rusers`.`id` INNER JOIN `node` ON `node`.`uid` = `rusers`.`id` INNER JOIN `node_revisions` ON `node_revisions`.`nid` = `node`.`nid` WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT 2

jywarren · 2018-08-09T19:27:05Z

This is hard!!! 😭

jywarren · 2018-08-09T19:37:43Z

OK, we're also affected by this: https://dev.mysql.com/doc/refman/5.7/en/group-by-handling.html

I'm going to take a short break here, probs a good idea!

jywarren · 2018-08-09T19:40:38Z

OK, thinking thinking. Although we could potentially solve this, our intent is to return people who are recently active.

Thinking outside the box, what if we simply sorted by user.last_request_at or user.last_login_at ?

stefannibrasil · 2018-08-10T16:17:24Z

I checked here, we are using this version of mariadb:

+-----------------+
| VERSION()       |
+-----------------+
| 10.2.16-MariaDB |
+-----------------+

milaaraujo · 2018-08-10T17:29:01Z

I think the last commit will solve the problem for mysql! 🤞

But Travis is failing with this new weird error:

ERROR: Service 'web' failed to build: The command '/bin/sh -c bundle install --jobs 4' returned a non-zero code: 5

😩

jywarren · 2018-08-10T19:43:46Z

oh exciting... i'll try it out myself too -- let me see about the travis error maybe we can restart it...

…

On Fri, Aug 10, 2018 at 1:29 PM Camila Araújo ***@***.***> wrote: I think the last commit will solve the problem for mysql! 🤞 But Travis is failing with this new weird error: ERROR: Service 'web' failed to build: The command '/bin/sh -c bundle install --jobs 4' returned a non-zero code: 5 😩 — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#3217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ6tsyDD_0XAuONI2eqbYOYGlgZQ4ks5uPcLdgaJpZM4VzLMP> .

jywarren · 2018-08-10T19:44:37Z

Looks like a timeout issue, maybe travis servers lost internet connection for a bit. Restarting!

fatal: unable to access 'https://github.com/publiclab/inline-markdown-editor.git/': Failed to connect to github.com port 443: Connection timed out

milaaraujo · 2018-08-10T22:02:41Z

My last commit didn't work on Travis, so I'm removing the changes.

milaaraujo · 2018-08-11T06:00:51Z

Hey Jeff, now I'm running mysql locally and I could reproduce the error you were getting. That fix I tried early really solved the problem for mysql... But it failed on Travis.

1 - This query works for mariaDB, Sqlite and Travis:

User.where('rusers.status <> 0')\
    .joins(:user_tags)\
    .where('value LIKE "lat:%"')\
    .joins(:revisions)\
    .order("node_revisions.timestamp DESC")\
    .distinct

2 - This one works for mysql:

User.select('rusers.*,node_revisions.timestamp')
    .where('rusers.status <> 0')\
    .joins(:user_tags)\
    .where('value LIKE "lat:%"')\
    .joins(:revisions)\
    .order("node_revisions.timestamp DESC")\
    .distinct

3 - So I'm trying now this logic (last commit). It works for mysql and sqlite, but failed on Travis:

if ActiveRecord::Base.connection.adapter_name == 'Mysql2'
	User.select('rusers.*,node_revisions.timestamp')
        .where('rusers.status <> 0')\
        .joins(:user_tags)\
        .where('value LIKE "lat:%"')\
        .joins(:revisions)\
        .order("node_revisions.timestamp DESC")\
        .distinct
else
    User.where('rusers.status <> 0')\
        .joins(:user_tags)\
        .where('value LIKE "lat:%"')\
        .joins(:revisions)\
        .order("node_revisions.timestamp DESC")\
        .distinct
end

The conclusion today is that it is not possible to make everyone happy! My question now is which database does Travis use and which database is used in production? If a code passes without errors by Travis, does that mean it will run smoothly in production? If this is true, can we leave the version 2 that works on Travis?

milaaraujo · 2018-08-11T22:38:52Z

Hey, @jywarren! More insights here about this... I'm using mysql to run the site locally and a lot of things (not related to our work) are broken, for example when I try to create a new question:

So I don't think we are using mysql in production, right? I think it makes more sense to keep the version that works on Travis, what do you think?

icarito · 2018-08-12T00:30:11Z

Just a note that we are using Mariadb 10.2 in production. Not sure why you are seeing this issue! Thanks for your hard work!

milaaraujo · 2018-08-12T18:13:40Z

Hey @icarito! I was testing locally with mysql, because Jeff pointed out he was seeing some errors in my query using mysql... But then I discovered that other things also break when I test with mysql.

Now I'm focusing on leaving the code error-free testing with MariaDB (which is what's in production). My last commit seems to be working okay!

Now that Jeff is on vacation, could you please review it for me? 😄

jywarren · 2018-08-12T20:13:45Z

Thanks this is really thorough and good, I'm planning on doing a more thorough review later today before finally taking off. I haven't forgotten! 😄

…

On Sun, Aug 12, 2018, 2:14 PM Camila Araújo ***@***.***> wrote: Hey @icarito <https://github.com/icarito>! I was testing locally with mysql, because Jeff pointed out he was seeing some errors in my query using mysql... But then I discovered that other things also break when I test with mysql. Now I'm focusing on leaving the code error-free testing with MariaDB (which is what's in production). My last commit seems to be working okay! Now that Jeff is on vacation, could you please review it for me? 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ-SdO1fU1Buhx2F_fvLLpVVR3y_uks5uQHBUgaJpZM4VzLMP> .

jywarren · 2018-08-13T02:53:44Z

OK, going through this now. First, on the includes() solution, you can leave out the final .collect(&:username) and it'll return the full results. Ah, i see you figured that out.

While includes() does select a lot, it is just more detailed in aliasing all the selected columns, and to my understanding of SQL, the results will include all the same columns. It's a long dense query when Rails writes it all out and makes all the aliases, but the effective response from the database is the same. I may be wrong about this! But that's my best understanding and the resources I've read show that includes() (using a different join type) is not necessarily less efficient than joins().

Does includes() work everywhere? If so let's use it. Otherwise, let's open a new issue to very clearly tell everyone to use mariadb and not mysql and then go ahead with your solution that works in maridab -- i totally agree with your assessment.

In general it's nice to use generic-enough SQL features that it'll work in all similar database systems. But we're already pushing this a bit since we use full-text search, for example. So if includes() doesn't work, I'm OK with formally adopting mariadb and dropping support for mysql.

Thanks for looking through this so carefully! A very evidence-driven decision making process. If you try includes() and it doesn't work, just tell me when you're ready with the mariadb version (is it 3d7c5e4 ?)

Thanks!!!

jywarren · 2018-08-13T02:54:52Z

(also just want to note this error here for future reference; e4a72e5 produces the following):

- [ ] There was a test error at:  Failure: test_search_recent_people_functionality_having_specified_tagName(Minitest::Result) [/usr/local/bundle/gems/mysql2-0.5.2/lib/mysql2/client.rb:131]: ActiveRecord::StatementInvalid: Mysql2::Error: Operand should contain 1 column(s): SELECT 'rusers'.* FROM 'rusers' INNER JOIN 'user_tags' ON 'user_tags'.'uid' = 'rusers'.'id' WHERE (user_tags.value LIKE 'tool:barometer') AND ('rusers'.'id') IN (SELECT DISTINCT rusers.*,node_revisions.timestamp, rusers.id FROM 'rusers' INNER JOIN 'user_tags' ON 'user_tags'.'uid' = 'rusers'.'id' INNER JOIN 'node' ON 'node'.'uid' = 'rusers'.'id' INNER JOIN 'node_revisions' ON 'node_revisions'.'nid' = 'node'.'nid' WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC) LIMIT 100 app/services/search_service.rb:178:in 'people_locations' app/services/execute_search.rb:23:in 'execute' app/services/execute_search.rb:3:in 'by' app/api/srch/search.rb:112:in 'execute' app/api/srch/search.rb:100:in 'block (2 levels) in <class:Search>' <a href='https://github.com/milaaraujo/plots2/tree/improve-peopleLocations-endpoint/test/functional/search_api_test.rb#L214'>test/functional/search_api_test.rb:214</a>:in 'block in <class:SearchApiTest>'

milaaraujo · 2018-08-13T21:31:54Z

Hey @jywarren! I know that the includes() solution definitely works, I tried it before! The problem with it is not all the aliases it creates, the problem is that it selects/returns all the columns of the revision table:

SELECT DISTINCT "rusers"."id" AS t0_r0, "rusers"."username" AS t0_r1, "rusers"."email" AS t0_r2, "rusers"."crypted_password" AS t0_r3, "rusers"."password_salt" AS t0_r4, "rusers"."persistence_token" AS t0_r5, "rusers"."login_count" AS t0_r6, "rusers"."failed_login_count" AS t0_r7, "rusers"."last_request_at" AS t0_r8, "rusers"."current_login_at" AS t0_r9, "rusers"."last_login_at" AS t0_r10, "rusers"."current_login_ip" AS t0_r11, "rusers"."last_login_ip" AS t0_r12, "rusers"."created_at" AS t0_r13, "rusers"."updated_at" AS t0_r14, "rusers"."openid_identifier" AS t0_r15, "rusers"."role" AS t0_r16, "rusers"."reset_key" AS t0_r17, "rusers"."photo_file_name" AS t0_r18, "rusers"."photo_content_type" AS t0_r19, "rusers"."photo_file_size" AS t0_r20, "rusers"."bio" AS t0_r21, "rusers"."token" AS t0_r22, "rusers"."status" AS t0_r23, "rusers"."password_checker" AS t0_r24, **"node_revisions"."vid" AS t1_r0, "node_revisions"."nid" AS t1_r1, "node_revisions"."uid" AS t1_r2, "node_revisions"."title" AS t1_r3, "node_revisions"."body" AS t1_r4, "node_revisions"."teaser" AS t1_r5, "node_revisions"."log" AS t1_r6, "node_revisions"."timestamp" AS t1_r7, "node_revisions"."format" AS t1_r8, "node_revisions"."status" AS t1_r9** FROM "rusers" INNER JOIN "user_tags" ON "user_tags"."uid" = "rusers"."id" LEFT OUTER JOIN "node" ON "node"."uid" = "rusers"."id"
 LEFT OUTER JOIN "node_revisions" ON "node_revisions"."nid" = "node"."nid" WHERE (rusers.status <> 0) AND (value LIKE "lat:%") AND ("rusers"."id") IN (?, ?) ORDER BY node_revisions.timestamp DESC [["id", 6], ["id", 5]]

The joins() solution only selects/returns the user table:

SELECT DISTINCT "rusers".* FROM "rusers" INNER JOIN "user_tags" ON "user_tags"."uid" = "rusers"."id" INNER JOIN "node" ON "node"."uid" = "rusers"."id" INNER JOIN "node_revisions" ON "node_revisions"."nid" = "node"."nid" WHERE (rusers.status <> 0) AND (value LIKE "lat:%") ORDER BY node_revisions.timestamp DESC LIMIT ?

So the effective response from the database is not exactly the same. If you don't think this is a problem, I can certainly send a new commit (using includes())! 😃

milaaraujo · 2018-08-13T21:41:13Z

And I see you are worried about the support for mysql. But I don't think at this point, considering the code we have in production now, we really don't support mysql. I run rake test yesterday in the master branch, using mysql, and it pointed out 43 errors! Did you try to do this before? Or maybe I'm using a wrong mysql version... Sorry to be pushy about it! But I'm just trying to understand what's going on.

jywarren · 2018-08-14T09:22:52Z

We'll, if you want to try select() with includes, we can try narrowing, but otherwise I think the includes() version will be fine. Thanks!

…

On Tue, Aug 14, 2018, 12:41 AM Camila Araújo ***@***.***> wrote: And I see you are worried about the support for mysql. But I don't think at this point, considering the code we have in production now, we really support mysql. I run rake test yesterday in the master branch, using mysql, and it pointed out 43 errors (or something close to this, I can't remember)! Did you try to do this before? Or maybe I'm using a wrong mysql version... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3217 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ6etqcMJ2aJdte30OBTsa7QOWH0Pks5uQfJ5gaJpZM4VzLMP> .

milaaraujo · 2018-08-14T17:36:56Z

@jywarren, done.

jywarren · 2018-08-14T23:26:55Z

Fantastic!

…3217) * version 1 * Version 2 * changing mothod name: recentPeople -> people_locations * Adding distinct * trying to fix mysql error * code climate fixes * removing mysql fixes (it didn't work) * ActiveRecord::Base.connection.adapter_name == 'Mysql2' * Version that works on Travis * using 'includes()'

milaaraujo added 3 commits August 3, 2018 22:15

version 1

cb87809

Version 2

0a46da6

changing mothod name: recentPeople -> people_locations

eef691a

ghost assigned milaaraujo Aug 8, 2018

ghost added the in progress label Aug 8, 2018

milaaraujo assigned stefannibrasil Aug 8, 2018

stefannibrasil mentioned this pull request Aug 8, 2018

PLANNING ISSUE: v2 API development #2755

Closed

milaaraujo requested a review from jywarren August 8, 2018 03:31

milaaraujo added summer-of-code rgsoc Rails Girls Summer of Code labels Aug 8, 2018

jywarren reviewed Aug 8, 2018

View reviewed changes

milaaraujo mentioned this pull request Aug 9, 2018

Community Check-in #12 Summer Of Code Week 13 #3222

Closed

Adding distinct

762a8e3

jywarren mentioned this pull request Aug 9, 2018

Profile api test PR #3224

Closed

ghost added the in progress label Aug 10, 2018

removing mysql fixes (it didn't work)

9aa3ef5

milaaraujo force-pushed the improve-peopleLocations-endpoint branch from 67bd2e5 to 9aa3ef5 Compare August 11, 2018 03:19

ActiveRecord::Base.connection.adapter_name == 'Mysql2'

e4a72e5

Version that works on Travis

3d7c5e4

milaaraujo requested a review from icarito August 12, 2018 18:14

stefannibrasil mentioned this pull request Aug 14, 2018

Community Check-in #13 Summer Of Code Week 14 #3225

Closed

milaaraujo added 2 commits August 14, 2018 10:11

using 'includes()'

4172d4b

Merge branch 'master' into improve-peopleLocations-endpoint

a3848ea

jywarren merged commit be06b33 into publiclab:master Aug 14, 2018

ghost removed the in progress label Aug 14, 2018

milaaraujo deleted the improve-peopleLocations-endpoint branch October 31, 2018 04:19

Improve peopleLocations endpoint: people_locations method #3217

Improve peopleLocations endpoint: people_locations method #3217

Conversation

milaaraujo commented Aug 8, 2018

plotsbot commented Aug 8, 2018 • edited Loading

jywarren commented Aug 8, 2018

jywarren Aug 8, 2018

Choose a reason for hiding this comment

milaaraujo Aug 9, 2018

Choose a reason for hiding this comment

jywarren commented Aug 9, 2018 via email

milaaraujo commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018 • edited Loading

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

jywarren commented Aug 9, 2018

stefannibrasil commented Aug 10, 2018

milaaraujo commented Aug 10, 2018

jywarren commented Aug 10, 2018 via email

jywarren commented Aug 10, 2018

milaaraujo commented Aug 10, 2018

milaaraujo commented Aug 11, 2018 • edited Loading

milaaraujo commented Aug 11, 2018 • edited Loading

icarito commented Aug 12, 2018

milaaraujo commented Aug 12, 2018

jywarren commented Aug 12, 2018 via email

jywarren commented Aug 13, 2018

jywarren commented Aug 13, 2018

milaaraujo commented Aug 13, 2018 • edited Loading

milaaraujo commented Aug 13, 2018 • edited Loading

jywarren commented Aug 14, 2018 via email

milaaraujo commented Aug 14, 2018 • edited Loading

jywarren commented Aug 14, 2018

plotsbot commented Aug 8, 2018 •

edited

Loading

jywarren commented Aug 9, 2018 •

edited

Loading

milaaraujo commented Aug 11, 2018 •

edited

Loading

milaaraujo commented Aug 11, 2018 •

edited

Loading

milaaraujo commented Aug 13, 2018 •

edited

Loading

milaaraujo commented Aug 13, 2018 •

edited

Loading

milaaraujo commented Aug 14, 2018 •

edited

Loading