From d0b87ae3b11188c386fdee1c2525d7431e7423a4 Mon Sep 17 00:00:00 2001 From: Naveen Michaud-Agrawal Date: Wed, 10 Oct 2018 23:50:44 -0400 Subject: [PATCH 1/2] Computed columns broken with delta updates on non-dependent columns --- packages/perspective/test/js/constructors.js | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/perspective/test/js/constructors.js b/packages/perspective/test/js/constructors.js index a970f747b4..0d7b78dacb 100644 --- a/packages/perspective/test/js/constructors.js +++ b/packages/perspective/test/js/constructors.js @@ -362,6 +362,33 @@ module.exports = perspective => { expect(expected).toEqual(result); }); + it("Computed column of arity 2 with updates on non-dependent columns", async function() { + var meta = { + w: "float", + x: "float", + y: "string", + z: "boolean" + }; + var table = perspective.table(meta, {index: "y"}); + let table2 = table.add_computed([ + { + column: "ratio", + type: "float", + func: (w, x) => w / x, + inputs: ["w", "x"] + } + ]); + + table2.update(data_3); + + let delta_upd = [{y: "a", z: false}, {y: "b", z: true}, {y: "c", z: false}, {y: "d", z: true}]; + table2.update(delta_upd); + + let result = await table2.view({aggregate: [{op: "count", column: "y"}, {op: "count", column: "ratio"}]}).to_json(); + let expected = [{y: "a", ratio: 1.5}, {y: "b", ratio: 1.25}, {y: "c", ratio: 1.1666666666666667}, {y: "d", ratio: 1.125}]; + expect(expected).toEqual(result); + }); + it("String computed column of arity 1", async function() { var table = perspective.table(data); From b3d7a498e53ea8b6e9cce59b8e255aeab5037821 Mon Sep 17 00:00:00 2001 From: Naveen Michaud-Agrawal Date: Thu, 11 Oct 2018 00:03:10 -0400 Subject: [PATCH 2/2] Don't compute if any dependencies are null --- packages/perspective/src/cpp/main.cpp | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/perspective/src/cpp/main.cpp b/packages/perspective/src/cpp/main.cpp index dc331fe34f..76f39b3b33 100644 --- a/packages/perspective/src/cpp/main.cpp +++ b/packages/perspective/src/cpp/main.cpp @@ -780,6 +780,9 @@ sort(t_ctx2_sptr ctx2, val j_sortby) { */ val scalar_to_val(const t_tscalar scalar) { + if (!scalar.is_valid()) { + return val::null(); + } switch (scalar.get_dtype()) { case DTYPE_BOOL: { if (scalar) { @@ -934,7 +937,7 @@ table_add_computed_column(t_table_sptr table, t_str name, t_dtype dtype, val fun t_uindex size = table->size(); for (t_uindex ridx = 0; ridx < size; ++ridx) { - val value = val::null(); + val value = val::undefined(); switch (arity) { case 0: { @@ -943,20 +946,26 @@ table_add_computed_column(t_table_sptr table, t_str name, t_dtype dtype, val fun } case 1: { i1 = scalar_to_val(icols[0]->get_scalar(ridx)); - value = func(i1); + if (!i1.isNull()) { + value = func(i1); + } break; } case 2: { i1 = scalar_to_val(icols[0]->get_scalar(ridx)); i2 = scalar_to_val(icols[1]->get_scalar(ridx)); - value = func(i1, i2); + if (!i1.isNull() && !i2.isNull()) { + value = func(i1, i2); + } break; } case 3: { i1 = scalar_to_val(icols[0]->get_scalar(ridx)); i2 = scalar_to_val(icols[1]->get_scalar(ridx)); i3 = scalar_to_val(icols[2]->get_scalar(ridx)); - value = func(i1, i2, i3); + if (!i1.isNull() && !i2.isNull() && !i3.isNull()) { + value = func(i1, i2, i3); + } break; } case 4: { @@ -964,7 +973,9 @@ table_add_computed_column(t_table_sptr table, t_str name, t_dtype dtype, val fun i2 = scalar_to_val(icols[1]->get_scalar(ridx)); i3 = scalar_to_val(icols[2]->get_scalar(ridx)); i4 = scalar_to_val(icols[3]->get_scalar(ridx)); - value = func(i1, i2, i3, i4); + if (!i1.isNull() && !i2.isNull() && !i3.isNull() && !i4.isNull()) { + value = func(i1, i2, i3, i4); + } break; } default: { @@ -973,7 +984,9 @@ table_add_computed_column(t_table_sptr table, t_str name, t_dtype dtype, val fun } } - set_column_nth(out, ridx, value); + if (!value.isUndefined()) { + set_column_nth(out, ridx, value); + } } }