sdk: clean up ABI aggregates funcs

Author: malone-at-work

mysql-test/suite/villagesql/std_data/aggregate_vdf.cc

@@ -16,27 +16,15 @@
 
 // Test aggregate VDFs for VillageSQL.
 //
-// This extension demonstrates different ways to implement aggregate VDFs,
-// ordered from most idiomatic to most manual:
+// This extension demonstrates different aggregate VDF implementations using
+// make_aggregate_func. All three callbacks use typed C++ signatures:
 //
-// 1. vdf_sum(INT) -> INT: Typed API with std::optional<long long> as state.
-//    Uses .state<T>() for automatic allocation, typed clear/accumulate
-//    callbacks taking State&, and a result function returning
-//    std::optional<long long> (nullopt -> SQL NULL).
+// 1. vdf_sum(INT) -> INT: std::optional<long long> state (nullable result).
+// 2. vdf_count(INT) -> INT: plain long long state (non-nullable result).
+// 3. vdf_concat(STRING) -> STRING: std::optional<std::string> state.
+// 4. vdf_max(INT) -> INT: struct state with typed clear/accumulate/result.
 //
-// 2. vdf_count(INT) -> INT: Typed API with plain long long as state. Same
-//    pattern as vdf_sum but the result is never NULL, so the result function
-//    returns long long directly.
-//
-// 3. vdf_concat(STRING) -> STRING: Typed API with std::optional<std::string>
-//    as state. Shows that the typed aggregate API works with string types.
-//
-// 4. vdf_max(INT) -> INT: Raw ABI with manual prerun/postrun and explicit
-//    user_data casts. For authors who prefer direct control over state
-//    management or need custom prerun/postrun logic beyond simple allocation.
-//
-// Also includes simple_double(INT) -> INT as a scalar VDF for testing mixed
-// scalar/aggregate queries.
+// Also includes simple_double(INT) -> INT as a scalar VDF for mixed testing.
 
 #include <villagesql/vsql.h>
 
@@ -46,7 +34,6 @@
 using namespace vsql;
 
 // vdf_sum: aggregate that sums INT values, returns NULL for empty groups.
-// State is optional<long long> — nullopt means no non-NULL values seen.
 
 using SumState = std::optional<long long>;
 
@@ -58,7 +45,13 @@ void vdf_sum_accumulate(SumState &state, IntArg val) {
   }
 }
 
-std::optional<long long> vdf_sum_result(const SumState &state) { return state; }
+void vdf_sum_result(const SumState &state, IntResult out) {
+  if (!state.has_value()) {
+    out.set_null();
+    return;
+  }
+  out.set(state.value());
+}
 
 // vdf_count: aggregate that counts non-NULL INT values (always returns a value)
 
@@ -72,10 +65,11 @@ void vdf_count_accumulate(CountState &state, IntArg val) {
   }
 }
 
-long long vdf_count_result(const CountState &state) { return state; }
+void vdf_count_result(const CountState &state, IntResult out) {
+  out.set(state);
+}
 
 // vdf_concat: aggregate that concatenates STRING values with commas.
-// Returns NULL for empty groups.
 
 using ConcatState = std::optional<std::string>;
 
@@ -92,57 +86,41 @@ void vdf_concat_accumulate(ConcatState &state, StringArg val) {
   }
 }
 
-std::optional<std::string> vdf_concat_result(const ConcatState &state) {
-  return state;
+void vdf_concat_result(const ConcatState &state, StringResult out) {
+  if (!state.has_value()) {
+    out.set_null();
+    return;
+  }
+  out.set(state.value());
 }
 
 // vdf_max: aggregate that returns the maximum INT value.
-// Uses raw ABI callbacks with manual prerun/postrun instead of the typed API,
-// for authors who prefer explicit control over state management.
 
 struct MaxState {
   long long max_val = 0;
   bool has_value = false;
 };
 
-void vdf_max_prerun(vef_context_t *, vef_prerun_args_t *,
-                    vef_prerun_result_t *result) {
-  result->user_data = new MaxState{};
-  result->type = VEF_RESULT_VALUE;
-}
-
-void vdf_max_postrun(vef_context_t *, vef_postrun_args_t *args,
-                     vef_postrun_result_t *) {
-  delete static_cast<MaxState *>(args->user_data);
+void vdf_max_clear(MaxState &state) {
+  state.max_val = 0;
+  state.has_value = false;
 }
 
-void vdf_max_clear(vef_context_t *, vef_vdf_args_t *args) {
-  auto *state = static_cast<MaxState *>(args->user_data);
-  state->max_val = 0;
-  state->has_value = false;
-}
-
-void vdf_max_accumulate(vef_context_t *ctx, vef_vdf_args_t *args,
-                        vef_vdf_result_t *) {
-  auto *state = static_cast<MaxState *>(args->user_data);
-  vef_invalue_t val = vsql::func_builder::get_invalue(ctx, args, 0);
-  if (!val.is_null) {
-    if (!state->has_value || val.int_value > state->max_val) {
-      state->max_val = val.int_value;
-      state->has_value = true;
+void vdf_max_accumulate(MaxState &state, IntArg val) {
+  if (!val.is_null()) {
+    if (!state.has_value || val.value() > state.max_val) {
+      state.max_val = val.value();
+      state.has_value = true;
     }
   }
 }
 
-void vdf_max_result(vef_context_t *, vef_vdf_args_t *args,
-                    vef_vdf_result_t *out) {
-  auto *state = static_cast<MaxState *>(args->user_data);
-  if (!state->has_value) {
-    out->type = VEF_RESULT_NULL;
+void vdf_max_result(const MaxState &state, IntResult out) {
+  if (!state.has_value) {
+    out.set_null();
     return;
   }
-  out->int_value = state->max_val;
-  out->type = VEF_RESULT_VALUE;
+  out.set(state.max_val);
 }
 
 // Scalar function that doubles an INT value (for mixed testing)
@@ -156,32 +134,27 @@ void simple_double_impl(IntArg input, IntResult out) {
 
 VEF_GENERATE_ENTRY_POINTS(
     make_extension()
-        .func(make_func<&vdf_sum_result>("vdf_sum")
+        .func(make_aggregate_func<SumState, &vdf_sum_result>("vdf_sum")
                   .returns(INT)
                   .param(INT)
-                  .state<SumState>()
                   .clear<&vdf_sum_clear>()
                   .accumulate<&vdf_sum_accumulate>()
                   .build())
-        .func(make_func<&vdf_count_result>("vdf_count")
+        .func(make_aggregate_func<CountState, &vdf_count_result>("vdf_count")
                   .returns(INT)
                   .param(INT)
-                  .state<CountState>()
                   .clear<&vdf_count_clear>()
                   .accumulate<&vdf_count_accumulate>()
                   .build())
-        .func(make_func<&vdf_concat_result>("vdf_concat")
+        .func(make_aggregate_func<ConcatState, &vdf_concat_result>("vdf_concat")
                   .returns(STRING)
                   .param(STRING)
-                  .state<ConcatState>()
                   .clear<&vdf_concat_clear>()
                   .accumulate<&vdf_concat_accumulate>()
                   .build())
-        .func(make_func<&vdf_max_result>("vdf_max")
+        .func(make_aggregate_func<MaxState, &vdf_max_result>("vdf_max")
                   .returns(INT)
                   .param(INT)
-                  .prerun<&vdf_max_prerun>()
-                  .postrun<&vdf_max_postrun>()
                   .clear<&vdf_max_clear>()
                   .accumulate<&vdf_max_accumulate>()
                   .build())

villagesql/sdk/include/villagesql/vsql/func_builder.h

@@ -289,55 +289,6 @@ struct AggResultWithOutputWrapper {
   }
 };
 
-// Wraps T(const State&) or optional<T>(const State&) -> vef_vdf_func_t
-template <typename State, auto Func>
-struct AggResultWrapper {
-  static void invoke(vef_context_t *, vef_vdf_args_t *args,
-                     vef_vdf_result_t *result) {
-    const auto &state = *static_cast<State *>(args->user_data);
-    write_result(Func(state), result);
-  }
-
- private:
-  template <typename T>
-  static void write_result(const std::optional<T> &val,
-                           vef_vdf_result_t *result) {
-    if (!val.has_value()) {
-      result->type = VEF_RESULT_NULL;
-    } else {
-      write_scalar(*val, result);
-    }
-  }
-
-  template <typename T>
-  static void write_result(const T &val, vef_vdf_result_t *result) {
-    write_scalar(val, result);
-  }
-
-  static void write_scalar(long long v, vef_vdf_result_t *r) {
-    r->int_value = v;
-    r->type = VEF_RESULT_VALUE;
-  }
-
-  static void write_scalar(double v, vef_vdf_result_t *r) {
-    r->real_value = v;
-    r->type = VEF_RESULT_VALUE;
-  }
-
-  static void write_scalar(const std::string &v, vef_vdf_result_t *r) {
-    if (v.size() > r->max_str_len) {
-      r->type = VEF_RESULT_ERROR;
-      snprintf(r->error_msg, VEF_MAX_ERROR_LEN,
-               "aggregate result (%zu bytes) exceeds buffer (%zu bytes)",
-               v.size(), r->max_str_len);
-      return;
-    }
-    memcpy(r->str_buf, v.data(), v.size());
-    r->actual_len = v.size();
-    r->type = VEF_RESULT_VALUE;
-  }
-};
-
 // =============================================================================
 // Wrapper Template
 // =============================================================================
@@ -861,8 +812,6 @@ struct FuncBuilder {
         buffer_size_(0),
         prerun_(nullptr),
         postrun_(nullptr),
-        clear_(nullptr),
-        accumulate_(nullptr),
         deterministic_(false) {}
 
   const char *name_;
@@ -871,8 +820,6 @@ struct FuncBuilder {
   size_t buffer_size_;
   vef_prerun_func_t prerun_;
   vef_postrun_func_t postrun_;
-  vef_vdf_clear_func_t clear_;
-  vef_vdf_accumulate_func_t accumulate_;
   bool deterministic_;
 
   constexpr FuncBuilder<Func, NumParams> &returns(const char *t) {
@@ -887,8 +834,6 @@ struct FuncBuilder {
     next.buffer_size_ = buffer_size_;
     next.prerun_ = prerun_;
     next.postrun_ = postrun_;
-    next.clear_ = clear_;
-    next.accumulate_ = accumulate_;
     next.deterministic_ = deterministic_;
     for (size_t i = 0; i < NumParams; ++i) {
       next.param_types_[i] = param_types_[i];
@@ -919,74 +864,17 @@ struct FuncBuilder {
     return *this;
   }
 
-  // TODO(villagesql-beta): remove .clear(), .accumulate(), and .state() from
-  // FuncBuilder once all callers have migrated to make_aggregate_func, which
-  // validates all three callback signatures against State at compile time.
-  // Raw vef_vdf_clear_func_t / vef_vdf_accumulate_func_t are accepted here
-  // for backward compatibility with existing extensions.
-  template <auto Fn>
-  constexpr FuncBuilder<Func, NumParams> &clear() {
-    if constexpr (std::is_same_v<decltype(Fn), vef_vdf_clear_func_t>) {
-      clear_ = Fn;
-    } else {
-      using Params = typename FuncParamTypes<decltype(Fn)>::type;
-      using State = std::remove_reference_t<std::tuple_element_t<0, Params>>;
-      clear_ = &agg_clear_wrapper<State, Fn>;
-    }
-    return *this;
-  }
-
-  template <auto Fn>
-  constexpr FuncBuilder<Func, NumParams> &accumulate() {
-    if constexpr (std::is_same_v<decltype(Fn), vef_vdf_accumulate_func_t>) {
-      accumulate_ = Fn;
-    } else {
-      using Params = typename FuncParamTypes<decltype(Fn)>::type;
-      using State = std::remove_reference_t<std::tuple_element_t<0, Params>>;
-      accumulate_ = &AggAccumulateWrapper<State, Fn, NumParams>::invoke;
-    }
-    return *this;
-  }
-
-  template <typename State>
-  constexpr FuncBuilder<Func, NumParams> &state() {
-    prerun_ = &auto_prerun<State>;
-    postrun_ = &auto_postrun<State>;
-    return *this;
-  }
-
   constexpr StaticFuncDesc<NumParams> build() const {
     static_assert(NumParams <= kMaxParams,
                   "Too many parameters (max is kMaxParams)");
-    if ((clear_ == nullptr) != (accumulate_ == nullptr)) {
-      config_error__aggregate_must_set_both_clear_and_accumulate();
-    }
 
     using AllParams = typename FuncParamTypes<decltype(Func)>::type;
     using UniquePTuple = typename unique_params_types<AllParams>::type;
 
     FuncWithMetadata meta{};
-    if constexpr (std::is_same_v<decltype(Func), vef_vdf_func_t>) {
-      // Raw vef_vdf_func_t: passed through for backward compatibility.
-      // TODO(villagesql-beta): remove once all callers use make_aggregate_func.
-      meta.f = Func;
-    } else if constexpr (std::tuple_size_v<AllParams> == 1 &&
-                         std::is_lvalue_reference_v<
-                             std::tuple_element_t<0, AllParams>> &&
-                         std::is_const_v<std::remove_reference_t<
-                             std::tuple_element_t<0, AllParams>>>) {
-      // Typed aggregate result: T(const State&) or optional<T>(const State&).
-      using State = std::remove_const_t<
-          std::remove_reference_t<std::tuple_element_t<0, AllParams>>>;
-      meta.f = &AggResultWrapper<State, Func>::invoke;
-    } else {
-      // Typed scalar VDF.
-      meta.f = &Wrapper<Func, NumParams>::invoke;
-    }
+    meta.f = &Wrapper<Func, NumParams>::invoke;
     meta.prerun = prerun_;
     meta.postrun = postrun_;
-    meta.clear = clear_;
-    meta.accumulate = accumulate_;
     meta.return_type = to_vef_type(return_type_);
     meta.num_params = NumParams;
     meta.buffer_size = buffer_size_;
@@ -1007,9 +895,9 @@ struct FuncBuilder {
 // AggFuncBuilder
 // =============================================================================
 //
-// Builder for aggregate VDFs. Prefer make_aggregate_func<State, &result_fn>()
-// over make_func() + .state<>() for aggregates: the State type is explicit,
-// and clear/accumulate signatures are validated against State at compile time.
+// Builder for aggregate VDFs. Use make_aggregate_func<State, &result_fn>().
+// The State type is explicit, and clear/accumulate signatures are validated
+// against State at compile time.
 
 template <typename State, auto Func, size_t NumParams>
 struct AggFuncBuilder {
@@ -1155,14 +1043,11 @@ constexpr AggFuncBuilder<State, Func, 0> make_aggregate_func(const char *name) {
 }
 
 // make_func<&impl>("name")
-// TODO(villagesql-beta): remove the vef_vdf_func_t exception below once all
-// callers have migrated to make_aggregate_func.
 template <auto Func>
 constexpr FuncBuilder<Func, 0> make_func(const char *name) {
   using AllParams = typename FuncParamTypes<decltype(Func)>::type;
   static_assert(
-      std::is_same_v<decltype(Func), vef_vdf_func_t> ||
-          std::tuple_size_v<AllParams> == 0 ||
+      std::tuple_size_v<AllParams> == 0 ||
           !is_context_param<std::tuple_element_t<0, AllParams>>::value,
       "vsql make_func: deprecated vef_context_t* first parameter not "
       "supported; use a typed function or make_aggregate_func (see "