Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable aggregates and group_by #109

Merged
merged 5 commits into from
Oct 25, 2024
Merged

Conversation

catalyst17
Copy link
Contributor

@catalyst17 catalyst17 commented Oct 18, 2024

TL;DR

Enhanced the query functionality to support multiple grouping fields and aggregations, with improved type handling for ClickHouse data types.

What changed?

  • Modified GroupBy field in QueryParams to accept an array of strings instead of a single string
  • Added GetAggregations method to handle grouped and aggregated queries separately
  • Updated Aggregations field in QueryResponse to use []map[string]interface{} for flexible result handling
  • Implemented comprehensive ClickHouse type mapping to Go types with support for nullable, array, and low cardinality types
  • Separated data retrieval logic in handlers to handle aggregations distinctly from regular queries
  • Added unit tests for ClickHouse type mapping functionality

How to test?

  1. Query logs/transactions endpoints with multiple group_by parameters
  2. Test various aggregation functions (COUNT, SUM, etc.) with grouping
  3. Verify response structure for both regular and aggregated queries
  4. Test handling of different ClickHouse data types and their nullable variants
  5. Run new unit tests for type mapping functionality

Why make this change?

This enhancement provides more robust support for data analysis by properly handling grouped queries and aggregations. The improved type mapping system ensures accurate data representation when working with ClickHouse's various data types, while maintaining clean separation between regular queries and aggregations.

@catalyst17 catalyst17 changed the title feat: enable aggregates and group_by Enable aggregates and group_by Oct 18, 2024
Copy link
Contributor Author

catalyst17 commented Oct 18, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @catalyst17 and the rest of your teammates on Graphite Graphite

@catalyst17 catalyst17 marked this pull request as ready for review October 18, 2024 18:26
@catalyst17 catalyst17 force-pushed the arsenii/aggregates-and-group-by-logs branch 3 times, most recently from d22e74d to 8e4c390 Compare October 23, 2024 14:11
@catalyst17 catalyst17 force-pushed the arsenii/fix-topic-value-format branch from bd55ca1 to a55695d Compare October 24, 2024 13:52
@catalyst17 catalyst17 force-pushed the arsenii/aggregates-and-group-by-logs branch 3 times, most recently from 2922b7c to 8acc2cc Compare October 24, 2024 16:52
@catalyst17 catalyst17 assigned AmineAfia and iuwqyir and unassigned AmineAfia and iuwqyir Oct 24, 2024
@catalyst17 catalyst17 force-pushed the arsenii/aggregates-and-group-by-logs branch from 8acc2cc to 7698103 Compare October 25, 2024 08:18
Copy link
Contributor Author

catalyst17 commented Oct 25, 2024

Merge activity

  • Oct 25, 11:01 AM EDT: A user started a stack merge that includes this pull request via Graphite.
  • Oct 25, 11:04 AM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 25, 11:05 AM EDT: A user merged this pull request with Graphite.

@catalyst17 catalyst17 changed the base branch from arsenii/fix-topic-value-format to graphite-base/109 October 25, 2024 15:01
@catalyst17 catalyst17 changed the base branch from graphite-base/109 to main October 25, 2024 15:02
@catalyst17 catalyst17 force-pushed the arsenii/aggregates-and-group-by-logs branch from 7698103 to 3e4e848 Compare October 25, 2024 15:03
@catalyst17 catalyst17 merged commit 18234d0 into main Oct 25, 2024
5 checks passed
@catalyst17 catalyst17 deleted the arsenii/aggregates-and-group-by-logs branch October 25, 2024 15:05
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.

3 participants