added example from the issue and fixed empty over#1483
Conversation
There was a problem hiding this comment.
This serializer specialization is unused.
There was a problem hiding this comment.
We can reuse streaming_actions_tuple:
ss << " OVER (" << streaming_actions_tuple(arguments, context) << ")";It makes sense to rename streaming_actions_tuple to streaming_definition_tuple.
In general it would be nice to keep the [first = true], sep[std::exchange(first, false)] logic that we have everywhere else.
There was a problem hiding this comment.
We can reuse streaming_actions_tuple:
ss << "WINDOW " << statement.name << " AS (" << streaming_actions_tuple(statement.arguments, context) << ")";It makes sense to rename streaming_actions_tuple to streaming_definition_tuple.
| @@ -548,9 +548,10 @@ namespace sqlite_orm::internal { | |||
| template<class Tuple, class Ctx> | |||
| void serialize_over_arguments(std::stringstream& ss, const Tuple& arguments, const Ctx& context) { | |||
| using args_tuple = std::decay_t<Tuple>; | |||
There was a problem hiding this comment.
Decay is unnecessary here.
| void serialize_over_arguments(std::stringstream& ss, const Tuple& arguments, const Ctx& context) { | ||
| using args_tuple = std::decay_t<Tuple>; | ||
| if constexpr (std::tuple_size_v<args_tuple> == 0) { | ||
| if constexpr (std::tuple_size_v<Tuple> == 0) { |
There was a problem hiding this comment.
Oh sorry, I forgot to include this in the last review: we still need to use std::tuple_size<>::value. We require C++17 core language features but we still can't rely on a fully implemented C++17 Standard Library.
| ss << " OVER ()"; | ||
| } else if constexpr (std::tuple_size_v<args_tuple> == 1 && | ||
| std::is_same_v<std::tuple_element_t<0, args_tuple>, window_ref_t>) { | ||
| } else if constexpr (std::tuple_size_v<Tuple> == 1 && |
There was a problem hiding this comment.
Use std::tuple_size<>::value
2 participants
No description provided.