Added aggregations and GROUP BY#17
Conversation
There was a problem hiding this comment.
Hello @liuzicheng1987, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
In this pull request, Gemini has added support for SQL aggregations and the GROUP BY clause to the sqlgen library. This significantly enhances the query capabilities, allowing users to perform complex analytical queries directly from C++ in a type-safe manner. The changes involve a substantial refactoring of the select_from query building mechanism to accommodate the selection of arbitrary fields (columns, aggregations, literal values) and the introduction of new operators like group_by and to for specifying grouping criteria and result types/containers. Compile-time checks have been added to ensure correct usage of aggregations and GROUP BY clauses. Comprehensive documentation and tests for both PostgreSQL and SQLite backends are included.
Highlights
- SQL Aggregations: Introduced support for common SQL aggregation functions:
count(),count_distinct(col),avg(col),sum(col),max(col), andmin(col). - GROUP BY Clause: Added the
group_by(...)operator to specify columns for grouping results inSELECTqueries. - Flexible SELECT Fields: Refactored
select_fromto allow selecting arbitrary expressions, including columns, aggregations, and literal values, instead of being limited to mapping directly to a struct's fields. - Aliasing Fields: Introduced the
.as<"alias">()member function for columns and aggregations, and theas<"alias">(value)function/| as<"alias">operator for aliasing selected fields. - Result Type Specification (
to<T>): Added theto<T>operator to explicitly specify the desired result type or container (e.g.,std::vector<MyStruct>) for aselect_fromquery, enabling mapping to custom structs that match the selected fields by name. - Compile-Time Checks: Implemented static assertions to validate the correct usage of aggregations and
GROUP BYclauses, ensuring that selected non-aggregated columns are included in theGROUP BYclause when aggregations are present. - Documentation and Tests: Added new documentation pages explaining
group_byand aggregations, and included new tests for both PostgreSQL and SQLite covering basic aggregations, aggregations with nullable columns, andGROUP BY. - Internal Refactoring: Made internal changes to the transpilation logic (
transpilation/) and dynamic query representation (dynamic/) to support the new features, including handling aliased fields and different types of selected expressions. - Memory Management: Modified the result reading logic (
internal/from_str_vec.hpp) to use placement new and manual destructor calls for potentially better memory management when constructing result objects.
Changelog
Click here to see the changelog
- README.md
- Added a new section 'Grouping and Aggregating Data' with a C++ example and generated SQL.
- docs/README.md
- Added a link to the new documentation page for
group_byand aggregations.
- Added a link to the new documentation page for
- docs/group_by_and_aggregations.md
- New documentation file detailing the usage of
sqlgen::group_byand aggregation functions. - Includes examples for basic aggregations and aggregations with
GROUP BY. - Explains how nullable values are handled in aggregations.
- Documents the
.asoperator for aliasing.
- New documentation file detailing the usage of
- include/sqlgen.hpp
- Included new header files for aggregations, aliasing (
as),group_by,select_from, andto.
- Included new header files for aggregations, aliasing (
- include/sqlgen/aggregations.hpp
- New header defining C++ wrappers for SQL aggregation functions (
avg,count,count_distinct,max,min,sum).
- New header defining C++ wrappers for SQL aggregation functions (
- include/sqlgen/as.hpp
- New header defining the
Asstruct andasconstant for aliasing.
- New header defining the
- include/sqlgen/col.hpp
- Added an
as()member function to theColstruct for aliasing columns.
- Added an
- include/sqlgen/dynamic/Aggregation.hpp
- New header defining dynamic representation for SQL aggregation functions.
- include/sqlgen/dynamic/ColumnOrAggregation.hpp
- New header defining a tagged union for dynamic columns or aggregations.
- include/sqlgen/dynamic/GroupBy.hpp
- New header defining dynamic representation for the
GROUP BYclause.
- New header defining dynamic representation for the
- include/sqlgen/dynamic/SelectFrom.hpp
- Modified the dynamic
SelectFromstruct to use a vector ofField(supporting columns, aggregations, values, and aliases) instead of just columns. - Added an optional
group_bymember.
- Modified the dynamic
- include/sqlgen/group_by.hpp
- New header defining the
GroupBystruct andgroup_byfunction. - Includes static assertions for correct clause ordering and usage.
- New header defining the
- include/sqlgen/internal/GetColType.hpp
- New internal helper for getting column types.
- include/sqlgen/internal/call_destructors_where_necessary.hpp
- New internal helper for manual destructor calls.
- include/sqlgen/internal/from_str_vec.hpp
- Included
call_destructors_where_necessary.hpp. - Modified result reading to use placement new and manual destructor calls.
- Included
- include/sqlgen/limit.hpp
- Added support for applying
limittoSelectFromqueries. - Added static assertions for correct clause ordering.
- Added support for applying
- include/sqlgen/order_by.hpp
- Added support for applying
order_bytoSelectFromqueries. - Added static assertions for correct clause ordering.
- Improved static assertion message.
- Added support for applying
- include/sqlgen/read.hpp
- Changed transpilation function for
Readtoread_to_select_from. - Removed
order_by_member fromReadstruct.
- Changed transpilation function for
- include/sqlgen/select_from.hpp
- New header defining the core
SelectFromstruct andselect_fromfunction. - Implements the query building logic with support for fields, where, group by, order by, limit, and mapping to result types via
to<T>.
- New header defining the core
- include/sqlgen/to.hpp
- New header defining the
Tostruct andtoconstant for specifying result types.
- New header defining the
- include/sqlgen/transpilation/As.hpp
- New header defining the transpilation struct for aliasing.
- include/sqlgen/transpilation/aggregations.hpp
- New header defining transpilation structs for aggregation functions.
- include/sqlgen/transpilation/check_aggregations.hpp
- New header implementing compile-time checks for aggregation and group by usage.
- include/sqlgen/transpilation/fields_to_named_tuple_t.hpp
- New internal helper for determining the result named tuple type.
- include/sqlgen/transpilation/group_by_t.hpp
- New header defining the transpilation struct for
GROUP BY. - Includes static assertion for column existence.
- New header defining the transpilation struct for
- include/sqlgen/transpilation/is_nullable.hpp
- Modified
is_nullable_vto handle cvref qualifiers.
- Modified
- include/sqlgen/transpilation/make_field.hpp
- New internal helper for creating dynamic fields from user input.
- Includes static assertions for valid aggregation arguments.
- include/sqlgen/transpilation/make_fields.hpp
- New internal helper for creating a vector of dynamic fields.
- include/sqlgen/transpilation/order_by_t.hpp
- Improved static assertion message.
- include/sqlgen/transpilation/read_to_select_from.hpp
- New internal helper for transpiling
Readoperations to dynamicSelectFrom.
- New internal helper for transpiling
- include/sqlgen/transpilation/remove_as_t.hpp
- New internal helper for removing the
Aswrapper type.
- New internal helper for removing the
- include/sqlgen/transpilation/remove_nullable_t.hpp
- New internal helper for removing nullable wrappers.
- include/sqlgen/transpilation/to_group_by.hpp
- New internal helper for transpiling
GroupByto dynamicGroupBy.
- New internal helper for transpiling
- include/sqlgen/transpilation/to_select_from.hpp
- Modified the main transpilation function for
SelectFromto handle the new field structure andGROUP BY. - Included
check_aggregationsstatic assertion.
- Modified the main transpilation function for
- include/sqlgen/transpilation/to_sql.hpp
- Added support for transpiling the new
SelectFromstruct. - Updated
Readtranspilation to useread_to_select_from.
- Added support for transpiling the new
- include/sqlgen/where.hpp
- Added support for applying
wheretoSelectFromqueries. - Added static assertions for correct clause ordering.
- Added support for applying
- src/sqlgen/postgres/to_sql.cpp
- Implemented SQL generation for dynamic aggregations.
- Implemented SQL generation for the new dynamic
SelectFromstructure, including fields, aliases, andGROUP BY.
- src/sqlgen/sqlite/to_sql.cpp
- Implemented SQL generation for dynamic aggregations.
- Implemented SQL generation for the new dynamic
SelectFromstructure, including fields, aliases, andGROUP BY.
- tests/postgres/test_aggregations.cpp
- New test file for basic PostgreSQL aggregations.
- tests/postgres/test_aggregations_with_nullable.cpp
- New test file for PostgreSQL aggregations with nullable columns.
- tests/postgres/test_delete_from_dry.cpp
- Removed unused include.
- tests/postgres/test_drop_dry.cpp
- Removed unused include.
- tests/postgres/test_group_by.cpp
- New test file for PostgreSQL
GROUP BYwith aggregations.
- New test file for PostgreSQL
- tests/postgres/test_range_select_from.cpp
- New test file for PostgreSQL
select_fromreturning a range.
- New test file for PostgreSQL
- tests/postgres/test_select_from_with_to.cpp
- New test file for PostgreSQL
select_fromusingto<std::vector<T>>.
- New test file for PostgreSQL
- tests/postgres/test_to_select_from2_dry.cpp
- New test file for dry run SQL generation of a complex
select_fromquery.
- New test file for dry run SQL generation of a complex
- tests/postgres/test_to_select_from_dry.cpp
- Removed unused include.
- tests/sqlite/test_aggregations.cpp
- New test file for basic SQLite aggregations.
- tests/sqlite/test_aggregations_with_nullable.cpp
- New test file for SQLite aggregations with nullable columns.
- tests/sqlite/test_group_by.cpp
- New test file for SQLite
GROUP BYwith aggregations.
- New test file for SQLite
- tests/sqlite/test_range_select_from.cpp
- New test file for SQLite
select_fromreturning a range.
- New test file for SQLite
- tests/sqlite/test_range_select_from_with_to.cpp
- New test file for SQLite
select_fromusingto<std::vector<T>>.
- New test file for SQLite
- tests/sqlite/test_to_select_from.cpp
- Updated dry run test to use
read_to_select_from.
- Updated dry run test to use
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces valuable aggregation and GROUP BY functionalities, significantly enhancing the query capabilities of sqlgen. The implementation leverages C++ templates effectively for type safety and provides good compile-time checks. The new documentation is also helpful for users to understand these new features.
Overall, the changes are well-structured. I've identified a couple of areas for potential refinement, particularly concerning memory management in from_str_vec.hpp and a potentially unused function in the SQLite SQL generation.
Summary of Findings
- Memory Management in
from_str_vec.hpp: The use of manual memory management techniques (placement new, manual destructors) infrom_str_vec.hppis complex and warrants careful review to ensure correctness and understand the driving factors. - Potentially Unused Code: The function
column_or_agg_to_strinsrc/sqlgen/sqlite/to_sql.cppappears to be unused in the current diff and might be redundant. - Documentation Example Clarity: In
docs/group_by_and_aggregations.md(line 75), the examplecount_distinct("last_name"_c)in a query already grouped bylast_namewill always yield 1. While technically correct, a different example might be more illustrative forcount_distinctin grouped queries. This is a minor point and was not commented on directly due to review settings. - Static Assert Message Phrasing: The phrasing of the static_assert message in
include/sqlgen/order_by.hpp(lines 24, 46) regardinglimitandorder_byorder is slightly awkward but its logical assertion is correct. This is a minor point and was not commented on directly due to review settings.
Merge Readiness
The pull request is a strong step forward for sqlgen. However, due to the medium-severity comments regarding the memory management changes in from_str_vec.hpp and the potentially unused code in the SQLite SQL generation, I recommend addressing or clarifying these points before merging. I am not authorized to approve pull requests, so please ensure these changes are also reviewed and approved by other maintainers.
No description provided.