Fix race conditions when setting computed columns as attribute expres…

…sions
finos · Jun 2, 2020 · 8a47108 · 8a47108
1 parent 1ff5621
commit 8a47108
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 7 deletions.
diff --git a/packages/perspective-viewer/src/js/autocomplete_widget.js b/packages/perspective-viewer/src/js/autocomplete_widget.js
@@ -219,7 +219,6 @@ class PerspectiveAutocompleteWidget extends HTMLElement {
         if (idx > -1 && children.length > 0) {
             children[idx].setAttribute("aria-selected", "true");
             children[idx].scrollIntoView({
-                behavior: "smooth",
                 block: "nearest"
             });
 

diff --git a/packages/perspective-viewer/src/js/viewer/perspective_element.js b/packages/perspective-viewer/src/js/viewer/perspective_element.js
@@ -182,10 +182,10 @@ export class PerspectiveElement extends StateElement {
 
         if (parsed_computed_columns.length === 0) {
             // Fallback for race condition on workspace - need to parse
-            // computed expressions, and assume that `parsed-computed-columns`
-            // will be set when the setAttribute callback fires
-            // *after* the table has been loaded.
+            // computed expressions and then set `parsed-computed-columns`
+            // so that future views can get the computed column.
             const computed_expressions = this._get_view_computed_columns();
+
             for (const expression of computed_expressions) {
                 if (typeof expression === "string") {
                     parsed_computed_columns = parsed_computed_columns.concat(this._computed_expression_parser.parse(expression));
@@ -195,9 +195,17 @@ export class PerspectiveElement extends StateElement {
             }
         }
 
-        const computed_column_names = parsed_computed_columns.map(x => x.column);
         const computed_schema = await table.computed_schema(parsed_computed_columns);
 
+        // Validate the computed columns and make sure no invalid columns
+        // are present, as invalid columns can cause segfaults later on.
+        const validated = await this._validate_parsed_computed_columns(parsed_computed_columns, computed_schema);
+        parsed_computed_columns = validated;
+
+        // Update the viewer with the parsed computed columns
+        this.setAttribute("parsed-computed-columns", JSON.stringify(parsed_computed_columns));
+
+        const computed_column_names = parsed_computed_columns.map(x => x.column);
         cols = cols.concat(computed_column_names);
 
         if (!this.hasAttribute("columns")) {

diff --git a/packages/perspective-viewer/test/js/unit/computed_expressions_parser.spec.js b/packages/perspective-viewer/test/js/unit/computed_expressions_parser.spec.js
@@ -41,6 +41,23 @@ describe("Computed Expression Parser", function() {
         expect(parsed).toEqual(expected);
     });
 
+    it.skip("Should parse an operator notation expression with associativity", function() {
+        const expected = [
+            {
+                column: "(w + x)",
+                computed_function_name: "+",
+                inputs: ["w", "x"]
+            },
+            {
+                column: "((w + x) + z)",
+                computed_function_name: "+",
+                inputs: ["(w + x)", "z"]
+            }
+        ];
+        const parsed = COMPUTED_EXPRESSION_PARSER.parse('"w" + "x" + "z"');
+        expect(parsed).toEqual(expected);
+    });
+
     it("Should parse an operator notation expression named with 'AS'", function() {
         const expected = [
             {

diff --git a/packages/perspective-viewer/test/results/linux.docker.json b/packages/perspective-viewer/test/results/linux.docker.json
@@ -8,15 +8,15 @@
     "Computed_Expressions_Typing_a_datetime_function_should_show_autocomplete_for_datetime_columns": "d88e3c1958db28913986caa9b52bc9af",
     "Computed_Expressions_Typing_a_partial_column_name_should_show_autocomplete": "dc6bc237afed37845241281d9e7013d7",
     "Computed_Expressions_Typing_a_long_expression_should_dock_the_autocomplete": "d8bbc17948371907e494b76c1e23f2b5",
-    "Computed_Expressions_Typing_a_long_expression_should_dock_the_autocomplete,_and_the_details_panel_should_show": "35335cde3e70ad743da4b544500ccf71",
+    "Computed_Expressions_Typing_a_long_expression_should_dock_the_autocomplete,_and_the_details_panel_should_show": "62688ada39c92285ac2c1b596155d3b8",
     "Computed_Expressions_Typing_an_expression_in_the_textarea_should_work_even_when_pushed_down_to_page_bottom_": "c5f3de03cb95f581a31cdbb37acadbdf",
     "Computed_Expressions_Typing_enter_should_save_a_valid_expression": "409fb88c2373dceb480991f6f5f985fc",
     "Computed_Expressions_Typing_enter_should_not_save_an_invalid_expression": "266ffcd4766f1505bf925d7ba5ce583f",
     "Computed_Expressions_Pressing_arrow_down_should_select_the_next_autocomplete_item": "e2ba1f9f03f7e046dd2309fc04b6433e",
     "Computed_Expressions_Pressing_arrow_down_on_the_last_item_should_select_the_first_autocomplete_item": "67c477a9515f85734ba044c8f4d68eec",
     "Computed_Expressions_Pressing_arrow_up_should_select_the_previous_autocomplete_item": "67c477a9515f85734ba044c8f4d68eec",
     "Computed_Expressions_Pressing_arrow_up_from_the_first_item_should_select_the_last_autocomplete_item": "d5ac2a245dd20f99d8b1bbab292c66c6",
-    "Computed_Expressions_Pressing_arrow_down_on_an_undocked_autocomplete_should_select_the_next_autocomplete_item": "f9e63e4c0aa5307543360abbfe2e09c1",
+    "Computed_Expressions_Pressing_arrow_down_on_an_undocked_autocomplete_should_select_the_next_autocomplete_item": "32cc425a0c1426805776307befa4808c",
     "Computed_Expressions_Pressing_arrow_down_on_the_last_item_on_an_undocked_autocomplete_should_select_the_first_autocomplete_item": "a87b2a4caeb89462218838e0aa4b1c99",
     "Computed_Expressions_Pressing_arrow_up_on_an_undocked_autocomplete_should_select_the_previous_autocomplete_item": "9fb08a9f61a7624be9c5440c15d0e959",
     "Computed_Expressions_Pressing_arrow_up_from_the_first_item_on_an_undocked_autocomplete_should_select_the_last_autocomplete_item": "9fb08a9f61a7624be9c5440c15d0e959",