Bug: Alias for an aggregate result is removed about pagy HOT 9 CLOSED

OK, so the :all IS necessary, but it may not work with "custom select statements", or group, order etc., right? That's a quite old AR problem AFAIR.

It also performs two SQL queries

Standard pagination requires knowing the count, and unluckily the count doesn't come with the results in most DBs, but requires an extra query.

If avoiding one query is a priority, Pagy::Countless can help (if your pagination requirements can accommodate its limitations).

I am contented with the workaround in the issue description

If the count is right, it looks like the simplest solution to me!

from pagy.

Pagy::Countless
Countless extra

from pagy.

@AbhayVAshokan Thank you for the report and the solution ;)

Pagy strives to be as agnostic as possible, hence it "shouldn't" know much about ActiveRecord, and it does not by design.

However, since AR is overwhelmingly used in most apps, pagy tries to be as "AR friendly" as possible, as you can notice in the code that it runs on ANY collection in order to get the collection count:

That's hardly a one-size-fits-all solution, but it works in an overwhelmingly number of cases. Indeed you may need to override the method with apps using other ORMs (usually by simply removing the :all argument). In cases where the collection.count(:all) does not work, you can pass your own count to the helper as explained here.

What would be the right ruby statement to get the right count from the pages_and_block_counts in your alias case?

from pagy.

I can consider the possibility to add a :count_args pagy variable, which could be used to pass the arguments to the count. Defaut count_args: [:all]. That would allow things like:

@pagy, @records = pagy(my_collection, count_args: ['age']) 

or set its default to an empty array, for the ORMs that use count with no argument.

Would that be helpful also in your case?

from pagy.

Thank you for the prompt responses. I am assuming that count is used to set the upper limit during pagination. In my case setting count as Page.count gets the work done.

from pagy.

I am a bit rusty on ActiveRecord, and I forgot why we have to add :all to the statement in the past, but there was some problem with not passing anything to count AFAIR.

It's just a matter of picking the right default at this point, since :count_args will definitely be part of the incoming 7.0.

Maybe things changed more recently and we could remove :all?

from pagy.

@benkoshy ?

from pagy.

Passing :all arg to the count method is necessary. The original query can have custom select statements. The COUNT operation will not perform correctly in all these situations.

For instance, in my example, a count without :all will try to perform COUNT(pages.name, COUNT(blocks.id) AS blocks_count) AS "count_pages_name_count_blocks_id_as_blocks_count" and fail miserably.

from pagy.

I took some time to understand the problem better. I think this is how I should write a custom count operation:

- pagy(pages_and_block_counts)
+ count = pages_and_block_counts.unscope(:order).count(:all).size
+ pagy(pages_and_block_counts, count: count)

1. Remove the order query.
2. Perform a count(:all) operation.
3. Since count(:all) returns a hash, perform a size operation.

This will execute two SQL queries

  Page Count (1.1ms)  SELECT COUNT(*) AS "count_all", "pages"."id" AS "pages_id" FROM "pages" LEFT OUTER JOIN "blocks" ON "blocks"."deleted_at" IS NULL AND "blocks"."page_id" = "pages"."id" GROUP BY "pages"."id"
  Page Load (0.8ms)  SELECT pages.name, COUNT(blocks.id) AS blocks_count FROM "pages" LEFT OUTER JOIN "blocks" ON "blocks"."deleted_at" IS NULL AND "blocks"."page_id" = "pages"."id" GROUP BY "pages"."id" /* loading for pp */ ORDER BY "pages"."sequence" ASC, blocks_count DESC LIMIT $1 OFFSET $2  [["LIMIT", 11], ["OFFSET", 0]]

I am contented with the workaround in the issue description. It also performs two SQL queries.

Adding a count_args will not be helpful in my case :(

from pagy.