camertron / scuttle-rb

A library for transforming raw SQL statements into ActiveRecord/Arel queries. Ruby wrapper and tests for scuttle-java.
86 stars 2 forks source link

Incorrect handling of count #1

Closed gstamp closed 10 years ago

gstamp commented 10 years ago
SELECT ip, COUNT(ip) FROM signin_attempts
GROUP BY ip ORDER BY COUNT(ip) DESC LIMIT 30

produces

SigninAttempt.select(:ip, :ip.count).order(:ip).reverse_order.group(:ip).limit(30)
camertron commented 10 years ago

Hey @gstamp,

There's a bit of text beneath the editor on scuttle.io that describes that count and other aggregate methods require the column to be fully qualified. Try this:

SELECT ip, COUNT(signin_attempts.ip) FROM signin_attempts
GROUP BY ip ORDER BY COUNT(ip) DESC LIMIT 30

This is a limitation of scuttle-java. If you've got the time to look into fixing the issue, I'd really appreciate a pull request :)

camertron commented 10 years ago

Ah, and now that I read your question again, I realize the second count isn't being properly handled either. I'll look into it, thanks for the bug report.

camertron commented 10 years ago

Hey @gstamp,

I had some spare time yesterday so I updated scuttle.io with a fix for both these issues. Your SQL now produces the following ActiveRecord/Arel code:

SigninAttempt
  .select(:ip, SigninAttempt.arel_table[:ip].count)
  .order(SigninAttempt.arel_table[:ip].count)
  .reverse_order
  .group(:ip)
  .limit(30)
gstamp commented 10 years ago

Nice one. Thanks for sorting that out.

I noticed when you hit the "Make me Beautiful" button it resets the SQL now. Was that intended?

camertron commented 10 years ago

Hmm that doesn't happen for me... can you provide steps so I can reproduce?

gstamp commented 10 years ago

Sorry, I think I made a mistake. On second try it seems to be working fine.

camertron commented 10 years ago

Ok, thanks!