Skip to content

Commit 7f08575

Browse files
Pierre-Luc Gagnéclaude
Pierre-Luc Gagné
and
claude
committed
refactor: replace order_by vector with string accumulation
Remove alias_order_entry struct and order_by_item variant. Replace std::vector<order_by_item> with a single std::string that accumulates ORDER BY clauses incrementally. order_by_alias() now resolves aliases eagerly at call time instead of deferring to build_sql(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 54eff20 commit 7f08575

2 files changed

Lines changed: 58 additions & 70 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ Versioning follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
88

99
## [Unreleased]
1010

11+
### Changed
12+
13+
- `select_query_builder`: replace `std::vector<order_by_item>` with a single `std::string` for ORDER BY clause accumulation. Remove `alias_order_entry` struct and `order_by_item` variant; `order_by_alias()` now resolves aliases eagerly at call time instead of deferring to `build_sql()`.
14+
1115
---
1216

1317
## [4.6.1] – 2026-03-30

‎lib/include/ds_mysql/sql_dql.hpp‎

Lines changed: 54 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -3046,17 +3046,6 @@ concept GroupBySpec = requires {
30463046
{ detail::group_by_spec_traits<T>::cols_string() } -> std::convertible_to<std::string>;
30473047
};
30483048

3049-
// alias_order_entry — deferred alias lookup for order_by_alias<Proj>()
3050-
// Resolved at build_sql() time: emits the alias name if set, otherwise
3051-
// falls back to the projection's sql_expr().
3052-
struct alias_order_entry {
3053-
std::size_t proj_index{};
3054-
std::string fallback_expr; // sql_expr() of the projection
3055-
sort_order dir{sort_order::asc};
3056-
};
3057-
3058-
using order_by_item = std::variant<std::string, alias_order_entry>;
3059-
30603049
enum class select_lock_mode {
30613050
none,
30623051
for_update,
@@ -3180,28 +3169,28 @@ struct select_query_builder {
31803169
return std::string(column_traits<Col>::column_name());
31813170
}
31823171
}();
3183-
copy.order_by_clauses_.push_back("(" + col_name +
3172+
copy.append_order_by_("(" + col_name +
31843173
(ColSpec::puts_nulls_last ? " IS NULL) ASC" : " IS NULL) DESC"));
3185-
copy.order_by_clauses_.push_back(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
3174+
copy.append_order_by_(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
31863175
} else if constexpr (QualifiedCol<ColSpec>) {
3187-
copy.order_by_clauses_.push_back(qualified_col_name<typename ColSpec::col_type>() +
3176+
copy.append_order_by_(qualified_col_name<typename ColSpec::col_type>() +
31883177
(Dir == sort_order::asc ? " ASC" : " DESC"));
31893178
} else if constexpr (PositionWrapper<ColSpec>) {
31903179
static_assert((std::is_same_v<typename ColSpec::proj_type, Projs> || ...),
31913180
"order_by(position<Proj>): Proj must be one of the projections in this SELECT");
31923181
static constexpr std::size_t Index =
31933182
sql_detail::proj_index_in_pack<typename ColSpec::proj_type, Projs...>() + 1;
3194-
copy.order_by_clauses_.push_back(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
3183+
copy.append_order_by_(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
31953184
} else if constexpr (ColIndexWrapper<ColSpec>) {
31963185
static_assert(ColSpec::value <= sizeof...(Projs),
31973186
"order_by(col_index<N>): N is out of range for this SELECT list");
3198-
copy.order_by_clauses_.push_back(std::to_string(ColSpec::value) +
3187+
copy.append_order_by_(std::to_string(ColSpec::value) +
31993188
(Dir == sort_order::asc ? " ASC" : " DESC"));
32003189
} else if constexpr (Projection<ColSpec>) {
3201-
copy.order_by_clauses_.push_back(std::string(projection_traits<ColSpec>::sql_expr()) +
3190+
copy.append_order_by_(std::string(projection_traits<ColSpec>::sql_expr()) +
32023191
(Dir == sort_order::asc ? " ASC" : " DESC"));
32033192
} else {
3204-
copy.order_by_clauses_.push_back(std::string(column_traits<ColSpec>::column_name()) +
3193+
copy.append_order_by_(std::string(column_traits<ColSpec>::column_name()) +
32053194
(Dir == sort_order::asc ? " ASC" : " DESC"));
32063195
}
32073196
} else {
@@ -3217,28 +3206,28 @@ struct select_query_builder {
32173206
return std::string(column_traits<Col>::column_name());
32183207
}
32193208
}();
3220-
copy.order_by_clauses_.push_back("(" + col_name +
3209+
copy.append_order_by_("(" + col_name +
32213210
(ColSpec::puts_nulls_last ? " IS NULL) ASC" : " IS NULL) DESC"));
3222-
copy.order_by_clauses_.push_back(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
3211+
copy.append_order_by_(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
32233212
} else if constexpr (QualifiedCol<ColSpec>) {
3224-
copy.order_by_clauses_.push_back(qualified_col_name<typename ColSpec::col_type>() +
3213+
copy.append_order_by_(qualified_col_name<typename ColSpec::col_type>() +
32253214
(Dir == sort_order::asc ? " ASC" : " DESC"));
32263215
} else if constexpr (PositionWrapper<ColSpec>) {
32273216
static_assert((std::is_same_v<typename ColSpec::proj_type, Projs> || ...),
32283217
"order_by(position<Proj>): Proj must be one of the projections in this SELECT");
32293218
static constexpr std::size_t Index =
32303219
sql_detail::proj_index_in_pack<typename ColSpec::proj_type, Projs...>() + 1;
3231-
copy.order_by_clauses_.push_back(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
3220+
copy.append_order_by_(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
32323221
} else if constexpr (ColIndexWrapper<ColSpec>) {
32333222
static_assert(ColSpec::value <= sizeof...(Projs),
32343223
"order_by(col_index<N>): N is out of range for this SELECT list");
3235-
copy.order_by_clauses_.push_back(std::to_string(ColSpec::value) +
3224+
copy.append_order_by_(std::to_string(ColSpec::value) +
32363225
(Dir == sort_order::asc ? " ASC" : " DESC"));
32373226
} else if constexpr (Projection<ColSpec>) {
3238-
copy.order_by_clauses_.push_back(std::string(projection_traits<ColSpec>::sql_expr()) +
3227+
copy.append_order_by_(std::string(projection_traits<ColSpec>::sql_expr()) +
32393228
(Dir == sort_order::asc ? " ASC" : " DESC"));
32403229
} else {
3241-
copy.order_by_clauses_.push_back(std::string(column_traits<ColSpec>::column_name()) +
3230+
copy.append_order_by_(std::string(column_traits<ColSpec>::column_name()) +
32423231
(Dir == sort_order::asc ? " ASC" : " DESC"));
32433232
}
32443233
}
@@ -3261,28 +3250,28 @@ struct select_query_builder {
32613250
return std::string(column_traits<Col>::column_name());
32623251
}
32633252
}();
3264-
order_by_clauses_.push_back("(" + col_name +
3253+
append_order_by_("(" + col_name +
32653254
(ColSpec::puts_nulls_last ? " IS NULL) ASC" : " IS NULL) DESC"));
3266-
order_by_clauses_.push_back(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
3255+
append_order_by_(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
32673256
} else if constexpr (QualifiedCol<ColSpec>) {
3268-
order_by_clauses_.push_back(qualified_col_name<typename ColSpec::col_type>() +
3257+
append_order_by_(qualified_col_name<typename ColSpec::col_type>() +
32693258
(Dir == sort_order::asc ? " ASC" : " DESC"));
32703259
} else if constexpr (PositionWrapper<ColSpec>) {
32713260
static_assert((std::is_same_v<typename ColSpec::proj_type, Projs> || ...),
32723261
"order_by(position<Proj>): Proj must be one of the projections in this SELECT");
32733262
static constexpr std::size_t Index =
32743263
sql_detail::proj_index_in_pack<typename ColSpec::proj_type, Projs...>() + 1;
3275-
order_by_clauses_.push_back(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
3264+
append_order_by_(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
32763265
} else if constexpr (ColIndexWrapper<ColSpec>) {
32773266
static_assert(ColSpec::value <= sizeof...(Projs),
32783267
"order_by(col_index<N>): N is out of range for this SELECT list");
3279-
order_by_clauses_.push_back(std::to_string(ColSpec::value) +
3268+
append_order_by_(std::to_string(ColSpec::value) +
32803269
(Dir == sort_order::asc ? " ASC" : " DESC"));
32813270
} else if constexpr (Projection<ColSpec>) {
3282-
order_by_clauses_.push_back(std::string(projection_traits<ColSpec>::sql_expr()) +
3271+
append_order_by_(std::string(projection_traits<ColSpec>::sql_expr()) +
32833272
(Dir == sort_order::asc ? " ASC" : " DESC"));
32843273
} else {
3285-
order_by_clauses_.push_back(std::string(column_traits<ColSpec>::column_name()) +
3274+
append_order_by_(std::string(column_traits<ColSpec>::column_name()) +
32863275
(Dir == sort_order::asc ? " ASC" : " DESC"));
32873276
}
32883277
} else {
@@ -3298,28 +3287,28 @@ struct select_query_builder {
32983287
return std::string(column_traits<Col>::column_name());
32993288
}
33003289
}();
3301-
order_by_clauses_.push_back("(" + col_name +
3290+
append_order_by_("(" + col_name +
33023291
(ColSpec::puts_nulls_last ? " IS NULL) ASC" : " IS NULL) DESC"));
3303-
order_by_clauses_.push_back(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
3292+
append_order_by_(col_name + (Dir == sort_order::asc ? " ASC" : " DESC"));
33043293
} else if constexpr (QualifiedCol<ColSpec>) {
3305-
order_by_clauses_.push_back(qualified_col_name<typename ColSpec::col_type>() +
3294+
append_order_by_(qualified_col_name<typename ColSpec::col_type>() +
33063295
(Dir == sort_order::asc ? " ASC" : " DESC"));
33073296
} else if constexpr (PositionWrapper<ColSpec>) {
33083297
static_assert((std::is_same_v<typename ColSpec::proj_type, Projs> || ...),
33093298
"order_by(position<Proj>): Proj must be one of the projections in this SELECT");
33103299
static constexpr std::size_t Index =
33113300
sql_detail::proj_index_in_pack<typename ColSpec::proj_type, Projs...>() + 1;
3312-
order_by_clauses_.push_back(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
3301+
append_order_by_(std::to_string(Index) + (Dir == sort_order::asc ? " ASC" : " DESC"));
33133302
} else if constexpr (ColIndexWrapper<ColSpec>) {
33143303
static_assert(ColSpec::value <= sizeof...(Projs),
33153304
"order_by(col_index<N>): N is out of range for this SELECT list");
3316-
order_by_clauses_.push_back(std::to_string(ColSpec::value) +
3305+
append_order_by_(std::to_string(ColSpec::value) +
33173306
(Dir == sort_order::asc ? " ASC" : " DESC"));
33183307
} else if constexpr (Projection<ColSpec>) {
3319-
order_by_clauses_.push_back(std::string(projection_traits<ColSpec>::sql_expr()) +
3308+
append_order_by_(std::string(projection_traits<ColSpec>::sql_expr()) +
33203309
(Dir == sort_order::asc ? " ASC" : " DESC"));
33213310
} else {
3322-
order_by_clauses_.push_back(std::string(column_traits<ColSpec>::column_name()) +
3311+
append_order_by_(std::string(column_traits<ColSpec>::column_name()) +
33233312
(Dir == sort_order::asc ? " ASC" : " DESC"));
33243313
}
33253314
}
@@ -3332,13 +3321,13 @@ struct select_query_builder {
33323321
[[nodiscard]] select_query_builder order_by(case_when_builder<ValueType> expr,
33333322
sort_order dir = sort_order::asc) const& {
33343323
auto copy = *this;
3335-
copy.order_by_clauses_.push_back(expr.build_sql() + (dir == sort_order::asc ? " ASC" : " DESC"));
3324+
copy.append_order_by_(expr.build_sql() + (dir == sort_order::asc ? " ASC" : " DESC"));
33363325
return copy;
33373326
}
33383327
template <typename ValueType>
33393328
[[nodiscard]] select_query_builder order_by(case_when_builder<ValueType> expr,
33403329
sort_order dir = sort_order::asc) && {
3341-
order_by_clauses_.push_back(expr.build_sql() + (dir == sort_order::asc ? " ASC" : " DESC"));
3330+
append_order_by_(expr.build_sql() + (dir == sort_order::asc ? " ASC" : " DESC"));
33423331
return std::move(*this);
33433332
}
33443333

@@ -3356,7 +3345,7 @@ struct select_query_builder {
33563345
expr += ", " + ::ds_mysql::sql_detail::to_sql_value(v);
33573346
}
33583347
expr += ")";
3359-
copy.order_by_clauses_.push_back(std::move(expr) + (dir == sort_order::asc ? " ASC" : " DESC"));
3348+
copy.append_order_by_(std::move(expr) + (dir == sort_order::asc ? " ASC" : " DESC"));
33603349
return copy;
33613350
}
33623351
template <FieldOrderBy ColSpec, SqlValue ValueType>
@@ -3368,43 +3357,38 @@ struct select_query_builder {
33683357
expr += ", " + ::ds_mysql::sql_detail::to_sql_value(v);
33693358
}
33703359
expr += ")";
3371-
order_by_clauses_.push_back(std::move(expr) + (dir == sort_order::asc ? " ASC" : " DESC"));
3360+
append_order_by_(std::move(expr) + (dir == sort_order::asc ? " ASC" : " DESC"));
33723361
return std::move(*this);
33733362
}
33743363

33753364
// order_by_alias<Proj>() — ORDER BY the alias assigned to Proj via with_alias<Proj>()
33763365
// order_by_alias<Proj, desc>() — ORDER BY alias DESC
33773366
// Falls back to the projection's sql_expr() if no alias has been set.
3367+
// Note: with_alias() must be called before order_by_alias() for the alias to be used.
33783368
template <AnyProjection Proj>
33793369
requires((std::is_same_v<Proj, Projs> || ...))
33803370
[[nodiscard]] select_query_builder order_by_alias(Proj) const& {
3381-
static constexpr std::size_t Index = sql_detail::proj_index_in_pack<Proj, Projs...>();
33823371
auto copy = *this;
3383-
copy.order_by_clauses_.push_back(
3384-
alias_order_entry{Index, proj_sql_expr(std::get<Index>(projs_)), sort_order::asc});
3372+
copy.append_order_by_(resolve_alias_order_<Proj>(sort_order::asc));
33853373
return copy;
33863374
}
33873375
template <AnyProjection Proj>
33883376
requires((std::is_same_v<Proj, Projs> || ...))
33893377
[[nodiscard]] select_query_builder order_by_alias(Proj) && {
3390-
static constexpr std::size_t Index = sql_detail::proj_index_in_pack<Proj, Projs...>();
3391-
order_by_clauses_.push_back(alias_order_entry{Index, proj_sql_expr(std::get<Index>(projs_)), sort_order::asc});
3378+
append_order_by_(resolve_alias_order_<Proj>(sort_order::asc));
33923379
return std::move(*this);
33933380
}
33943381
template <AnyProjection Proj>
33953382
requires((std::is_same_v<Proj, Projs> || ...))
33963383
[[nodiscard]] select_query_builder order_by_alias(desc_order<Proj>) const& {
3397-
static constexpr std::size_t Index = sql_detail::proj_index_in_pack<Proj, Projs...>();
33983384
auto copy = *this;
3399-
copy.order_by_clauses_.push_back(
3400-
alias_order_entry{Index, proj_sql_expr(std::get<Index>(projs_)), sort_order::desc});
3385+
copy.append_order_by_(resolve_alias_order_<Proj>(sort_order::desc));
34013386
return copy;
34023387
}
34033388
template <AnyProjection Proj>
34043389
requires((std::is_same_v<Proj, Projs> || ...))
34053390
[[nodiscard]] select_query_builder order_by_alias(desc_order<Proj>) && {
3406-
static constexpr std::size_t Index = sql_detail::proj_index_in_pack<Proj, Projs...>();
3407-
order_by_clauses_.push_back(alias_order_entry{Index, proj_sql_expr(std::get<Index>(projs_)), sort_order::desc});
3391+
append_order_by_(resolve_alias_order_<Proj>(sort_order::desc));
34083392
return std::move(*this);
34093393
}
34103394

@@ -3809,23 +3793,9 @@ struct select_query_builder {
38093793
sql += " HAVING ";
38103794
sql += having_->build_sql();
38113795
}
3812-
if (!order_by_clauses_.empty()) {
3796+
if (!order_by_clause_.empty()) {
38133797
sql += " ORDER BY ";
3814-
bool f = true;
3815-
for (auto const& item : order_by_clauses_) {
3816-
if (!f) {
3817-
sql += ", ";
3818-
}
3819-
if (std::holds_alternative<std::string>(item)) {
3820-
sql += std::get<std::string>(item);
3821-
} else {
3822-
auto const& e = std::get<alias_order_entry>(item);
3823-
auto it = aliases_.find(e.proj_index);
3824-
sql += (it != aliases_.end() ? it->second : e.fallback_expr);
3825-
sql += (e.dir == sort_order::asc ? " ASC" : " DESC");
3826-
}
3827-
f = false;
3828-
}
3798+
sql += order_by_clause_;
38293799
}
38303800
if (limit_.has_value()) {
38313801
sql += " LIMIT ";
@@ -3908,12 +3878,26 @@ struct select_query_builder {
39083878
return s;
39093879
}
39103880

3881+
void append_order_by_(std::string clause) {
3882+
if (!order_by_clause_.empty()) order_by_clause_ += ", ";
3883+
order_by_clause_ += std::move(clause);
3884+
}
3885+
3886+
template <typename Proj>
3887+
[[nodiscard]] std::string resolve_alias_order_(sort_order dir) const {
3888+
static constexpr std::size_t Index = sql_detail::proj_index_in_pack<Proj, Projs...>();
3889+
auto it = aliases_.find(Index);
3890+
std::string result = (it != aliases_.end()) ? it->second : proj_sql_expr(std::get<Index>(projs_));
3891+
result += (dir == sort_order::asc ? " ASC" : " DESC");
3892+
return result;
3893+
}
3894+
39113895
std::optional<sql_predicate> where_;
39123896
std::string group_by_;
39133897
bool with_rollup_ = false;
39143898
group_by_mode group_by_mode_ = group_by_mode::standard;
39153899
std::optional<sql_predicate> having_;
3916-
std::vector<order_by_item> order_by_clauses_;
3900+
std::string order_by_clause_;
39173901
std::optional<std::size_t> limit_;
39183902
std::optional<std::size_t> offset_;
39193903
select_lock_mode lock_mode_ = select_lock_mode::none;

0 commit comments

Comments
 (0)