From 843e39b3d011f5415faa567e6577acc4d4d598f1 Mon Sep 17 00:00:00 2001 From: Roland Bock Date: Thu, 15 Aug 2024 09:30:21 +0200 Subject: [PATCH] more tests and more clarity for aggregates --- docs/DifferencesToVersion-1.0.md | 10 + include/sqlpp11/core/aggregate_function/avg.h | 7 +- .../sqlpp11/core/aggregate_function/count.h | 7 +- include/sqlpp11/core/aggregate_function/max.h | 7 +- include/sqlpp11/core/aggregate_function/min.h | 7 +- .../sqlpp11/core/aggregate_function/over.h | 2 +- include/sqlpp11/core/aggregate_function/sum.h | 7 +- include/sqlpp11/core/basic/column.h | 3 - include/sqlpp11/core/clause/group_by.h | 15 +- include/sqlpp11/core/query/dynamic.h | 3 +- include/sqlpp11/core/type_traits/aggregates.h | 56 +++-- tests/core/types/type_traits/aggregates.cpp | 227 +++++++++--------- 12 files changed, 166 insertions(+), 185 deletions(-) diff --git a/docs/DifferencesToVersion-1.0.md b/docs/DifferencesToVersion-1.0.md index f7ba70c0..968d5d2b 100644 --- a/docs/DifferencesToVersion-1.0.md +++ b/docs/DifferencesToVersion-1.0.md @@ -27,6 +27,16 @@ The library now offers `is_distinct_from` and `is_not_distinct_from` which safel # Selecting aggregate functions The automatic name for selected aggregate functions drops the `_`. +# Aggregates and non-aggregates +They must not be mixed in a select. + +`group_by` accepts columns only (there is an escape hatch: `group_by_column`) +`select` requires either +- all selected columns are aggregate expressions (either an aggregate function or a group by column), or +- no selected column is an aggregate expression + +If group_by is specified in the select, then all columns have to be aggregate expressions. + # Dynamic queries We don't always have a completely fixed structure for our queries. For instance, there might columns that we only want to select under certain circumstances. In version 1.0, this was handled by dynamic queries. Now we introduce conditional query parts that may or may not be used at runtime: diff --git a/include/sqlpp11/core/aggregate_function/avg.h b/include/sqlpp11/core/aggregate_function/avg.h index 620a0981..4a993724 100644 --- a/include/sqlpp11/core/aggregate_function/avg.h +++ b/include/sqlpp11/core/aggregate_function/avg.h @@ -63,12 +63,7 @@ namespace sqlpp }; template - struct contains_aggregate_function> : public std::true_type - { - }; - - template - struct contains_non_aggregate> : public std::false_type + struct is_aggregate_function> : public std::true_type { }; diff --git a/include/sqlpp11/core/aggregate_function/count.h b/include/sqlpp11/core/aggregate_function/count.h index 48561929..7577fc5b 100644 --- a/include/sqlpp11/core/aggregate_function/count.h +++ b/include/sqlpp11/core/aggregate_function/count.h @@ -63,12 +63,7 @@ namespace sqlpp }; template - struct contains_aggregate_function> : public std::true_type - { - }; - - template - struct contains_non_aggregate> : public std::false_type + struct is_aggregate_function> : public std::true_type { }; diff --git a/include/sqlpp11/core/aggregate_function/max.h b/include/sqlpp11/core/aggregate_function/max.h index 78efaf7a..d4f53286 100644 --- a/include/sqlpp11/core/aggregate_function/max.h +++ b/include/sqlpp11/core/aggregate_function/max.h @@ -62,12 +62,7 @@ namespace sqlpp }; template - struct contains_aggregate_function> : public std::true_type - { - }; - - template - struct contains_non_aggregate> : public std::false_type + struct is_aggregate_function> : public std::true_type { }; diff --git a/include/sqlpp11/core/aggregate_function/min.h b/include/sqlpp11/core/aggregate_function/min.h index b4b274f9..eb06ca56 100644 --- a/include/sqlpp11/core/aggregate_function/min.h +++ b/include/sqlpp11/core/aggregate_function/min.h @@ -62,12 +62,7 @@ namespace sqlpp }; template - struct contains_aggregate_function> : public std::true_type - { - }; - - template - struct contains_non_aggregate> : public std::false_type + struct is_aggregate_function> : public std::true_type { }; diff --git a/include/sqlpp11/core/aggregate_function/over.h b/include/sqlpp11/core/aggregate_function/over.h index ea3a84e1..81f5325c 100644 --- a/include/sqlpp11/core/aggregate_function/over.h +++ b/include/sqlpp11/core/aggregate_function/over.h @@ -62,7 +62,7 @@ namespace sqlpp struct value_type_of>: public value_type_of {}; template - using check_over_args = ::sqlpp::enable_if_t::value>; + using check_over_args = ::sqlpp::enable_if_t::value>; template auto to_sql_string(Context& context, const over_t& t) -> std::string diff --git a/include/sqlpp11/core/aggregate_function/sum.h b/include/sqlpp11/core/aggregate_function/sum.h index f08bfb56..45c1daf7 100644 --- a/include/sqlpp11/core/aggregate_function/sum.h +++ b/include/sqlpp11/core/aggregate_function/sum.h @@ -62,12 +62,7 @@ namespace sqlpp }; template - struct contains_aggregate_function> : public std::true_type - { - }; - - template - struct contains_non_aggregate> : public std::false_type + struct is_aggregate_function> : public std::true_type { }; diff --git a/include/sqlpp11/core/basic/column.h b/include/sqlpp11/core/basic/column.h index ca4a2cdc..626663d8 100644 --- a/include/sqlpp11/core/basic/column.h +++ b/include/sqlpp11/core/basic/column.h @@ -85,9 +85,6 @@ namespace sqlpp } }; - template - struct contains_non_aggregate> : public std::true_type {}; - template struct value_type_of> { diff --git a/include/sqlpp11/core/clause/group_by.h b/include/sqlpp11/core/clause/group_by.h index 98a42582..221e3cf7 100644 --- a/include/sqlpp11/core/clause/group_by.h +++ b/include/sqlpp11/core/clause/group_by.h @@ -62,9 +62,7 @@ namespace sqlpp using _traits = make_traits; using _nodes = detail::type_vector; - using _provided_aggregates = detail::make_type_set_t; - - using _data_t = group_by_data_t; + using _data_t = group_by_data_t; // Base template to be inherited by the statement template @@ -82,8 +80,14 @@ namespace sqlpp }; }; - SQLPP_PORTABLE_STATIC_ASSERT(assert_group_by_args_have_values_t, - "all arguments for group_by() must have values"); + template + struct known_aggregate_expressions_of> + { + using type = detail::type_vector; + }; + + SQLPP_PORTABLE_STATIC_ASSERT(assert_group_by_args_have_values_t, "all arguments for group_by() must have values"); + template struct check_group_by { @@ -118,6 +122,7 @@ namespace sqlpp using _consistency_check = consistent_t; +#warning: Arguments should not be constants and not contain aggregate functions template auto group_by(Expressions... expressions) const -> _new_statement_t, group_by_t> diff --git a/include/sqlpp11/core/query/dynamic.h b/include/sqlpp11/core/query/dynamic.h index 69b8e22c..417999fa 100644 --- a/include/sqlpp11/core/query/dynamic.h +++ b/include/sqlpp11/core/query/dynamic.h @@ -55,8 +55,9 @@ namespace sqlpp // No value_type_of or name_tag_of defined for dynamic_t, to prevent its usage outside of select columns. template - struct nodes_of> : public nodes_of + struct nodes_of> { + using type = Expr; }; template diff --git a/include/sqlpp11/core/type_traits/aggregates.h b/include/sqlpp11/core/type_traits/aggregates.h index 218ce85a..6804baae 100644 --- a/include/sqlpp11/core/type_traits/aggregates.h +++ b/include/sqlpp11/core/type_traits/aggregates.h @@ -32,47 +32,51 @@ namespace sqlpp { - // Finds calls to aggregate functions (avg, count, max, min, sum). + // We don't want to mix aggregate and non-aggregate expressions as the results are unspecified. + // Aggregates are either results of aggregate functions or GROUP BY columns. + // Non-aggregates are columns (unless they are aggregate expressions). + // Constant values are neutral. + template - struct contains_aggregate_function : public std::integral_constant>::value> + struct is_aggregate_function : public std::false_type + { + }; + + // Finds calls to aggregate functions (avg, count, max, min, sum) in expressions. + // This is important as aggregated functions must not be nested. + template + struct contains_aggregate_function + : public std::integral_constant::value or + contains_aggregate_function>::value> { }; template struct contains_aggregate_function> - : public std::integral_constant::value...>::value> - { - }; - - // Finds group_by expression. - // @GroupByExpressions: type_vector - template - struct contains_aggregate_expression - : public std::integral_constant::value or - contains_aggregate_expression>::value> - { - }; - - template - struct contains_aggregate_expression> : public std::integral_constant< bool, - logic::any_t<(GroupByExpressions::template contains::value or - contains_aggregate_expression::value)...>::value> + logic::any_t<(is_aggregate_function::value or contains_aggregate_function::value)...>::value> { }; - // Finds columns. - // Note that explicit values like `value(7)` are compatible with both aggregate and non-aggregate. + // Obtain known aggregate expressions, i.e. GROUP BY columns. template - struct contains_non_aggregate : public std::integral_constant>::value> + struct known_aggregate_expressions_of { + using type = detail::type_vector<>; }; - template - struct contains_non_aggregate> - : public std::integral_constant::value...>::value> + template + using known_aggregate_expressions_of_t = typename known_aggregate_expressions_of::type; + + // Checks if T is an aggregate expression (either an aggregate function or a known aggregate). + // @KnownAggregateExpressions: type_vector as obtained through known_aggregate_expressions_of_t + template + struct is_aggregate_expression + : public std::integral_constant::value or + KnownAggregateExpressions::template contains::value> { }; diff --git a/tests/core/types/type_traits/aggregates.cpp b/tests/core/types/type_traits/aggregates.cpp index 97ad0f28..a0309954 100644 --- a/tests/core/types/type_traits/aggregates.cpp +++ b/tests/core/types/type_traits/aggregates.cpp @@ -28,6 +28,65 @@ SQLPP_ALIAS_PROVIDER(cheese); +void test_is_aggregate_function() +{ + auto v = sqlpp::value(17); + auto t = sqlpp::value(""); + auto col_int = test::TabFoo{}.id; + auto col_txt = test::TabFoo{}.textNnD; + + // Constant values are neutral and therefore considered neither aggregate and non-aggregate. + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + + // Columns are not aggregate functions + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + + // Normal functions of values or non-aggregates do not contain aggregate functions. + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + + // Aggregate functions of non-aggregates and values are aggregate functions :-) + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(sqlpp::is_aggregate_function::value, ""); + + static_assert(sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + + // Expressions containing aggregate functions are not aggregate functions. + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + + static_assert(not sqlpp::is_aggregate_function::value, ""); + + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + + static_assert(not sqlpp::is_aggregate_function::value, ""); + + // Clauses do not expose aggregates functions. + static_assert(not sqlpp::is_aggregate_function v))>::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); + static_assert(not sqlpp::is_aggregate_function::value, ""); +} + void test_contains_aggregate_function() { auto v = sqlpp::value(17); @@ -35,7 +94,7 @@ void test_contains_aggregate_function() auto col_int = test::TabFoo{}.id; auto col_txt = test::TabFoo{}.textNnD; - // Values are aggregate neutral and therefore considered neither aggregate and non-aggregate context. + // Constant values are neutral and therefore considered neither aggregate and non-aggregate. static_assert(not sqlpp::contains_aggregate_function::value, ""); static_assert(not sqlpp::contains_aggregate_function::value, ""); @@ -48,11 +107,11 @@ void test_contains_aggregate_function() static_assert(not sqlpp::contains_aggregate_function::value, ""); // Aggregate functions of non-aggregates and values contain aggregate functions. - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); + static_assert(sqlpp::contains_aggregate_function::value, ""); + static_assert(sqlpp::contains_aggregate_function::value, ""); + static_assert(sqlpp::contains_aggregate_function::value, ""); + static_assert(sqlpp::contains_aggregate_function::value, ""); + static_assert(sqlpp::contains_aggregate_function::value, ""); static_assert(sqlpp::contains_aggregate_function::value, ""); static_assert(sqlpp::contains_aggregate_function::value, ""); @@ -79,140 +138,70 @@ void test_contains_aggregate_function() static_assert(sqlpp::contains_aggregate_function::value, ""); static_assert(sqlpp::contains_aggregate_function::value, ""); - // Clauses expose non-aggregates (probably irrelevant) + // Clauses expose non-aggregates. static_assert(not sqlpp::contains_aggregate_function v))>::value, ""); + static_assert(not sqlpp::contains_aggregate_function::value, ""); static_assert(sqlpp::contains_aggregate_function::value, ""); + static_assert(sqlpp::contains_aggregate_function::value, ""); } -void test_contains_aggregate_expression() +void test_is_aggregate_expression() { auto v_not_null = sqlpp::value(17); auto v = sqlpp::value(17); auto t = sqlpp::value(""); - auto col_int = test::TabFoo{}.id; - auto col_txt = test::TabFoo{}.textNnD; + auto agg_int = test::TabFoo{}.id; + auto agg_txt = test::TabFoo{}.textNnD; + + auto col_int = test::TabBar{}.id; + auto col_txt = test::TabBar{}.textN; - using known_aggregates = sqlpp::detail::type_vector; using unknown = sqlpp::detail::type_vector<>; + using known_aggregates = sqlpp::detail::type_vector; - // ... - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); + // If there are no known aggregate expressions, then only aggregate functions will be found. + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); - // ... - static_assert(not sqlpp::contains_aggregate_expression::value, ""); - static_assert(not sqlpp::contains_aggregate_expression::value, ""); + // Known aggregate expressions are detected as such. + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); -#warning: activate -#if 0 - // Columns are non-aggregates. - static_assert(not sqlpp::contains_aggregate_expression::value, ""); - static_assert(not sqlpp::contains_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); - // Normal expressions of values or non-aggregates do not contain aggregate functions. - static_assert(not sqlpp::contains_aggregate_expression::value, ""); - static_assert(not sqlpp::contains_aggregate_expression::value, ""); + // Known aggregate expressions are not detected as such in expressions. + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); - // Aggregate expressions of non-aggregates contain aggregate functions. - static_assert(not sqlpp::contains_aggregate_expressions::value, ""); - static_assert(not sqlpp::contains_aggregate_expressions::value, ""); - static_assert(not sqlpp::contains_aggregate_expressions::value, ""); - static_assert(not sqlpp::contains_aggregate_expressions::value, ""); - static_assert(not sqlpp::contains_aggregate_expressions::value, ""); + // Known aggregate expressions are not detected as such in aggregate functions. + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); + static_assert(sqlpp::is_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - - // Expressions of aggregate expressions and non-aggregates contain aggregate functions. - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - - static_assert(sqlpp::contains_aggregate_expression::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); - - // Clauses expose non-aggregates (probably irrelevant) - static_assert(not sqlpp::contains_aggregate_expression v))>::value, ""); - static_assert(sqlpp::contains_aggregate_expression::value, ""); -#endif - -} - -void test_contains_non_aggregate() -{ - auto v = sqlpp::value(17); - auto t = sqlpp::value(""); - auto col_int = test::TabFoo{}.id; - auto col_txt = test::TabFoo{}.textNnD; - - // Values are aggregate neutral and therefore considered neither aggregate and non-aggregate context. - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - - // Columns are non-aggregates. - static_assert(sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - - // Functions can contain non-aggregates. - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - - // Aggregate functions of non-aggregates and values are aggregate functions. - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - - static_assert(not sqlpp::contains_non_aggregate::value, ""); - static_assert(not sqlpp::contains_non_aggregate::value, ""); - - // Expressions of aggregate functions and non-aggregates contain non-aggregates. - static_assert(sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - - static_assert(sqlpp::contains_non_aggregate::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); - - // Clauses expose non-aggregates (probably irrelevant) - static_assert(sqlpp::contains_non_aggregate v))>::value, ""); - static_assert(sqlpp::contains_non_aggregate::value, ""); + // Known aggregate expressions are not exposed as such by clauses. + static_assert(not sqlpp::is_aggregate_expression::value, ""); + static_assert(not sqlpp::is_aggregate_expression::value, ""); } int main() { + void test_is_aggregate_function(); void test_contains_aggregate_function(); - void test_contains_aggregate_expression(); - void test_contains_non_aggregate(); + void test_is_aggregate_expression(); }