From 8bce7ec137db79ab1631f4504ae891b5bd42c1d8 Mon Sep 17 00:00:00 2001 From: Robin van der Noord Date: Tue, 20 Jan 2026 14:56:10 +0100 Subject: [PATCH 1/4] feat: add groupby and having methods to QueryBuilder Add .groupby() and .having() methods for SQL aggregate queries with group filtering. Methods available on both QueryBuilder and TypedTable classes, enabling chaining like Table.groupby(field).having(condition). --- docs/3_building_queries.md | 31 ++++++++ src/typedal/query_builder.py | 27 +++++++ src/typedal/tables.py | 12 ++++ src/typedal/types.py | 4 ++ tests/test_query_builder.py | 132 +++++++++++++++++++++++++++++++++-- 5 files changed, 202 insertions(+), 4 deletions(-) diff --git a/docs/3_building_queries.md b/docs/3_building_queries.md index 3fa98c2..0ab0195 100644 --- a/docs/3_building_queries.md +++ b/docs/3_building_queries.md @@ -22,6 +22,8 @@ A Query Builder can be initialized by calling one of these methods on a TypedTab - where - select - join +- groupby +- having - cache e.g. `Person.where(...)` -> `QueryBuilder[Person]` @@ -93,6 +95,35 @@ This can be overwritten with the `method` keyword argument (left or inner) Person.join('articles', method='inner') # will only yield persons that have related articles ``` +### groupby & having + +Group query results by one or more fields, typically used with aggregate functions like `count()`, `sum()`, `avg()`, etc. +Use `having` to filter the grouped results based on aggregate conditions. + +```python +# Basic grouping: count articles per author +Article.select(Article.author, Article.id.count().with_alias("article_count")) + .groupby(Article.author) + .collect() + +# Group by multiple fields +Sale.select(Sale.product, Sale.region, Sale.amount.sum().with_alias("total")) + .groupby(Sale.product, Sale.region) + .collect() + +# Filter groups with having: only authors with more than 5 articles +Article.select(Article.author, Article.id.count().with_alias("article_count")) + .groupby(Article.author) + .having(Article.id.count() > 5) + .collect() + +# Can be chained in any order +School.groupby(School.id) + .having(Team.id.count() > 0) + .select(School.id, Team.id.count()) + .collect() +``` + ### cache ```python diff --git a/src/typedal/query_builder.py b/src/typedal/query_builder.py index 3c26893..6765dc3 100644 --- a/src/typedal/query_builder.py +++ b/src/typedal/query_builder.py @@ -21,6 +21,8 @@ Condition, Expression, Field, + GroupBy, + Having, Metadata, OnQuery, OrderBy, @@ -165,6 +167,31 @@ def orderby(self, *fields: OrderBy) -> "QueryBuilder[T_MetaInstance]": """ return self.select(orderby=fields) + def groupby(self, *fields: t.Any) -> "QueryBuilder[T_MetaInstance]": + """ + Group the query results by specified fields. + + Args: + fields: Field(s) to group by (e.g., Table.column) + + Returns: + QueryBuilder: A new QueryBuilder instance with grouping applied. + """ + groupby_value = fields[0] if len(fields) == 1 else fields + return self.select(groupby=groupby_value) + + def having(self, condition: t.Any) -> "QueryBuilder[T_MetaInstance]": + """ + Filter grouped query results based on aggregate conditions. + + Args: + condition: Query condition for filtering groups (e.g., Team.id.count() > 0) + + Returns: + QueryBuilder: A new QueryBuilder instance with having condition applied. + """ + return self.select(having=condition) + def where( self, *queries_or_lambdas: Query | t.Callable[[t.Type[T_MetaInstance]], Query] | dict[str, t.Any], diff --git a/src/typedal/tables.py b/src/typedal/tables.py index 2ad6699..a6465f8 100644 --- a/src/typedal/tables.py +++ b/src/typedal/tables.py @@ -326,6 +326,18 @@ def orderby(self: t.Type[T_MetaInstance], *fields: OrderBy) -> "QueryBuilder[T_M """ return QueryBuilder(self).orderby(*fields) + def groupby(self: t.Type[T_MetaInstance], *fields: t.Any) -> "QueryBuilder[T_MetaInstance]": + """ + See QueryBuilder.groupby! + """ + return QueryBuilder(self).groupby(*fields) + + def having(self: t.Type[T_MetaInstance], condition: t.Any) -> "QueryBuilder[T_MetaInstance]": + """ + See QueryBuilder.having! + """ + return QueryBuilder(self).having(condition) + def cache(self: t.Type[T_MetaInstance], *deps: t.Any, **kwargs: t.Any) -> "QueryBuilder[T_MetaInstance]": """ See QueryBuilder.cache! diff --git a/src/typedal/types.py b/src/typedal/types.py index b9810b9..3f0f164 100644 --- a/src/typedal/types.py +++ b/src/typedal/types.py @@ -217,6 +217,8 @@ class SelectKwargs(t.TypedDict, total=False): join: t.Optional[list[Expression]] left: t.Optional[list[Expression]] orderby: "OrderBy | t.Iterable[OrderBy] | None" + groupby: "GroupBy | t.Iterable[GroupBy] | None" + having: "Having | None" limitby: t.Optional[tuple[int, int]] distinct: bool | Field | Expression orderby_on_limitby: bool @@ -319,5 +321,7 @@ class FieldSettings(t.TypedDict, total=False): CacheTuple = tuple[CacheModel, int] OrderBy: t.TypeAlias = str | Expression +GroupBy: t.TypeAlias = Field | Expression +Having: t.TypeAlias = Query | Expression T_annotation = t.Type[t.Any] | types.UnionType diff --git a/tests/test_query_builder.py b/tests/test_query_builder.py index 9dbbd40..bf3ac14 100644 --- a/tests/test_query_builder.py +++ b/tests/test_query_builder.py @@ -1,11 +1,7 @@ -import inspect -import typing - import pytest from pydal.objects import Query from src.typedal import TypeDAL, TypedField, TypedTable, relationship -from typedal.types import CacheFn, CacheModel, CacheTuple, Rows db = TypeDAL("sqlite:memory") @@ -532,3 +528,131 @@ class HTTP(BaseException): ... with pytest.raises(HTTP): TestRelationship.where(TestRelationship.id == 3245892384).first_or_fail(HTTP(404)) + + +def test_groupby_basic(): + """Test basic groupby with count aggregation.""" + _setup_data() + + result = TestRelationship.select( + TestRelationship.querytable.with_alias("query_table"), + TestRelationship.querytable.count().with_alias("count"), + ).groupby(TestRelationship.querytable).execute() + + assert len(result) == 2 + for row in result: + assert row["count"] == 4 + + +def test_groupby_multiple_fields(): + """Test grouping by multiple fields.""" + _setup_data() + + result = TestRelationship.select( + TestRelationship.querytable, + TestRelationship.value, + TestRelationship.id.count().with_alias("count"), + ).groupby(TestRelationship.querytable, TestRelationship.value).execute() + + # Should group by combination of querytable and value + assert len(result) > 0 + + +def test_groupby_with_having(): + """Test groupby with having to filter groups.""" + _setup_data() + + result = TestRelationship.select( + TestRelationship.querytable.with_alias("query_table"), + TestRelationship.querytable.count().with_alias("count"), + ).groupby(TestRelationship.querytable).having(TestRelationship.querytable.count() > 3).execute() + + # Only groups with count > 3 + assert len(result) == 2 + for row in result: + assert row["count"] > 3 + + +def test_having_filters_aggregates(): + """Test that having properly filters based on aggregate conditions.""" + _setup_data() + + # Get all groups + all_groups = TestRelationship.select( + TestRelationship.querytable, + TestRelationship.querytable.count().with_alias("count"), + ).groupby(TestRelationship.querytable).execute() + + # Filter with having + filtered = TestRelationship.select( + TestRelationship.querytable, + TestRelationship.querytable.count().with_alias("count"), + ).groupby(TestRelationship.querytable).having(TestRelationship.querytable.count() > 10).execute() + + # Should have fewer results (or zero if no groups have count > 10) + assert len(filtered) <= len(all_groups) + + +def test_groupby_to_sql(): + """Verify SQL generation includes GROUP BY.""" + sql = TestRelationship.select( + TestRelationship.querytable, TestRelationship.querytable.count() + ).groupby(TestRelationship.querytable).to_sql() + + assert "GROUP BY" in sql + + +def test_having_to_sql(): + """Verify SQL generation includes HAVING.""" + sql = ( + TestRelationship.select(TestRelationship.querytable, TestRelationship.querytable.count()) + .groupby(TestRelationship.querytable) + .having(TestRelationship.querytable.count() > 0) + .to_sql() + ) + + assert "GROUP BY" in sql + assert "HAVING" in sql + + +def test_groupby_chaining(): + """Test that multiple groupby calls work (last one should win).""" + _setup_data() + + # First groupby by querytable + builder1 = TestRelationship.select( + TestRelationship.querytable, TestRelationship.querytable.count().with_alias("count") + ).groupby(TestRelationship.querytable) + + # Then groupby by value (should override) + builder2 = builder1.groupby(TestRelationship.value) + + sql = builder2.to_sql() + # Should only have the second groupby + assert "GROUP BY" in sql + + +def test_groupby_having_on_table_class(): + """Test calling .groupby() and .having() directly on table class in different orders.""" + _setup_data() + + builder1 = ( + TestRelationship.groupby(TestRelationship.querytable) + .having(TestRelationship.querytable.count() > 0) + .select(TestRelationship.querytable, TestRelationship.querytable.count()) + ) + + sql1 = builder1.to_sql() + + builder2 = ( + TestRelationship.having(TestRelationship.querytable.count() > 0) + .groupby(TestRelationship.querytable) + .select(TestRelationship.querytable, TestRelationship.querytable.count()) + ) + sql2 = builder2.to_sql() + + assert sql1 == sql2 + assert "GROUP BY" in sql1 + assert "HAVING" in sql1 + + assert builder1.execute() == builder2.execute() From edcfa72ef5185d85389063eb7f06197b7e842285 Mon Sep 17 00:00:00 2001 From: Robin van der Noord Date: Tue, 20 Jan 2026 15:01:54 +0100 Subject: [PATCH 2/4] refactor: remove unused parent_id parameter from _process_nested_relationships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parent_id parameter became unused after commit b093454 fixed the nested join bug by passing instance.id directly instead of propagating the parent_id from the caller. This commit removes the unused parameter and its documentation. 💘 Generated with Crush Assisted-by: Anthropic: Claude Sonnet 4.5 via Crush --- src/typedal/query_builder.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/typedal/query_builder.py b/src/typedal/query_builder.py index 6765dc3..503c94a 100644 --- a/src/typedal/query_builder.py +++ b/src/typedal/query_builder.py @@ -21,8 +21,6 @@ Condition, Expression, Field, - GroupBy, - Having, Metadata, OnQuery, OrderBy, @@ -869,7 +867,6 @@ def _process_relationship_data( row=row, relation=relation, instance=instance, - parent_id=parent_id, seen_relations=seen_relations, db=db, path=current_path, @@ -891,7 +888,6 @@ def _process_nested_relationships( row: t.Any, relation: Relationship[t.Any], instance: t.Any, - parent_id: t.Any, seen_relations: dict[str, set[str]], db: t.Any, path: str, @@ -903,7 +899,6 @@ def _process_nested_relationships( row: The database row containing relationship data relation: The parent Relationship object containing nested relationships instance: The instance to attach nested data to - parent_id: ID of the root parent for tracking seen_relations: Dict tracking which relationships we've already processed db: Database instance path: Current relationship path From 98f1061bc26e84b3e4798b2a304a4fc9ff6e246d Mon Sep 17 00:00:00 2001 From: Robin van der Noord Date: Tue, 20 Jan 2026 15:10:03 +0100 Subject: [PATCH 3/4] chore: fix mypy type checking errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove unused type: ignore comments from type wrapper classes - Replace redundant t.cast() with direct type annotations - Use annotated assignments for Query and Metadata types 💘 Generated with Crush Assisted-by: Anthropic: Claude Sonnet 4.5 via Crush --- src/typedal/caching.py | 2 +- src/typedal/core.py | 2 +- src/typedal/define.py | 2 +- src/typedal/for_py4web.py | 2 +- src/typedal/query_builder.py | 8 ++++---- src/typedal/rows.py | 2 +- src/typedal/types.py | 18 +++++++++--------- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/typedal/caching.py b/src/typedal/caching.py index 4add6e0..83d17da 100644 --- a/src/typedal/caching.py +++ b/src/typedal/caching.py @@ -260,7 +260,7 @@ def _load_from_cache(key: str, db: "TypeDAL") -> t.Any | None: inst.db = db inst.model = db._class_map[inst.model] - inst.model._setup_instance_methods(inst.model) # type: ignore + inst.model._setup_instance_methods(inst.model) return inst diff --git a/src/typedal/core.py b/src/typedal/core.py index 6f67653..b44e18c 100644 --- a/src/typedal/core.py +++ b/src/typedal/core.py @@ -138,7 +138,7 @@ def resolve_annotation(ftype: str) -> type: # pragma: no cover return resolve_annotation_314(ftype) -class TypeDAL(pydal.DAL): # type: ignore +class TypeDAL(pydal.DAL): """ Drop-in replacement for pyDAL with layer to convert class-based table definitions to classical pydal define_tables. """ diff --git a/src/typedal/define.py b/src/typedal/define.py index 42f0f54..ba059de 100644 --- a/src/typedal/define.py +++ b/src/typedal/define.py @@ -157,7 +157,7 @@ def annotation_to_pydal_fieldtype( elif origin_is_subclass(ftype, TypedField): # TypedField[int] return self.annotation_to_pydal_fieldtype(t.get_args(ftype)[0], mut_kw) - elif isinstance(ftype, types.GenericAlias) and t.get_origin(ftype) in (list, TypedField): # type: ignore + elif isinstance(ftype, types.GenericAlias) and t.get_origin(ftype) in (list, TypedField): # list[str] -> str -> string -> list:string _child_type = t.get_args(ftype)[0] _child_type = self.annotation_to_pydal_fieldtype(_child_type, mut_kw) diff --git a/src/typedal/for_py4web.py b/src/typedal/for_py4web.py index 2456df1..446b072 100644 --- a/src/typedal/for_py4web.py +++ b/src/typedal/for_py4web.py @@ -14,7 +14,7 @@ from .web2py_py4web_shared import AuthUser -class Fixture(_Fixture): # type: ignore +class Fixture(_Fixture): """ Make mypy happy. """ diff --git a/src/typedal/query_builder.py b/src/typedal/query_builder.py index 503c94a..03be6f3 100644 --- a/src/typedal/query_builder.py +++ b/src/typedal/query_builder.py @@ -61,7 +61,7 @@ def __init__( """ self.model = model table = model._ensure_table_defined() - default_query = t.cast(Query, table.id > 0) + default_query: Query = table.id > 0 self.query = add_query or default_query self.select_args = select_args or [] self.select_kwargs = select_kwargs or {} @@ -93,7 +93,7 @@ def __bool__(self) -> bool: Querybuilder is truthy if it has t.Any conditions. """ table = self.model._ensure_table_defined() - default_query = t.cast(Query, table.id > 0) + default_query: Query = table.id > 0 return any( [ self.query != default_query, @@ -490,7 +490,7 @@ def execute(self, add_id: bool = False) -> Rows: Raw version of .collect which only executes the SQL, without performing t.Any magic afterwards. """ db = self._get_db() - metadata = t.cast(Metadata, self.metadata.copy()) + metadata: Metadata = self.metadata.copy() query, select_args, select_kwargs = self._before_query(metadata, add_id=add_id) @@ -509,7 +509,7 @@ def collect( _to = TypedRows db = self._get_db() - metadata = t.cast(Metadata, self.metadata.copy()) + metadata: Metadata = self.metadata.copy() if metadata.get("cache", {}).get("enabled") and (result := self._collect_cached(metadata)): return result diff --git a/src/typedal/rows.py b/src/typedal/rows.py index 6590f91..4d21626 100644 --- a/src/typedal/rows.py +++ b/src/typedal/rows.py @@ -491,7 +491,7 @@ def as_dict(self, *_: t.Any, **__: t.Any) -> PaginateDict: # type: ignore return {"data": super().as_dict(), "pagination": self.pagination} -class TypedSet(pydal.objects.Set): # type: ignore # pragma: no cover +class TypedSet(pydal.objects.Set): # pragma: no cover """ Used to make pydal Set more typed. diff --git a/src/typedal/types.py b/src/typedal/types.py index 3f0f164..d16767f 100644 --- a/src/typedal/types.py +++ b/src/typedal/types.py @@ -88,15 +88,15 @@ def open(self, file: str, mode: str = "r") -> t.IO[t.Any]: # --------------------------------------------------------------------------- -class Query(_Query): # type: ignore +class Query(_Query): """Pydal Query object. Makes mypy happy.""" -class Expression(_Expression): # type: ignore +class Expression(_Expression): """Pydal Expression object. Make mypy happy.""" -class Set(_Set): # type: ignore +class Set(_Set): """Pydal Set object. Make mypy happy.""" @@ -117,19 +117,19 @@ def __setitem__(self, key: str, value: t.Any) -> None: else: - class OpRow(_OpRow): # type: ignore + class OpRow(_OpRow): """Runtime OpRow, using pydal's version.""" -class Reference(_Reference): # type: ignore +class Reference(_Reference): """Pydal Reference object. Make mypy happy.""" -class Field(_Field): # type: ignore +class Field(_Field): """Pydal Field object. Make mypy happy.""" -class Rows(_Rows): # type: ignore +class Rows(_Rows): """Pydal Rows object. Make mypy happy.""" def column(self, column: t.Any = None) -> list[t.Any]: @@ -146,11 +146,11 @@ class Row(_Row): """Pydal Row object. Make mypy happy.""" -class Validator(_Validator): # type: ignore +class Validator(_Validator): """Pydal Validator object. Make mypy happy.""" -class Table(_Table, TableProtocol): # type: ignore +class Table(_Table, TableProtocol): """Table with protocol support. Make mypy happy.""" From 5c9c78f77925bcd6cdc8a4e0ec7a66e4c7b1c0df Mon Sep 17 00:00:00 2001 From: Robin van der Noord Date: Tue, 20 Jan 2026 15:10:14 +0100 Subject: [PATCH 4/4] chore: black formatting --- src/typedal/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/typedal/cli.py b/src/typedal/cli.py index ae6c7a0..d18204a 100644 --- a/src/typedal/cli.py +++ b/src/typedal/cli.py @@ -392,7 +392,8 @@ def fake_migrations( previously_migrated = ( db( - db.ewh_implemented_features.name.belongs(to_fake) & (db.ewh_implemented_features.installed == True) # noqa E712 + db.ewh_implemented_features.name.belongs(to_fake) + & (db.ewh_implemented_features.installed == True) # noqa E712 ) .select(db.ewh_implemented_features.name) .column("name")