remove operator bool() for t_tscalar, implement custom size_t > t_tsc…

…alar comparator, fix vectors for exprtk
finos · texodus · Sep 26, 2021 · Sep 15, 2021 · Sep 22, 2021 · Sep 22, 2021
commit 50c28250fb4d29d518c52981c8edae7235abde08
diff --git a/cpp/perspective/src/cpp/context_unit.cpp b/cpp/perspective/src/cpp/context_unit.cpp
@@ -381,7 +381,7 @@ t_ctxunit::unity_get_row_data(t_uindex idx) const {
 
 std::vector<t_tscalar>
 t_ctxunit::unity_get_row_path(t_uindex idx) const {
-    return std::vector<t_tscalar>(mktscalar(idx));
+    return {};
 }
 
 std::vector<t_tscalar>

diff --git a/cpp/perspective/src/cpp/context_zero.cpp b/cpp/perspective/src/cpp/context_zero.cpp
@@ -798,7 +798,7 @@ t_ctx0::unity_get_column_data(t_uindex idx) const {
 
 std::vector<t_tscalar>
 t_ctx0::unity_get_row_path(t_uindex idx) const {
-    return std::vector<t_tscalar>(mktscalar(idx));
+    return {};
 }
 
 std::vector<t_tscalar>

diff --git a/cpp/perspective/src/cpp/emscripten.cpp b/cpp/perspective/src/cpp/emscripten.cpp
@@ -111,11 +111,7 @@ namespace binding {
         }
         switch (scalar.get_dtype()) {
             case DTYPE_BOOL: {
-                if (scalar) {
-                    return t_val(true);
-                } else {
-                    return t_val(false);
-                }
+                return t_val(scalar.as_bool());
             }
             case DTYPE_TIME: {
                 if (cast_double) {

diff --git a/cpp/perspective/src/cpp/expression_tables.cpp b/cpp/perspective/src/cpp/expression_tables.cpp
@@ -75,8 +75,7 @@ t_expression_tables::calculate_transitions(
 
                 bool prev_valid = prev_column.is_valid(ridx);
                 bool curr_valid = current_column.is_valid(ridx);
-                bool prev_curr_eq
-                    = prev_valid && curr_value && prev_value == curr_value;
+                bool prev_curr_eq = prev_valid && curr_valid && (prev_value == curr_value);
 
                 t_value_transition transition;
 

diff --git a/cpp/perspective/src/cpp/multi_sort.cpp b/cpp/perspective/src/cpp/multi_sort.cpp
@@ -120,8 +120,8 @@ get_minmax_idx(const std::vector<t_tscalar>& vec, t_sorttype stype) {
             for (t_index idx = 0, loop_end = vec.size(); idx < loop_end;
                  ++idx) {
                 double val = std::abs(vec[idx].to_double());
-                double mindbl = std::abs(double(min_max.first));
-                double maxdbl = std::abs(double(min_max.second));
+                double mindbl = std::abs(double(min_max.first.as_bool()));
+                double maxdbl = std::abs(double(min_max.second.as_bool()));
 
                 if (val <= mindbl) {
                     min_max.first.set(val);

diff --git a/cpp/perspective/src/cpp/scalar.cpp b/cpp/perspective/src/cpp/scalar.cpp
@@ -25,6 +25,10 @@ SUPPRESS_WARNINGS_VC(4800)
 
 namespace perspective {
 
+bool operator>(const std::size_t& lhs, const t_tscalar& rhs) {
+   return rhs.operator<(lhs);
+}
+
 #define BINARY_OPERATOR_BODY(OP)                                               \
     t_tscalar rval;                                                            \
     rval.clear();                                                              \
@@ -867,7 +871,8 @@ t_tscalar::is_signed() const {
         || m_type == DTYPE_INT16 || m_type == DTYPE_INT8);
 }
 
-t_tscalar::operator bool() const {
+bool
+t_tscalar::as_bool() const {
     if (m_status != STATUS_VALID)
         return false;
 

diff --git a/cpp/perspective/src/cpp/sparse_tree.cpp b/cpp/perspective/src/cpp/sparse_tree.cpp
@@ -1089,7 +1089,7 @@ t_stree::update_agg_table(t_uindex nidx, t_agg_update_info& info,
                 apply_from_gstate(gstate, expression_master_table,
                     spec.get_dependencies()[0].name(), pkeys, new_value,
                     [](const t_tscalar& row_value, t_tscalar& output) {
-                        if (row_value) {
+                        if (row_value.as_bool()) {
                             output.set(row_value);
                             return true;
                         }
@@ -1231,17 +1231,21 @@ t_stree::update_agg_table(t_uindex nidx, t_agg_update_info& info,
                 old_value.set(dst->get_scalar(dst_ridx));
                 auto pkeys = get_pkeys(nidx);
 
-                new_value.set(reduce_from_gstate<
-                    std::function<t_tscalar(std::vector<t_tscalar>&)>>(gstate,
-                    expression_master_table, spec.get_dependencies()[0].name(),
-                    pkeys, [](std::vector<t_tscalar>& values) {
-                        t_tscalar rval;
-                        rval.set(true);
-
-                        for (const auto& v : values) {
-                            if (!v) {
-                                rval.set(false);
-                                break;
+                new_value.set(
+                    reduce_from_gstate<std::function<t_tscalar(std::vector<t_tscalar>&)>>(
+                        gstate,
+                        expression_master_table,
+                        spec.get_dependencies()[0].name(),
+                        pkeys,
+                        [](std::vector<t_tscalar>& values) {
+                            t_tscalar rval;
+                            rval.set(true);
+
+                            for (const auto& v : values) {
+                                if (!v.as_bool()) {
+                                    rval.set(false);
+                                    break;
+                                }
                             }
                         }
                         return rval;

diff --git a/cpp/perspective/src/include/perspective/scalar.h b/cpp/perspective/src/include/perspective/scalar.h
@@ -164,7 +164,15 @@ struct PERSPECTIVE_EXPORT t_tscalar {
     bool ends_with(const t_tscalar& other) const;
     bool contains(const t_tscalar& other) const;
     bool is_valid() const;
-    operator bool() const;
+
+    /**
+     * @brief Identical to operator bool(), but less ambiguous.
+     * 
+     * @return true 
+     * @return false 
+     */
+    bool as_bool() const;
+
     void clear();
     t_dtype get_dtype() const;
     const char* get_char_ptr() const;
@@ -408,6 +416,17 @@ mktscalar(const T& v) {
 
 t_tscalar mktscalar();
 
+/**
+ * @brief Overload comparisons between size_t and t_tscalar as the comparison
+ * is used in exprtk for scalar initialization.
+ * 
+ * @param lhs 
+ * @param rhs 
+ * @return true 
+ * @return false 
+ */
+bool operator>(const std::size_t& lhs, const t_tscalar& rhs);
+
 } // end namespace perspective
 
 namespace std {

diff --git a/packages/perspective/test/js/expressions/updates.js b/packages/perspective/test/js/expressions/updates.js
@@ -896,6 +896,59 @@ module.exports = (perspective) => {
             table.delete();
         });
 
+        it("multiple updates change validity", async function () {
+            const table = await perspective.table(
+                {
+                    a: "float",
+                    b: "float",
+                    c: "string",
+                    index: "integer",
+                },
+                {index: "index"}
+            );
+
+            table.update({
+                a: [null, null, 1.5, null],
+                b: [null, 2.5, null, null],
+                c: ["a", "b", "b", "a"],
+                index: [1, 2, 3, 4],
+            });
+
+            const view = await table.view({
+                expressions: ['// computed\n"a" + "b"'],
+            });
+
+            let result = await view.to_columns();
+            expect(result["computed"]).toEqual([null, null, null, null]);
+
+            table.update({
+                index: [4],
+                a: [100],
+            });
+
+            result = await view.to_columns();
+            expect(result["computed"]).toEqual([null, null, null, null]);
+
+            table.update({
+                index: [4],
+                b: [100],
+            });
+
+            result = await view.to_columns();
+            expect(result["computed"]).toEqual([null, null, null, 200]);
+
+            table.update({
+                index: [3],
+                b: [100],
+            });
+
+            result = await view.to_columns();
+            expect(result["computed"]).toEqual([null, null, 101.5, 200]);
+
+            await view.delete();
+            await table.delete();
+        });
+
         it("multiple partial update on single computed source column", async function () {
             const table = await perspective.table(data);
             const view = await table.view({