Merge pull request #3161 from perspective-dev/weighted-mean-bug

Fix multi-column aggregates by expressions

Author: texodus

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

@@ -221,6 +221,25 @@ const std = (nums) => {
             table.delete();
         });
 
+        test("['z'], weighted mean by expression", async function () {
+            var table = await perspective.table(data2);
+            var view = await table.view({
+                group_by: ["z"],
+                aggregates: { x: ["weighted mean", ["q"]] },
+                expressions: { q: `"y"` },
+                columns: ["x"],
+            });
+            var answer = [
+                { __ROW_PATH__: [], x: 2.8333333333333335 },
+                { __ROW_PATH__: [false], x: 3.3333333333333335 },
+                { __ROW_PATH__: [true], x: 2.3333333333333335 },
+            ];
+            let result = await view.to_json();
+            expect(result).toEqual(answer);
+            view.delete();
+            table.delete();
+        });
+
         test("['z'], weighted mean on a table created from schema should return valid values after update", async function () {
             const table = await perspective.table({
                 x: "integer",

rust/perspective-python/perspective/tests/table/test_view.py

@@ -455,6 +455,44 @@ def test_view_aggregate_weighted_mean(self):
             {"__ROW_PATH__": ["a"], "y": (1.0 * 200 + 2 * 100) / (1.0 + 2)},
         ]
 
+    def test_view_aggregate_weighted_mean_by_expression(self):
+        data = [
+            {"a": "a", "x": 1, "y": 200},
+            {"a": "a", "x": 2, "y": 100},
+            {"a": "a", "x": 3, "y": None},
+        ]
+        tbl = Table(data)
+        view = tbl.view(
+            aggregates={"y": ("weighted mean", ["z"])},
+            group_by=["a"],
+            columns=["y", "z"],
+            expressions={"z": '"x"'},
+        )
+
+        assert view.to_records() == [
+            {"__ROW_PATH__": [], "y": (1.0 * 200 + 2 * 100) / (1.0 + 2), "z": 6},
+            {"__ROW_PATH__": ["a"], "y": (1.0 * 200 + 2 * 100) / (1.0 + 2), "z": 6},
+        ]
+
+    def test_view_aggregate_weighted_mean_by_expression_without_column_ref(self):
+        data = [
+            {"a": "a", "x": 1, "y": 200},
+            {"a": "a", "x": 2, "y": 100},
+            {"a": "a", "x": 3, "y": None},
+        ]
+        tbl = Table(data)
+        view = tbl.view(
+            aggregates={"y": ("weighted mean", ["z"])},
+            group_by=["a"],
+            columns=["y"],
+            expressions={"z": '"x" + 1'},
+        )
+
+        assert view.to_records() == [
+            {"__ROW_PATH__": [], "y": (2 * 200 + 3 * 100) / (2 + 3)},
+            {"__ROW_PATH__": ["a"], "y": (2 * 200 + 3 * 100) / (2 + 3)},
+        ]
+
     def test_view_aggregate_weighted_mean_with_negative_weights(self):
         data = [
             {"a": "a", "x": 1, "y": 200},

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

@@ -151,17 +151,16 @@ t_view_config::get_used_expressions() {
         std::inserter(used_cols, used_cols.end())
     );
 
-    std::copy(
-        m_row_pivots.begin(),
-        m_row_pivots.end(),
-        std::inserter(used_cols, used_cols.end())
-    );
-
-    std::copy(
-        m_row_pivots.begin(),
-        m_row_pivots.end(),
-        std::inserter(used_cols, used_cols.end())
-    );
+    for (const auto& agg : m_aggregates) {
+        if (std::find(m_columns.begin(), m_columns.end(), agg.first)
+            == m_columns.end()) {
+            continue;
+        }
+        const auto& aggregate = agg.second;
+        for (std::size_t i = 1; i < aggregate.size(); ++i) {
+            used_cols.insert(aggregate[i]);
+        }
+    }
 
     auto iter = std::remove_if(
         exprs.begin(),