From 8a471082c4d60c13322cb001fac7d15a3b946730 Mon Sep 17 00:00:00 2001 From: Jun Tan Date: Tue, 2 Jun 2020 17:59:04 -0400 Subject: [PATCH] Fix race conditions when setting computed columns as attribute expressions --- .../src/js/autocomplete_widget.js | 1 - .../src/js/viewer/perspective_element.js | 16 ++++++++++++---- .../js/unit/computed_expressions_parser.spec.js | 17 +++++++++++++++++ .../test/results/linux.docker.json | 4 ++-- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/perspective-viewer/src/js/autocomplete_widget.js b/packages/perspective-viewer/src/js/autocomplete_widget.js index aad54d9abb..271f9400e6 100644 --- 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 index 1243165599..5b0f3cb209 100644 --- 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 index 6c7e74dc43..6456291cf6 100644 --- 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 index a965a483ac..3ff61fde82 100644 --- a/packages/perspective-viewer/test/results/linux.docker.json +++ b/packages/perspective-viewer/test/results/linux.docker.json @@ -8,7 +8,7 @@ "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", @@ -16,7 +16,7 @@ "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",