[Arel] Offset + Count != good

Hi Tobias, what database are you using? From the SQL generated, I assume mysql?

yes this is mysql. Frankly i think the right fix is to always clean out any offset if it's a .count query. I found quite a few current_orders.except(:offset).count like queries in the Shopify codebase already.

Is AR setting the offset, or is that coming from your application code?

I tend to agree with you about removing the offset on count(*) queries. I just wonder if anyone is relying on this functionality.

It comes from our application code, we have methods such as
shop.orders.filter_by_params(params) which looks at things like :page,
:order_status etc.

Because it looks at .page it has to add a offset, however we still need to
query the .count from the arel object after the filters have been applied,
hence .except(:offset).count.

I don't think anyone relies on this functionality. Offset + count(*) leads
to undefined behavior in most SQL servers.

• tobi

Hi,

Just wanted to note here that I submitted a patch for #5060 that would change the semantics of #count to apply the :limit and :offset first. If that approach were accepted, it would change the behavior associated with this ticket as well. It would also make #count consistent in behavior with ActiveRecord's other aggregate calculations (like sum). Tobias, it seems that such behavior would probably be what your app was expecting as well when passing in scopes with limits and offsets? Any feedback would be appreciated.

https://rails.lighthouseapp.com/projects/8994/tickets/5060-activere...

Thanks,
-john