Skip to content

update counts api to include group by clauses#1601

Merged
nikhilsinhaparseable merged 2 commits intoparseablehq:mainfrom
nikhilsinhaparseable:counts-group
Mar 31, 2026
Merged

update counts api to include group by clauses#1601
nikhilsinhaparseable merged 2 commits intoparseablehq:mainfrom
nikhilsinhaparseable:counts-group

Conversation

@nikhilsinhaparseable
Copy link
Copy Markdown
Contributor

@nikhilsinhaparseable nikhilsinhaparseable commented Mar 31, 2026

Summary by CodeRabbit

  • New Features
    • Count queries now support dynamic grouping and ordering by additional user-specified columns.
    • Query results will include those extra grouped columns alongside counts, enabling richer aggregation and ordering in analytics.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aa7ffbe2-88f6-4c5b-b4e6-4af42a1ff237

📥 Commits

Reviewing files that changed from the base of the PR and between bda6275 and d392e22.

📒 Files selected for processing (1)
  • src/query/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/query/mod.rs

Walkthrough

Adds dynamic GROUP BY support to count query SQL generation by deriving quoted identifiers from count_conditions.group_by into a group_clause. The clause is appended to SELECT, GROUP BY, and ORDER BY, allowing additional grouped columns to be emitted, grouped, and ordered in count queries.

Changes

Cohort / File(s) Summary
Query Grouping Enhancement
src/query/mod.rs
Derive group_by quoted column identifiers into a group_clause; append the clause to the SQL SELECT, GROUP BY, and ORDER BY fragments so count queries return and sort by additional grouped columns.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled on SQL late last night,
Found group_clause hiding in moonlight,
Now counts hop out with columns anew,
Grouped and ordered, a jubilant view! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author; the required template sections are completely missing. Add a description following the repository template, including: the issue being fixed, a description of the goal and rationale, and confirmation of testing and documentation.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: updating the counts API to support group by clauses.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/query/mod.rs (1)

696-705: Reduce SQL template duplication between conditional branches.

Only the WHERE fragment differs; duplicating full SELECT/GROUP BY/ORDER BY strings in both branches increases drift risk for future edits.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/query/mod.rs` around lines 696 - 705, The two branches in building
`query` duplicate the full SELECT/GROUP BY/ORDER BY template; extract the common
template and insert an optional WHERE fragment: call
`get_filter_string(conditions)` when `count_conditions.conditions` is Some to
produce `f` and set `where_clause` to `format!(" WHERE {}", f)` (or empty string
when None), then build `query` once using `{date_bin}{group_clause}`,
`"{table_name}"`, `{end_time_col_name}`, and `{start_time_col_name}` while
interpolating `where_clause`; update uses of `count_conditions`,
`get_filter_string`, `date_bin`, `group_clause`, `table_name`,
`end_time_col_name`, and `start_time_col_name` accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/query/mod.rs`:
- Around line 696-705: The two branches in building `query` duplicate the full
SELECT/GROUP BY/ORDER BY template; extract the common template and insert an
optional WHERE fragment: call `get_filter_string(conditions)` when
`count_conditions.conditions` is Some to produce `f` and set `where_clause` to
`format!(" WHERE {}", f)` (or empty string when None), then build `query` once
using `{date_bin}{group_clause}`, `"{table_name}"`, `{end_time_col_name}`, and
`{start_time_col_name}` while interpolating `where_clause`; update uses of
`count_conditions`, `get_filter_string`, `date_bin`, `group_clause`,
`table_name`, `end_time_col_name`, and `start_time_col_name` accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7e7a3d19-47ef-4d30-b041-f7442d977cd3

📥 Commits

Reviewing files that changed from the base of the PR and between 0b83b8a and bda6275.

📒 Files selected for processing (1)
  • src/query/mod.rs

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 31, 2026
@nikhilsinhaparseable nikhilsinhaparseable merged commit 63af80b into parseablehq:main Mar 31, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants