From e443cb56e07bc4b883a3db7ee32c5b5e7bf57b78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dr=2E=20Patrick=20Urbanke=20=28=E5=8A=89=E8=87=AA=E6=88=90?=
 =?UTF-8?q?=29?= <patrick@getml.com>
Date: Tue, 15 Jul 2025 22:43:00 +0200
Subject: [PATCH] Make sure we properly check the columns in the ORDER BY
 clause when there is a GROUP BY (#27)

---
 include/sqlgen/read.hpp                       |  2 +-
 include/sqlgen/select_from.hpp                | 15 ++++++---
 .../transpilation/check_aggregations.hpp      | 32 +++++++++----------
 include/sqlgen/transpilation/order_by_t.hpp   | 18 ++++++++---
 tests/postgres/test_joins_nested_grouped.cpp  |  2 +-
 5 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/include/sqlgen/read.hpp b/include/sqlgen/read.hpp
index ca8c9dd..c7c50eb 100644
--- a/include/sqlgen/read.hpp
+++ b/include/sqlgen/read.hpp
@@ -113,7 +113,7 @@ struct Read {
                   "You must assign at least one column to order by(...).");
     return Read<Type, WhereType,
                 transpilation::order_by_t<
-                    transpilation::value_t<Type>,
+                    transpilation::value_t<Type>, Nothing,
                     typename std::remove_cvref_t<ColTypes>::ColType...>,
                 LimitType>{.where_ = _r.where_};
   }
diff --git a/include/sqlgen/select_from.hpp b/include/sqlgen/select_from.hpp
index af4f1a6..1a26f21 100644
--- a/include/sqlgen/select_from.hpp
+++ b/include/sqlgen/select_from.hpp
@@ -206,11 +206,16 @@ struct SelectFrom {
                   "You cannot call to<...> before order_by(...).");
     static_assert(sizeof...(ColTypes) != 0,
                   "You must assign at least one column to order_by.");
-    return SelectFrom<
-        StructType, AliasType, FieldsType, JoinsType, WhereType, GroupByType,
-        transpilation::order_by_t<
-            StructType, typename std::remove_cvref_t<ColTypes>::ColType...>,
-        LimitType, ToType>{
+
+    using TableTupleType =
+        transpilation::table_tuple_t<StructType, AliasType, JoinsType>;
+
+    using NewOrderByType = transpilation::order_by_t<
+        TableTupleType, GroupByType,
+        typename std::remove_cvref_t<ColTypes>::ColType...>;
+
+    return SelectFrom<StructType, AliasType, FieldsType, JoinsType, WhereType,
+                      GroupByType, NewOrderByType, LimitType, ToType>{
         .fields_ = _s.fields_, .joins_ = _s.joins_, .where_ = _s.where_};
   }
 
diff --git a/include/sqlgen/transpilation/check_aggregations.hpp b/include/sqlgen/transpilation/check_aggregations.hpp
index 05c7086..ffe588a 100644
--- a/include/sqlgen/transpilation/check_aggregations.hpp
+++ b/include/sqlgen/transpilation/check_aggregations.hpp
@@ -12,24 +12,24 @@
 
 namespace sqlgen::transpilation {
 
-template <class StructType, class FieldsType, class GroupByType>
+template <class TablesType, class FieldsType, class GroupByType>
 struct CheckAggregation;
 
 /// Case: No aggregation, no group by.
-template <class StructType, class... FieldTypes>
-  requires(true && ... && !MakeField<StructType, FieldTypes>::is_aggregation)
-struct CheckAggregation<StructType, rfl::Tuple<FieldTypes...>, Nothing> {
+template <class TablesType, class... FieldTypes>
+  requires(true && ... && !MakeField<TablesType, FieldTypes>::is_aggregation)
+struct CheckAggregation<TablesType, rfl::Tuple<FieldTypes...>, Nothing> {
   static constexpr bool value = true;
 };
 
 /// Case: At least one aggregation, no group by.
-template <class StructType, class... FieldTypes>
-  requires(false || ... || MakeField<StructType, FieldTypes>::is_aggregation)
-struct CheckAggregation<StructType, rfl::Tuple<FieldTypes...>, Nothing> {
+template <class TablesType, class... FieldTypes>
+  requires(false || ... || MakeField<TablesType, FieldTypes>::is_aggregation)
+struct CheckAggregation<TablesType, rfl::Tuple<FieldTypes...>, Nothing> {
   static constexpr bool value =
       (true && ... &&
-       (MakeField<StructType, FieldTypes>::is_aggregation ||
-        !MakeField<StructType, FieldTypes>::is_column));
+       (MakeField<TablesType, FieldTypes>::is_aggregation ||
+        !MakeField<TablesType, FieldTypes>::is_column));
   static_assert(
       value,
       "If any column is aggregated and there is no GROUP BY, then all columns "
@@ -37,8 +37,8 @@ struct CheckAggregation<StructType, rfl::Tuple<FieldTypes...>, Nothing> {
 };
 
 /// Case: There is a group by.
-template <class StructType, class... FieldTypes, class... ColTypes>
-struct CheckAggregation<StructType, rfl::Tuple<FieldTypes...>,
+template <class TablesType, class... FieldTypes, class... ColTypes>
+struct CheckAggregation<TablesType, rfl::Tuple<FieldTypes...>,
                         GroupBy<ColTypes...>> {
   template <class F>
   static constexpr bool included_in_group_by =
@@ -48,9 +48,9 @@ struct CheckAggregation<StructType, rfl::Tuple<FieldTypes...>,
 
   static constexpr bool value =
       (true && ... &&
-       (MakeField<StructType, FieldTypes>::is_aggregation ||
-        (!MakeField<StructType, FieldTypes>::is_column &&
-         !MakeField<StructType, FieldTypes>::is_operation) ||
+       (MakeField<TablesType, FieldTypes>::is_aggregation ||
+        (!MakeField<TablesType, FieldTypes>::is_column &&
+         !MakeField<TablesType, FieldTypes>::is_operation) ||
         included_in_group_by<FieldTypes>));
 
   static_assert(value,
@@ -58,9 +58,9 @@ struct CheckAggregation<StructType, rfl::Tuple<FieldTypes...>,
                 "must either be aggregated or included inside the GROUP BY.");
 };
 
-template <class StructType, class FieldsType, class GroupByType>
+template <class TablesType, class FieldsType, class GroupByType>
 consteval bool check_aggregations() {
-  return CheckAggregation<std::remove_cvref_t<StructType>,
+  return CheckAggregation<std::remove_cvref_t<TablesType>,
                           std::remove_cvref_t<FieldsType>,
                           std::remove_cvref_t<GroupByType>>::value;
 }
diff --git a/include/sqlgen/transpilation/order_by_t.hpp b/include/sqlgen/transpilation/order_by_t.hpp
index 92709f3..4ceadab 100644
--- a/include/sqlgen/transpilation/order_by_t.hpp
+++ b/include/sqlgen/transpilation/order_by_t.hpp
@@ -8,6 +8,7 @@
 #include "Col.hpp"
 #include "Desc.hpp"
 #include "all_columns_exist.hpp"
+#include "check_aggregations.hpp"
 
 namespace sqlgen::transpilation {
 
@@ -31,17 +32,26 @@ struct OrderByWrapper<Desc<transpilation::Col<_name, _alias>>> {
 template <class... WrapperTypes>
 struct OrderBy {};
 
-template <class T, class... ColTypes>
+template <class TablesType, class GroupByType, class... ColTypes>
 auto make_order_by() {
   static_assert(
-      all_columns_exist<T, typename OrderByWrapper<ColTypes>::ColType...>(),
+      all_columns_exist<TablesType,
+                        typename OrderByWrapper<ColTypes>::ColType...>(),
       "A column in order_by does not exist.");
+  static_assert(
+      check_aggregations<
+          TablesType, rfl::Tuple<typename OrderByWrapper<ColTypes>::ColType...>,
+          GroupByType>(),
+      "The columns in the ORDER BY clause have not been properly "
+      "aggregated. Please refer to the stack trace for details.");
+
   return OrderBy<OrderByWrapper<ColTypes>...>{};
 }
 
-template <class T, class... ColTypes>
+template <class TablesType, class GroupByType, class... ColTypes>
 using order_by_t = std::invoke_result_t<
-    decltype(make_order_by<T, typename ColTypes::ColType...>)>;
+    decltype(make_order_by<TablesType, GroupByType,
+                           typename ColTypes::ColType...>)>;
 
 }  // namespace sqlgen::transpilation
 
diff --git a/tests/postgres/test_joins_nested_grouped.cpp b/tests/postgres/test_joins_nested_grouped.cpp
index 71355a8..003195d 100644
--- a/tests/postgres/test_joins_nested_grouped.cpp
+++ b/tests/postgres/test_joins_nested_grouped.cpp
@@ -23,7 +23,7 @@ struct Relationship {
   sqlgen::PrimaryKey<uint32_t> child_id;
 };
 
-TEST(postgres, test_joins_nested) {
+TEST(postgres, test_joins_nested_grouped) {
   const auto people1 = std::vector<Person>(
       {Person{
            .id = 0, .first_name = "Homer", .last_name = "Simpson", .age = 45},