Skip to content

Preserve DISTINCT and GROUP BY semantics in relation counts and pagination readers #343

Description

@coderabbitai

Summary

Correct ActiveRecord::Relation count semantics for DISTINCT and GROUP BY queries. Add regression coverage for count and the Kaminari pagination readers.

Requested by @eddygarcas as a follow-up to PR #330.

Background

In runtime/ruby/active_record/relation.rb, to_sql preserves DISTINCT, GROUP BY, and HAVING. count_sql omits these operations. As a result, count can report the number of underlying rows rather than the distinct or grouped result set.

The pagination implementation delegates total_count to count. This existing limitation can therefore produce incorrect total_count, total_pages, and page-boundary results. The shared count limitation predates PR #330. Fixing it affects callers beyond pagination, so it is tracked separately.

Required changes

  • Correct count SQL construction to preserve DISTINCT semantics, including the selected projection where applicable.
  • Preserve GROUP BY and HAVING semantics for grouped counts. Keep the existing group_count lowering and its Hash return contract consistent with Rails.
  • Make total_count return the scalar size of the unpaginated distinct or grouped result set. For grouped results, count the surviving groups rather than the underlying rows.
  • Exclude pagination LIMIT and OFFSET from total_count without changing the receiver's query state or loaded-record cache.
  • Keep total_pages and page-boundary readers based on the corrected total_count.
  • Check existing count callers before changing shared behavior. Preserve the intended count and group_count return contracts.
  • Keep framework behavior in the selected runtime/ruby Ruby/RBS units. Check loader target selection and emitter adaptations for affected targets.

Affected areas

  • runtime/ruby/active_record/relation.rb: count, count_sql, group_count, total_count, total_pages, and page-boundary readers.
  • tests/kaminari_pagination.rs: pagination regression coverage.
  • Existing relation-count tests and grouped-count lowering coverage, as identified during implementation.

Acceptance criteria

  • A DISTINCT query with duplicate underlying rows reports the correct count.
  • DISTINCT with an explicit projection has regression coverage.
  • A GROUP BY query with multiple rows per group preserves the grouped count contract.
  • A grouped query with HAVING counts only groups that survive HAVING.
  • For paginated DISTINCT and GROUP BY queries, total_count reports the unpaginated result-set size and total_pages uses that size.
  • Regression tests cover first_page?, last_page?, out_of_range?, next_page, and prev_page for these queries, including empty results and out-of-range pages.
  • Reading pagination metadata leaves LIMIT, OFFSET, query clauses, and loaded records unchanged.
  • Existing ungrouped count and pagination behavior continues to pass.
  • Validate affected emitted targets using the repository's supported test workflow.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions