Merge pull request finos#2842 from finos/fix-integer-cast-function

Fix `integer` cast expression function
Point72 · Nov 10, 2024 · 452bfb0 · 452bfb0
2 parents 992bf93 + 7b88aa1
commit 452bfb0
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 5 deletions.
diff --git a/cpp/perspective/src/cpp/scalar.cpp b/cpp/perspective/src/cpp/scalar.cpp
@@ -1165,6 +1165,10 @@ t_tscalar::to_double() const {
 
 t_tscalar
 t_tscalar::coerce_numeric_dtype(t_dtype dtype) const {
+    if (dtype == m_type) {
+        return *this;
+    }
+
     switch (dtype) {
         case DTYPE_INT64: {
             return coerce_numeric<std::int64_t>();

diff --git a/cpp/perspective/src/cpp/sparse_tree.cpp b/cpp/perspective/src/cpp/sparse_tree.cpp
@@ -1146,6 +1146,7 @@ t_stree::update_agg_table(
 
                 // is_nan returns false for non-float types
                 if (is_expr || old_value.is_nan()) {
+
                     // if we previously had a NaN, add can't make it finite
                     // again; recalculate entire sum in case it is now finite
                     auto pkeys = get_pkeys(nidx);
@@ -1156,27 +1157,28 @@ t_stree::update_agg_table(
                             expression_master_table,
                             spec.get_dependencies()[0].name(),
                             pkeys,
-                            [](std::vector<t_tscalar>& values) {
+                            [&](std::vector<t_tscalar>& values) {
                                 if (values.empty()) {
                                     return mknone();
                                 }
 
                                 t_tscalar rval;
                                 rval.set(std::uint64_t(0));
-                                rval.m_type = values[0].m_type;
-
+                                rval.m_type = dst->get_dtype();
                                 for (const auto& v : values) {
                                     if (v.is_nan()) {
                                         continue;
                                     }
-                                    rval = rval.add(v);
+
+                                    rval = rval.add(
+                                        v.coerce_numeric_dtype(dst->get_dtype())
+                                    );
                                 }
 
                                 return rval;
                             }
                         )
                     );
-                    dst->set_scalar(dst_ridx, new_value);
                 } else {
                     new_value.set(dst_scalar.add(src_scalar));
                 }

diff --git a/packages/perspective-workspace/package.json b/packages/perspective-workspace/package.json
@@ -32,6 +32,7 @@
     "author": "",
     "license": "Apache-2.0",
     "dependencies": {
+        "@finos/perspective": "workspace:^",
         "@finos/perspective-viewer": "workspace:^",
         "@lumino/algorithm": ">=2 <3",
         "@lumino/commands": ">=2 <3",

diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/rust/perspective-js/test/js/expressions/numeric.spec.js b/rust/perspective-js/test/js/expressions/numeric.spec.js
@@ -706,6 +706,44 @@ function validate_binary_operations(output, expressions, operator) {
             });
         });
 
+        test.describe("Integers", function () {
+            test("Integer outputs with negative values dont overflow", async function () {
+                var data = [
+                    { x: -1, y: "a", z: true },
+                    { x: -2, y: "b", z: false },
+                    { x: -3, y: "c", z: true },
+                    { x: -4, y: "d", z: false },
+                ];
+
+                var table = await perspective.table({
+                    x: "integer",
+                    y: "string",
+                    z: "boolean",
+                });
+
+                await table.update(data);
+
+                var view = await table.view({
+                    group_by: ["z"],
+                    columns: ["x", "w"],
+                    expressions: {
+                        w: 'integer("x")',
+                    },
+                });
+
+                var answer = [
+                    { __ROW_PATH__: [], x: -10, w: -10 },
+                    { __ROW_PATH__: [false], x: -6, w: -6 },
+                    { __ROW_PATH__: [true], x: -4, w: -4 },
+                ];
+
+                let result = await view.to_json();
+                expect(result).toEqual(answer);
+                view.delete();
+                table.delete();
+            });
+        });
+
         test.describe("Functions", function () {
             test("min", async function () {
                 const table = await perspective.table({