Fix aggregates field in View::get_config

rust/perspective-client/src/rust/config/aggregates.rs

@@ -63,6 +63,19 @@ impl From<Aggregate> for view_config::AggList {
 
 impl From<view_config::AggList> for Aggregate {
     fn from(value: view_config::AggList) -> Self {
-        Aggregate::SingleAggregate(value.aggregations.first().unwrap().clone())
+        if value.aggregations.len() == 1 {
+            Aggregate::SingleAggregate(value.aggregations.first().unwrap().clone())
+        } else {
+            Aggregate::MultiAggregate(
+                value.aggregations.first().unwrap().clone(),
+                value
+                    .aggregations
+                    .get(1)
+                    .unwrap()
+                    .split(",")
+                    .map(|x| x.to_owned())
+                    .collect(),
+            )
+        }
     }
 }

rust/perspective-js/test/js/view_config.spec.js

@@ -13,20 +13,20 @@
 import { test, expect } from "@finos/perspective-test";
 import perspective from "./perspective_client";
 
+const data = [
+    { x: 1, y: "a", z: true },
+    { x: 2, y: "b", z: false },
+    { x: 3, y: "c", z: true },
+    {
+        x: 4,
+        y: "abcdefghijklmnopqrstuvwxyz",
+        z: false,
+    },
+];
+
 ((perspective) => {
     test.describe("View config", function () {
         test("Non-interned filter strings do not create corrupted view configs", async function () {
-            const data = [
-                { x: 1, y: "a", z: true },
-                { x: 2, y: "b", z: false },
-                { x: 3, y: "c", z: true },
-                {
-                    x: 4,
-                    y: "abcdefghijklmnopqrstuvwxyz",
-                    z: false,
-                },
-            ];
-
             const table = await perspective.table(data);
             const view = await table.view({
                 filter: [["y", "==", "abcdefghijklmnopqrstuvwxyz"]],
@@ -40,5 +40,135 @@ import perspective from "./perspective_client";
             view.delete();
             table.delete();
         });
+
+        test("Non-default aggregates are provided", async function () {
+            const table = await perspective.table(data);
+            const view = await table.view({
+                group_by: ["y"],
+                columns: ["x", "z"],
+                aggregates: { x: "count", z: "sum" },
+            });
+
+            const config = await view.get_config();
+            expect(config).toEqual({
+                aggregates: { x: "count", z: "sum" },
+                columns: ["x", "z"],
+                expressions: {},
+                filter: [],
+                group_by: ["y"],
+                sort: [],
+                split_by: [],
+            });
+
+            view.delete();
+            table.delete();
+        });
+
+        test("Compound aggregates are provided", async function () {
+            const table = await perspective.table(data);
+            const view = await table.view({
+                group_by: ["y"],
+                columns: ["x"],
+                aggregates: { x: ["weighted mean", ["y"]] },
+            });
+
+            const config = await view.get_config();
+            expect(config).toEqual({
+                aggregates: { x: ["weighted mean", ["y"]] },
+                columns: ["x"],
+                expressions: {},
+                filter: [],
+                group_by: ["y"],
+                sort: [],
+                split_by: [],
+            });
+
+            view.delete();
+            table.delete();
+        });
+
+        test("Expression aggregates are provided", async function () {
+            const table = await perspective.table(data);
+            const view = await table.view({
+                group_by: ["y"],
+                columns: ["x", "z", "new"],
+                expressions: { new: `"x" + 1` },
+                aggregates: { x: ["weighted mean", ["y"]] },
+            });
+
+            const config = await view.get_config();
+            expect(config).toEqual({
+                aggregates: {
+                    new: "sum",
+                    z: "count",
+                    x: ["weighted mean", ["y"]],
+                },
+                columns: ["x", "z", "new"],
+                expressions: { new: `"x" + 1` },
+                filter: [],
+                group_by: ["y"],
+                sort: [],
+                split_by: [],
+            });
+
+            view.delete();
+            table.delete();
+        });
+
+        // The `aggregates` field was omitted entirely in `3.0.0` until `3.9.0`
+        // (due to a bug introduced by an inebriated office drone). Prior to
+        // this, columns were sleectively omitted from the result if they were
+        // _non-default_ (to shorten JSON encoding). For example if a `"flaot"`
+        // column was anything other than `"sum"`.
+        //
+        // In revisiting this broken feature, I've updated the behavior to
+        // reflect a less certain view of what is "default", and output all
+        // columns which need aggregates. These tests preserve the `2.x`
+        // behavior.
+        test.skip("Default aggregates are omitted", async function () {
+            const table = await perspective.table(data);
+            const view = await table.view({
+                group_by: ["y"],
+                columns: ["x", "z"],
+                aggregates: { x: "sum", z: "count" },
+            });
+
+            const config = await view.get_config();
+            expect(config).toEqual({
+                aggregates: {},
+                columns: ["x", "z"],
+                expressions: {},
+                filter: [],
+                group_by: ["y"],
+                sort: [],
+                split_by: [],
+            });
+
+            view.delete();
+            table.delete();
+        });
+
+        test.skip("Mixed aggregates are provided and omitted", async function () {
+            const table = await perspective.table(data);
+            const view = await table.view({
+                group_by: ["y"],
+                columns: ["x", "z"],
+                aggregates: { x: "sum", z: "mean" },
+            });
+
+            const config = await view.get_config();
+            expect(config).toEqual({
+                aggregates: { z: "mean" },
+                columns: ["x", "z"],
+                expressions: {},
+                filter: [],
+                group_by: ["y"],
+                sort: [],
+                split_by: [],
+            });
+
+            view.delete();
+            table.delete();
+        });
     });
 })(perspective);

rust/perspective-server/cmake/modules/SetupClangd.cmake

@@ -1,6 +1,6 @@
 if(NOT WORKSPACE_ROOT)
-    message(STATUS "${Yellow}WORKSPACE_ROOT not set, assuming ${CMAKE_CURRENT_SOURCE_DIR}/../..${ColorReset}")
-    set(WORKSPACE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../..")
+    message(STATUS "${Yellow}WORKSPACE_ROOT not set, assuming ${CMAKE_CURRENT_SOURCE_DIR}/../../../..${ColorReset}")
+    set(WORKSPACE_ROOT "${CMAKE_CURRENT_SOURCE_DIR}/../../../..")
 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten")

rust/perspective-server/cpp/perspective/src/cpp/aggspec.cpp

@@ -120,10 +120,10 @@ t_aggspec::agg_str() const {
             return "sum";
         } break;
         case AGGTYPE_SUM_ABS: {
-            return "sum_abs";
+            return "sum abs";
         } break;
         case AGGTYPE_ABS_SUM: {
-            return "abs_sum";
+            return "abs sum";
         } break;
         case AGGTYPE_MUL: {
             return "mul";
@@ -135,7 +135,7 @@ t_aggspec::agg_str() const {
             return "mean";
         } break;
         case AGGTYPE_WEIGHTED_MEAN: {
-            return "weighted_mean";
+            return "weighted mean";
         } break;
         case AGGTYPE_UNIQUE: {
             return "unique";
@@ -156,13 +156,13 @@ t_aggspec::agg_str() const {
             return "join";
         } break;
         case AGGTYPE_SCALED_DIV: {
-            return "scaled_div";
+            return "scaled div";
         } break;
         case AGGTYPE_SCALED_ADD: {
-            return "scaled_add";
+            return "scaled add";
         } break;
         case AGGTYPE_SCALED_MUL: {
-            return "scaled_mul";
+            return "scaled mul";
         } break;
         case AGGTYPE_DOMINANT: {
             return "dominant";
@@ -171,13 +171,13 @@ t_aggspec::agg_str() const {
             return "first";
         } break;
         case AGGTYPE_LAST_BY_INDEX: {
-            return "last_by_index";
+            return "last by index";
         } break;
         case AGGTYPE_LAST_MINUS_FIRST: {
-            return "last_minus_first";
+            return "last minus first";
         } break;
         case AGGTYPE_PY_AGG: {
-            return "py_agg";
+            return "py agg";
         } break;
         case AGGTYPE_AND: {
             return "and";
@@ -186,13 +186,13 @@ t_aggspec::agg_str() const {
             return "or";
         } break;
         case AGGTYPE_LAST_VALUE: {
-            return "last_value";
+            return "last value";
         }
         case AGGTYPE_MAX_BY: {
-            return "max_by";
+            return "max by";
         }
         case AGGTYPE_MIN_BY: {
-            return "min_by";
+            return "min by";
         }
         case AGGTYPE_MAX: {
             return "max";
@@ -201,13 +201,13 @@ t_aggspec::agg_str() const {
             return "min";
         }
         case AGGTYPE_HIGH_WATER_MARK: {
-            return "high_water_mark";
+            return "high water mark";
         }
         case AGGTYPE_LOW_WATER_MARK: {
-            return "low_water_mark";
+            return "low water mark";
         }
         case AGGTYPE_HIGH_MINUS_LOW: {
-            return "high_minus_low";
+            return "high minuslow";
         } break;
         case AGGTYPE_UDF_COMBINER: {
             std::stringstream ss;
@@ -221,25 +221,25 @@ t_aggspec::agg_str() const {
             return ss.str();
         }
         case AGGTYPE_SUM_NOT_NULL: {
-            return "sum_not_null";
+            return "sum not null";
         }
         case AGGTYPE_MEAN_BY_COUNT: {
-            return "mean_by_count";
+            return "mean by count";
         }
         case AGGTYPE_IDENTITY: {
             return "identity";
         }
         case AGGTYPE_DISTINCT_COUNT: {
-            return "distinct_count";
+            return "distinct count";
         }
         case AGGTYPE_DISTINCT_LEAF: {
-            return "distinct_leaf";
+            return "distinct leaf";
         }
         case AGGTYPE_PCT_SUM_PARENT: {
-            return "pct_sum_parent";
+            return "pct sum parent";
         }
         case AGGTYPE_PCT_SUM_GRAND_TOTAL: {
-            return "pct_sum_grand_total";
+            return "pct sum grand total";
         }
         case AGGTYPE_VARIANCE: {
             return "variance";

rust/perspective-server/cpp/perspective/src/cpp/server.cpp

@@ -2244,6 +2244,26 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) {
                     ->add_columns(col);
             }
 
+            if (!view_config->get_row_pivots().empty()) {
+                for (const auto& aggspec : view_config->get_aggspecs()) {
+                    auto* proto_exprs = view_config_proto->mutable_aggregates();
+                    const auto agg = aggspec;
+                    if (aggspec.agg() == AGGTYPE_WEIGHTED_MEAN
+                        || aggspec.agg() == AGGTYPE_MAX_BY
+                        || aggspec.agg() == AGGTYPE_MIN_BY) {
+
+                        proto::ViewConfig_AggList agglist;
+                        agglist.add_aggregations(agg.agg_str());
+                        agglist.add_aggregations(agg.get_input_depnames()[1]);
+                        (*proto_exprs)[aggspec.name()] = agglist;
+                    } else {
+                        proto::ViewConfig_AggList agglist;
+                        agglist.add_aggregations(agg.agg_str());
+                        (*proto_exprs)[aggspec.name()] = agglist;
+                    }
+                }
+            }
+
             for (const auto& agg : view_config->get_row_pivots()) {
                 if (agg == "psp_pkey" || agg == "psp_okey") {
                     continue;