4 ms·

I think they could just have tried optimizing the query? My knowledge could be out of date, but I read that the problem with Limit is that it is applied on the

by onli 9y ago

I think they could just have tried optimizing the query? My knowledge could be out of date, but I read that the problem with Limit is that it is applied on the end. Meaning their original query will happily take all available results, sort them and then return the first 9. If normal values for freq are known, a simple WHERE freq > X could have helped. That might also make the join less painful, right? Additionally, I think there might be a logic issue with the query itself, but I can't fully wrap my head around it. Also, activities.user_id = #{id} smells like an SQL injection. But maybe that's just because of the simplification for the blog entry, or id not user chose-able. Still.

andrewfromx 9y ago
hey feel free to checkout the golang code here https://github.com/activityclub/pointspaced https://github.com/activityclub/pointspaced it's way more versatile than sql for our needs. BTW the #{id} we call .to_i on it first so ruby will make that 0 if it's a string with a single quote or something.
- packetized 9y ago
  That sounds like a recipe for disaster, if that's the only thing you're relying on for input validation of #{id}.
  - andrewfromx 9y ago
    what do you mean. "wefwkefjwe'".to_i will goto zero every time. How is sql injection possible if I to_i your string?
    - slantyyz 9y ago
      Wouldn't it be better to not even run the query if id is a non-integer value, or does id = 0 have some intrinsic meaning in your data?
- matt4077 9y ago
  I've had only a cursory look, but it really feels as if you're reinventing database indices. Indices are based on the same logic, i. e. "99% are reads, let's try to do more work on writes". Redis is great. But to be honest: so is MySQL. If you give MySQL enough RAM and create the right indices, you can easily get to 6-digit numbers of queries per second.
  - andrewfromx 9y ago
    https://news.ycombinator.com/item?id=14500889 https://news.ycombinator.com/item?id=14500889
smnc 9y ago
> I read that the problem with Limit is that it is applied on the end. Meaning their original query will happily take all available results, sort them and then return the first 9. If normal values for freq are known, a simple WHERE freq > X could have helped. Well you can't get around that considering that `freq` is the result of aggregation. Therefore you do have to "take all available results". Or you could keep a tally for all users and refresh it periodically.
slantyyz 9y ago
I agree - that query looks like it would benefit from optimization. At first glance, it seems to me like the query would be much faster if joined activity_types and activities were filtered subqueries instead. Also, activities.user_id = #{id} might be better handled with an SQL CASE statement as opposed to using the .to_i conversion in Ruby.
- matt4077 9y ago
  I don't quite get your idea about the case statement? This is a straight-up condition, and the database only ever gets something like id=12312. In fact there isn't even a to_i involved–ruby will do a .to_s, on something that is guaranteed to be an integer. I have similar feelings about subqueries. This is a rather elementary standard join query, and I always felt subqueries were only tacked onto SQL because people had trouble understanding joins.
  - slantyyz 9y ago
    >> I don't quite get your idea about the case statement? The case statement would only be there to set it to a numeric 0 when it's an unusual value like an apostrophe - which is what the OP said it did in a comment somewhere else in this discussion (https://news.ycombinator.com/item?id=14499589 https://news.ycombinator.com/item?id=14499589). >> I have similar feelings about subqueries. This is a rather elementary standard join query, and I always felt subqueries were only tacked onto SQL because people had trouble understanding joins. Not exactly sure what you meant by similar feelings (did you mean you also don't get the idea about it?). In the absence of knowing MySQL's execution plan for the OP's query, I tend to think using subqueries that reduce the rows of activity_types and activities would make the join, aggregation and ordering faster (assuming the tables are already properly indexed). I've seen this reduce query times on other database engines for similar types of queries.

by onli 9y ago

andrewfromx 9y ago

hey feel free to checkout the golang code here https://github.com/activityclub/pointspaced https://github.com/activityclub/pointspaced it's way more versatile than sql for our needs. BTW the #{id} we call .to_i on it first so ruby will make that 0 if it's a string with a single quote or something.

packetized 9y ago
That sounds like a recipe for disaster, if that's the only thing you're relying on for input validation of #{id}.
- andrewfromx 9y ago
  what do you mean. "wefwkefjwe'".to_i will goto zero every time. How is sql injection possible if I to_i your string?
  - slantyyz 9y ago
    Wouldn't it be better to not even run the query if id is a non-integer value, or does id = 0 have some intrinsic meaning in your data?
matt4077 9y ago
I've had only a cursory look, but it really feels as if you're reinventing database indices. Indices are based on the same logic, i. e. "99% are reads, let's try to do more work on writes". Redis is great. But to be honest: so is MySQL. If you give MySQL enough RAM and create the right indices, you can easily get to 6-digit numbers of queries per second.
- andrewfromx 9y ago
  https://news.ycombinator.com/item?id=14500889 https://news.ycombinator.com/item?id=14500889

smnc 9y ago

> I read that the problem with Limit is that it is applied on the end. Meaning their original query will happily take all available results, sort them and then return the first 9. If normal values for freq are known, a simple WHERE freq > X could have helped. Well you can't get around that considering that `freq` is the result of aggregation. Therefore you do have to "take all available results". Or you could keep a tally for all users and refresh it periodically.

slantyyz 9y ago

I agree - that query looks like it would benefit from optimization. At first glance, it seems to me like the query would be much faster if joined activity_types and activities were filtered subqueries instead. Also, activities.user_id = #{id} might be better handled with an SQL CASE statement as opposed to using the .to_i conversion in Ruby.

matt4077 9y ago
I don't quite get your idea about the case statement? This is a straight-up condition, and the database only ever gets something like id=12312. In fact there isn't even a to_i involved–ruby will do a .to_s, on something that is guaranteed to be an integer. I have similar feelings about subqueries. This is a rather elementary standard join query, and I always felt subqueries were only tacked onto SQL because people had trouble understanding joins.
- slantyyz 9y ago
  >> I don't quite get your idea about the case statement? The case statement would only be there to set it to a numeric 0 when it's an unusual value like an apostrophe - which is what the OP said it did in a comment somewhere else in this discussion (https://news.ycombinator.com/item?id=14499589 https://news.ycombinator.com/item?id=14499589). >> I have similar feelings about subqueries. This is a rather elementary standard join query, and I always felt subqueries were only tacked onto SQL because people had trouble understanding joins. Not exactly sure what you meant by similar feelings (did you mean you also don't get the idea about it?). In the absence of knowing MySQL's execution plan for the OP's query, I tend to think using subqueries that reduce the rows of activity_types and activities would make the join, aggregation and ordering faster (assuming the tables are already properly indexed). I've seen this reduce query times on other database engines for similar types of queries.