From 095eb2af842787219448182ee12963723333d84c Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Fri, 4 Sep 2026 21:16:51 -0400 Subject: [PATCH 1/3] Fix ExprTK coercion for operators Signed-off-by: Andrew Stein --- .../md/explanation/view/config/expressions.md | 34 +- .../test/js/expressions/functionality.spec.js | 4 +- .../test/js/expressions/numeric.spec.js | 308 ++++-------- .../test/js/expressions/type_check.spec.js | 459 ++++++++++++++++++ .../tests/table/test_view_expression.py | 120 ++++- .../src/cpp/computed_expression.cpp | 143 +++++- .../perspective/src/cpp/computed_function.cpp | 26 +- .../cpp/perspective/src/cpp/scalar.cpp | 19 + .../cpp/perspective/src/cpp/server.cpp | 65 +++ .../include/perspective/computed_expression.h | 6 + .../include/perspective/expression_compare.h | 432 +++++++++++++++++ .../src/include/perspective/exprtk.h | 249 +++++----- 12 files changed, 1495 insertions(+), 370 deletions(-) create mode 100644 rust/perspective-js/test/js/expressions/type_check.spec.js create mode 100644 rust/perspective-server/cpp/perspective/src/include/perspective/expression_compare.h diff --git a/docs/md/explanation/view/config/expressions.md b/docs/md/explanation/view/config/expressions.md index c362ddf0e6..d117b7cc0c 100644 --- a/docs/md/explanation/view/config/expressions.md +++ b/docs/md/explanation/view/config/expressions.md @@ -42,9 +42,10 @@ let view = table.view(Some(ViewConfigUpdate { ## Type Conversion and Coercion -Perspective expressions are strongly typed — each column and literal has a fixed -type, and most operators require matching types on both sides. To work across -types, use the conversion functions: +Perspective expressions are typed: every column, literal and function result has +a fixed type, and the validator reports an error before the expression is ever +computed if the types do not fit the operator. To move between types explicitly, +use the conversion functions: | Function | Description | | --------------- | ------------------------------------------------------------ | @@ -59,10 +60,29 @@ types, use the conversion functions: ### How coercion works -Perspective does not implicitly coerce types. For example, you cannot directly -add an `integer` to a `float` — you must cast one side explicitly. Similarly, -`datetime` and `date` values are not numeric: to perform arithmetic on them, you -must first convert to a numeric representation, do the math, then convert back. +Numeric types promote to each other. Arithmetic on any mix of `integer` and +`float` operands is computed in floating point and produces a `float`. The +comparison operators compare values across every numeric type: integers are +compared exactly (including signed against unsigned), and as soon as one side is +a `float` both sides are compared as doubles. Numeric literals are `float`, so +`"Quantity" > 3` works on an `integer` column without a cast. + +No other implicit coercion exists. `boolean`, `string`, `date` and `datetime` +values can only be compared with values of the same type; comparing a `string` +column to a number, a `boolean` to `1`, or a `date` to a `datetime` is a +validation error that names the operator and both types, for example +`Type Error - cannot compare string and float with '=='`. Similarly, `datetime` +and `date` values are not numeric: to perform arithmetic on them, you must first +convert to a numeric representation, do the math, then convert back. + +Boolean contexts cast instead. The condition of `if` and `? :`, and the operands +of `and`, `or`, `not`, `xor`, `nand`, `nor` and `xnor`, accept any type: `null` +is `false`, a `boolean` is its own value, a number is `true` when non-zero, and +a `string` is `true` when non-null. `x == null` and `x != null` test `x` for +null and return `boolean`, the same as `is_null(x)` and `is_not_null(x)`; the +`null` literal is otherwise a value like any null cell: `"x" > 2 ? null : "x"` +yields null in the first case, and `"x" + null` or `"x" < null` are null, exactly +as they would be for a column with a null value. Internally, `datetime` values are stored as milliseconds since the Unix epoch (1970-01-01T00:00:00Z). Converting a `datetime` to a `float` yields this diff --git a/rust/perspective-js/test/js/expressions/functionality.spec.js b/rust/perspective-js/test/js/expressions/functionality.spec.js index 685c7f895b..9b20440e91 100644 --- a/rust/perspective-js/test/js/expressions/functionality.spec.js +++ b/rust/perspective-js/test/js/expressions/functionality.spec.js @@ -124,7 +124,7 @@ import perspective from "../perspective_client"; table.delete(); }); - test.skip("functional if bool", async function () { + test("functional if bool", async function () { const table = await perspective.table( expressions_common.int_float_data, ); @@ -136,7 +136,7 @@ import perspective from "../perspective_client"; }); const results = await view.to_columns(); - expect(results['if ("z" == 1, 5, 10);']).toEqual([ + expect(results['if ("z" == true, 5, 10);']).toEqual([ 5, 10, 5, 10, ]); expect(results['if ("z" != true, 5, 10)']).toEqual([ diff --git a/rust/perspective-js/test/js/expressions/numeric.spec.js b/rust/perspective-js/test/js/expressions/numeric.spec.js index 34895cc2e6..d00e74aba1 100644 --- a/rust/perspective-js/test/js/expressions/numeric.spec.js +++ b/rust/perspective-js/test/js/expressions/numeric.spec.js @@ -112,6 +112,43 @@ function validate_unary_operations(output, expressions, operator) { } } +function calc_comparison(operator, left, right) { + switch (operator) { + case "==": + return left == right; + case "!=": + return left != right; + case ">": + return left > right; + case "<": + return left < right; + case ">=": + return left >= right; + case "<=": + return left <= right; + default: + throw new Error("Unknown operator"); + } +} + +function validate_comparison_operations(output, expressions, operator) { + for (const expr of expressions) { + const output_col = output[expr]; + const inputs = expr.split(` ${operator} `); + const left = inputs[0].substr(1, inputs[0].length - 2); + const right = inputs[1].substr(1, inputs[1].length - 2); + expect({ + expr, + result: output_col, + }).toEqual({ + expr, + result: output[left].map((v, idx) => + calc_comparison(operator, v, output[right][idx]), + ), + }); + } +} + /** * Validate the results of operations against all numeric types. * @@ -344,223 +381,61 @@ function validate_binary_operations(output, expressions, operator) { validate_binary_operations(result, expressions, "^"); }); - test("==", async function () { - const table = await perspective.table( - common.all_types_multi_arrow.slice(), - ); - const expressions = []; - - // comparisons only work when the two types are the same - for (const expr of NUMERIC_TYPES) { - expressions.push(`"${expr}" == "${expr} 2"`); - } - - const view = await table.view({ - expressions, - }); - - const col_names = await view.column_paths(); - - for (const expr of expressions) { - expect(col_names.includes(expr)).toBe(true); - } - - const result = await view.to_columns(); - - for (const expr of expressions) { - const output_col = result[expr]; - const inputs = expr.split(` == `); - const left = inputs[0].substr(1, inputs[0].length - 2); - const right = inputs[1].substr(1, inputs[1].length - 2); - expect(output_col).toEqual( - result[left].map( - (v, idx) => v == result[right][idx], - ), + for (const operator of ["==", "!=", ">", "<", ">=", "<="]) { + test(operator, async function () { + const table = await perspective.table( + common.arrow.slice(), ); - } - }); - - test("!=", async function () { - const table = await perspective.table( - common.all_types_multi_arrow.slice(), - ); - const expressions = []; - - // comparisons only work when the two types are the same - for (const expr of NUMERIC_TYPES) { - expressions.push(`"${expr}" != "${expr} 2"`); - } - - const view = await table.view({ - expressions, - }); + const expressions = + generate_binary_operations(operator); + const view = await table.view({ + expressions, + }); - const col_names = await view.column_paths(); + const col_names = await view.column_paths(); - for (const expr of expressions) { - expect(col_names.includes(expr)).toBe(true); - } + for (const expr of expressions) { + expect(col_names.includes(expr)).toBe(true); + } - const result = await view.to_columns(); - - for (const expr of expressions) { - const output_col = result[expr]; - const inputs = expr.split(` != `); - const left = inputs[0].substr(1, inputs[0].length - 2); - const right = inputs[1].substr(1, inputs[1].length - 2); - expect(output_col).toEqual( - result[left].map( - (v, idx) => v != result[right][idx], - ), + const result = await view.to_columns(); + validate_comparison_operations( + result, + expressions, + operator, ); - } - }); - - test(">", async function () { - const table = await perspective.table( - common.all_types_multi_arrow.slice(), - ); - const expressions = []; - - // comparisons only work when the two types are the same - for (const expr of NUMERIC_TYPES) { - expressions.push(`"${expr}" > "${expr} 2"`); - } - - const view = await table.view({ - expressions, + await view.delete(); + await table.delete(); }); + } - const col_names = await view.column_paths(); - - for (const expr of expressions) { - expect(col_names.includes(expr)).toBe(true); - } - + test("Unsigned columns compare against negative literals", async function () { + const table = await perspective.table(common.arrow.slice()); + const expressions = { + gt: '"ui64" > -1', + lt: '"ui32" < -1', + same: '"i64" >= "ui64"', + float: '"f32" == "i32"', + literal: '"ui8" <= 13', + }; + const view = await table.view({ expressions }); const result = await view.to_columns(); - - for (const expr of expressions) { - const output_col = result[expr]; - const inputs = expr.split(` > `); - const left = inputs[0].substr(1, inputs[0].length - 2); - const right = inputs[1].substr(1, inputs[1].length - 2); - expect(output_col).toEqual( - result[left].map( - (v, idx) => v > result[right][idx], - ), - ); - } - }); - - test("<", async function () { - const table = await perspective.table( - common.all_types_multi_arrow.slice(), + expect(result.gt).toEqual(result.ui64.map(() => true)); + expect(result.lt).toEqual(result.ui32.map(() => false)); + expect(result.same).toEqual( + result.i64.map((v, idx) => v >= result.ui64[idx]), ); - const expressions = []; - - // comparisons only work when the two types are the same - for (const expr of NUMERIC_TYPES) { - expressions.push(`"${expr}" < "${expr} 2"`); - } - - const view = await table.view({ - expressions, - }); - - const col_names = await view.column_paths(); - - for (const expr of expressions) { - expect(col_names.includes(expr)).toBe(true); - } - - const result = await view.to_columns(); - - for (const expr of expressions) { - const output_col = result[expr]; - const inputs = expr.split(` < `); - const left = inputs[0].substr(1, inputs[0].length - 2); - const right = inputs[1].substr(1, inputs[1].length - 2); - expect(output_col).toEqual( - result[left].map( - (v, idx) => v < result[right][idx], - ), - ); - } - }); - - test(">=", async function () { - const table = await perspective.table( - common.all_types_multi_arrow.slice(), + expect(result.float).toEqual( + result.f32.map((v, idx) => v == result.i32[idx]), ); - const expressions = []; - - // comparisons only work when the two types are the same - for (const expr of NUMERIC_TYPES) { - expressions.push(`"${expr}" >= "${expr} 2"`); - } - - const view = await table.view({ - expressions, - }); - - const col_names = await view.column_paths(); - - for (const expr of expressions) { - expect(col_names.includes(expr)).toBe(true); - } - - const result = await view.to_columns(); - - for (const expr of expressions) { - const output_col = result[expr]; - const inputs = expr.split(` >= `); - const left = inputs[0].substr(1, inputs[0].length - 2); - const right = inputs[1].substr(1, inputs[1].length - 2); - expect(output_col).toEqual( - result[left].map( - (v, idx) => v >= result[right][idx], - ), - ); - } - }); - - test("<=", async function () { - const table = await perspective.table( - common.all_types_multi_arrow.slice(), + expect(result.literal).toEqual( + result.ui8.map((v) => v <= 13), ); - const expressions = []; - - // comparisons only work when the two types are the same - for (const expr of NUMERIC_TYPES) { - expressions.push(`"${expr}" <= "${expr} 2"`); - } - - const view = await table.view({ - expressions, - }); - - const col_names = await view.column_paths(); - - for (const expr of expressions) { - expect(col_names.includes(expr)).toBe(true); - } - - const result = await view.to_columns(); - - for (const expr of expressions) { - const output_col = result[expr]; - const inputs = expr.split(` <= `); - const left = inputs[0].substr(1, inputs[0].length - 2); - const right = inputs[1].substr(1, inputs[1].length - 2); - expect(output_col).toEqual( - result[left].map( - (v, idx) => v <= result[right][idx], - ), - ); - } + await view.delete(); + await table.delete(); }); - test("Numeric comparisons should be false for different types", async function () { + test("Numeric comparisons promote across types", async function () { const table = await perspective.table({ a: "integer", b: "float", @@ -571,26 +446,33 @@ function validate_binary_operations(output, expressions, operator) { expressions: { '"a" == "b"': '"a" == "b"', '"a" != "b"': '"a" != "b"', + '"c" < "d"': '"c" < "d"', }, }); await table.update({ a: [1, 2, 3, 4], - b: [1.0, 2.0, 3.0, 4.0], + b: [1.0, 2.0, 3.5, 4.0], c: [1, 0, 1, 0], d: [1.0, 0, 3.0, 0.0], }); const result = await view.to_columns(); expect(result['"a" == "b"']).toEqual([ + true, + true, false, + true, + ]); + expect(result['"a" != "b"']).toEqual([ false, false, + true, false, ]); - expect(result['"a" != "b"']).toEqual([ - true, - true, - true, + expect(result['"c" < "d"']).toEqual([ + false, + false, true, + false, ]); await view.delete(); await table.delete(); @@ -1615,6 +1497,7 @@ function validate_binary_operations(output, expressions, operator) { e: [false, false, false, false], f: [true, true, true, true], }); + const view = await table.view({ expressions: { '"a" or "b"': '"a" or "b"', @@ -1626,6 +1509,7 @@ function validate_binary_operations(output, expressions, operator) { filtered: '"a" > 0.5 or "d" < 0.5', }, }); + const result = await view.to_columns(); expect(result['"a" or "b"']).toEqual([ false, @@ -1633,6 +1517,7 @@ function validate_binary_operations(output, expressions, operator) { false, true, ]); + expect(result['"c" or "d"']).toEqual([true, true, true, true]); expect(result['"e" or "f"']).toEqual([true, true, true, true]); expect(result["0 or 1"]).toEqual([true, true, true, true]); @@ -1642,13 +1527,16 @@ function validate_binary_operations(output, expressions, operator) { true, true, ]); + expect(result["false or false"]).toEqual([ false, false, false, false, ]); - expect(result["filtered"]).toEqual([true, true, true, true]); + + expect(result["filtered"]).toEqual([false, true, false, true]); + await view.delete(); await table.delete(); }); diff --git a/rust/perspective-js/test/js/expressions/type_check.spec.js b/rust/perspective-js/test/js/expressions/type_check.spec.js new file mode 100644 index 0000000000..be5384cd1f --- /dev/null +++ b/rust/perspective-js/test/js/expressions/type_check.spec.js @@ -0,0 +1,459 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +import { test, expect } from "@perspective-dev/test"; +import perspective from "../perspective_client"; + +async function mixed_table(perspective) { + const table = await perspective.table({ + a: "integer", + b: "float", + c: "string", + d: "boolean", + e: "date", + f: "datetime", + }); + await table.update({ + a: [1, 2, 3, 4], + b: [1.0, 2.5, 3.0, 4.5], + c: ["x", "y", "z", "w"], + d: [true, false, true, false], + e: [ + new Date(2020, 0, 1), + new Date(2020, 0, 2), + new Date(2020, 0, 3), + new Date(2020, 0, 4), + ], + f: [ + new Date(2020, 0, 1, 1), + new Date(2020, 0, 2, 1), + new Date(2020, 0, 3, 1), + new Date(2020, 0, 4, 1), + ], + }); + return table; +} + +((perspective) => { + test.describe("Expression type checking", function () { + test.describe("Numeric promotion", function () { + test("Integer columns compare against numeric literals", async function () { + const table = await mixed_table(perspective); + const expressions = { + lt: '"a" < 3', + lte: '"a" <= 3', + gt: '"a" > 3', + gte: '"a" >= 3', + eq: '"a" == 3', + ne: '"a" != 3', + }; + const validated = await table.validate_expressions(expressions); + expect(validated.errors).toEqual({}); + expect(validated.expression_schema).toEqual({ + lt: "boolean", + lte: "boolean", + gt: "boolean", + gte: "boolean", + eq: "boolean", + ne: "boolean", + }); + + const view = await table.view({ expressions }); + const result = await view.to_columns(); + expect(result.lt).toEqual([true, true, false, false]); + expect(result.lte).toEqual([true, true, true, false]); + expect(result.gt).toEqual([false, false, false, true]); + expect(result.gte).toEqual([false, false, true, true]); + expect(result.eq).toEqual([false, false, true, false]); + expect(result.ne).toEqual([true, true, false, true]); + await view.delete(); + await table.delete(); + }); + + test("Integer and float columns compare by value", async function () { + const table = await mixed_table(perspective); + const view = await table.view({ + expressions: { + eq: '"a" == "b"', + ne: '"a" != "b"', + lt: '"a" < "b"', + gte: '"b" >= "a"', + }, + }); + const result = await view.to_columns(); + expect(result.eq).toEqual([true, false, true, false]); + expect(result.ne).toEqual([false, true, false, true]); + expect(result.lt).toEqual([false, true, false, true]); + expect(result.gte).toEqual([true, true, true, true]); + await view.delete(); + await table.delete(); + }); + + test("Conditionals branch on integer comparisons", async function () { + const table = await mixed_table(perspective); + const view = await table.view({ + expressions: { + cond: 'if ("a" < 3) 10; else 100', + tern: '"a" == 2 ? 1 : 0', + }, + }); + const result = await view.to_columns(); + expect(result.cond).toEqual([10, 10, 100, 100]); + expect(result.tern).toEqual([0, 1, 0, 0]); + await view.delete(); + await table.delete(); + }); + + test("integer() casts compare against integer columns", async function () { + const table = await mixed_table(perspective); + const view = await table.view({ + expressions: { + eq: '"a" == integer(3)', + lt: 'integer("b") < "a"', + }, + }); + const result = await view.to_columns(); + expect(result.eq).toEqual([false, false, true, false]); + expect(result.lt).toEqual([false, false, false, false]); + await view.delete(); + await table.delete(); + }); + + test("inrange promotes numeric bounds", async function () { + const table = await mixed_table(perspective); + const view = await table.view({ + expressions: { + r: 'inrange(2, "a", 3)', + s: 'inrange("a", "b", 3)', + }, + }); + const result = await view.to_columns(); + expect(result.r).toEqual([false, true, true, false]); + expect(result.s).toEqual([true, true, true, false]); + await view.delete(); + await table.delete(); + }); + + test("Null integers compare like nulls", async function () { + const table = await perspective.table({ a: "integer" }); + await table.update({ a: [1, null, 3, null] }); + const view = await table.view({ + expressions: { + eq: '"a" == 1', + ne: '"a" != 1', + lt: '"a" < 2', + isnull: '"a" == null', + notnull: '"a" != null', + }, + }); + const result = await view.to_columns(); + expect(result.eq).toEqual([true, false, false, false]); + expect(result.ne).toEqual([false, true, true, true]); + expect(result.lt).toEqual([true, false, false, false]); + expect(result.isnull).toEqual([false, true, false, true]); + expect(result.notnull).toEqual([true, false, true, false]); + await view.delete(); + await table.delete(); + }); + }); + + test.describe("Type errors", function () { + test("Comparing incompatible types is a validation error", async function () { + const table = await mixed_table(perspective); + const validated = await table.validate_expressions({ + str_num: '"c" == 1', + bool_num: '"d" < 1', + date_datetime: '"e" == "f"', + num_str: "\"a\" == 'x'", + bool_str: '"d" == "c"', + str_gte: '"c" >= "a"', + }); + expect(validated.expression_schema).toEqual({}); + expect(validated.errors).toEqual({ + str_num: { + error_message: + "Type Error - cannot compare string and float with '=='", + line: 0, + column: 8, + }, + bool_num: { + error_message: + "Type Error - cannot compare boolean and float with '<'", + line: 0, + column: 8, + }, + date_datetime: { + error_message: + "Type Error - cannot compare date and datetime with '=='", + line: 0, + column: 8, + }, + num_str: { + error_message: + "Type Error - cannot compare integer and string with '=='", + line: 0, + column: 8, + }, + bool_str: { + error_message: + "Type Error - cannot compare boolean and string with '=='", + line: 0, + column: 8, + }, + str_gte: { + error_message: + "Type Error - cannot compare string and integer with '>='", + line: 0, + column: 8, + }, + }); + await table.delete(); + }); + + test("Type errors report the offending operator position", async function () { + const table = await mixed_table(perspective); + const validated = await table.validate_expressions({ + same_line: '"a" == 1 and "c" == 2', + next_line: '"a" == 1 and\n"c" == 2', + }); + expect(validated.expression_schema).toEqual({}); + expect(validated.errors).toEqual({ + same_line: { + error_message: + "Type Error - cannot compare string and float with '=='", + line: 0, + column: 25, + }, + next_line: { + error_message: + "Type Error - cannot compare string and float with '=='", + line: 1, + column: 8, + }, + }); + await table.delete(); + }); + + test("Incompatible inrange bounds are a validation error", async function () { + const table = await mixed_table(perspective); + const validated = await table.validate_expressions({ + r: 'inrange(1, "c", 3)', + }); + expect(validated.expression_schema).toEqual({}); + expect(validated.errors).toEqual({ + r: { + error_message: + "Type Error - cannot compare float and string with '<='", + line: 0, + column: 0, + }, + }); + await table.delete(); + }); + + test("Legacy type errors keep the generic message", async function () { + const table = await mixed_table(perspective); + const validated = await table.validate_expressions({ + x: '"c" + 1', + y: 'if ("a" > 1) 5', + }); + expect(validated.expression_schema).toEqual({}); + expect(validated.errors).toEqual({ + x: { + error_message: + "Type Error - inputs do not resolve to a valid expression.", + line: 0, + column: 0, + }, + y: { + error_message: + "Type Error - inputs do not resolve to a valid expression.", + line: 0, + column: 0, + }, + }); + await table.delete(); + }); + }); + + test.describe("Boolean contexts", function () { + test("Conditions cast non-boolean values", async function () { + const table = await perspective.table({ + a: "integer", + c: "string", + d: "boolean", + }); + await table.update({ + a: [1, null, 0, 4], + c: ["x", null, "", "w"], + d: [true, null, false, true], + }); + const expressions = { + cond: 'if ("a") 10; else 100', + tern: '"c" ? 1 : 0', + bool: 'if ("d") 1; else 0', + }; + const validated = await table.validate_expressions(expressions); + expect(validated.errors).toEqual({}); + expect(validated.expression_schema).toEqual({ + cond: "float", + tern: "float", + bool: "float", + }); + const view = await table.view({ expressions }); + const result = await view.to_columns(); + expect(result.cond).toEqual([10, 100, 100, 10]); + expect(result.tern).toEqual([1, 0, 1, 1]); + expect(result.bool).toEqual([1, 0, 0, 1]); + await view.delete(); + await table.delete(); + }); + + test("Logical operators cast operands", async function () { + const table = await perspective.table({ + a: "integer", + c: "string", + d: "boolean", + }); + await table.update({ + a: [1, null, 0, 4], + c: ["x", null, "", "w"], + d: [true, null, false, true], + }); + const expressions = { + and: '"a" and "d"', + or: '"c" or "d"', + not_int: 'not("a")', + not_bool: 'not("d")', + xor: '"a" xor "c"', + }; + const validated = await table.validate_expressions(expressions); + expect(validated.errors).toEqual({}); + expect(validated.expression_schema).toEqual({ + and: "boolean", + or: "boolean", + not_int: "boolean", + not_bool: "boolean", + xor: "boolean", + }); + const view = await table.view({ expressions }); + const result = await view.to_columns(); + expect(result.and).toEqual([true, false, false, true]); + expect(result.or).toEqual([true, false, true, true]); + expect(result.not_int).toEqual([false, true, true, false]); + expect(result.not_bool).toEqual([false, true, true, false]); + expect(result.xor).toEqual([false, false, true, false]); + await view.delete(); + await table.delete(); + }); + + test("Logical operators on boolean columns", async function () { + const table = await mixed_table(perspective); + const view = await table.view({ + expressions: { + and: '"d" and not("d")', + or: '"d" or not("d")', + not: 'not("d")', + xor: '"d" xor ("a" > 2)', + cond: 'if ("d") 1; else 0', + }, + }); + const result = await view.to_columns(); + expect(result.and).toEqual([false, false, false, false]); + expect(result.or).toEqual([true, true, true, true]); + expect(result.not).toEqual([false, true, false, true]); + expect(result.xor).toEqual([true, false, false, true]); + expect(result.cond).toEqual([1, 0, 1, 0]); + await view.delete(); + await table.delete(); + }); + + test("Null equality is boolean", async function () { + const table = await perspective.table({ + c: "string", + d: "boolean", + }); + await table.update({ + c: ["x", null, "null", null], + d: [true, null, false, null], + }); + const expressions = { + isnull: '"c" == null', + notnull: '"c" != null', + reversed: 'null == "c"', + bool_null: '"d" == null', + literal: "'a' == null", + both: "null == null", + text: "\"c\" == 'null'", + branch: 'if ("c" == null) 1; else 0', + }; + const validated = await table.validate_expressions(expressions); + expect(validated.errors).toEqual({}); + expect(validated.expression_schema).toEqual({ + isnull: "boolean", + notnull: "boolean", + reversed: "boolean", + bool_null: "boolean", + literal: "boolean", + both: "boolean", + text: "boolean", + branch: "float", + }); + const view = await table.view({ expressions }); + const result = await view.to_columns(); + expect(result.isnull).toEqual([false, true, false, true]); + expect(result.notnull).toEqual([true, false, true, false]); + expect(result.reversed).toEqual([false, true, false, true]); + expect(result.bool_null).toEqual([false, true, false, true]); + expect(result.literal).toEqual([false, false, false, false]); + expect(result.both).toEqual([true, true, true, true]); + expect(result.text).toEqual([false, false, true, false]); + expect(result.branch).toEqual([0, 1, 0, 1]); + await view.delete(); + await table.delete(); + }); + + test("Null as a value, in arithmetic and in ordering", async function () { + const table = await perspective.table({ a: "integer" }); + await table.update({ a: [1, null, 3, null] }); + const expressions = { + value: '"a" > 2 ? null : "a"', + lt: '"a" < null', + add: '"a" + null', + mul: "null * 2", + abs: "abs(null)", + pow: 'pow("a", null)', + }; + const validated = await table.validate_expressions(expressions); + expect(validated.errors).toEqual({}); + expect(validated.expression_schema).toEqual({ + value: "integer", + lt: "boolean", + add: "float", + mul: "float", + abs: "float", + pow: "float", + }); + const view = await table.view({ expressions }); + const result = await view.to_columns(); + expect(result.value).toEqual([1, null, null, null]); + expect(result.lt).toEqual([null, null, null, null]); + expect(result.add).toEqual([null, null, null, null]); + expect(result.mul).toEqual([null, null, null, null]); + expect(result.abs).toEqual([null, null, null, null]); + expect(result.pow).toEqual([null, null, null, null]); + await view.delete(); + await table.delete(); + }); + }); + }); +})(perspective); diff --git a/rust/perspective-python/perspective/tests/table/test_view_expression.py b/rust/perspective-python/perspective/tests/table/test_view_expression.py index 06bf0832d0..2849cf5cb0 100644 --- a/rust/perspective-python/perspective/tests/table/test_view_expression.py +++ b/rust/perspective-python/perspective/tests/table/test_view_expression.py @@ -336,17 +336,131 @@ def test_view_expression_string_literal_compare_null(self): table = Table({"a": [1, 2, 3, 4], "b": [5, 6, 7, 8]}) validated = table.validate_expressions({"computed": " 'a' == null"}) - assert validated["expression_schema"] == {"computed": "float"} + assert validated["expression_schema"] == {"computed": "boolean"} view = table.view(expressions={"computed": " 'a' == null"}) assert view.to_columns() == { "a": [1, 2, 3, 4], "b": [5, 6, 7, 8], - "computed": [0, 0, 0, 0], + "computed": [False, False, False, False], } - assert view.expression_schema() == {"computed": "float"} + assert view.expression_schema() == {"computed": "boolean"} + + def test_view_expression_column_compare_null(self): + table = Table({"a": [1, None, 3, None], "b": ["null", None, "x", None]}) + expressions = { + "isnull": '"a" == null', + "notnull": '"a" != null', + "reversed": 'null != "b"', + "text": "\"b\" == 'null'", + } + validated = table.validate_expressions(expressions) + assert validated["expression_schema"] == { + "isnull": "boolean", + "notnull": "boolean", + "reversed": "boolean", + "text": "boolean", + } + view = table.view(expressions=expressions) + assert view.to_columns() == { + "a": [1, None, 3, None], + "b": ["null", None, "x", None], + "isnull": [False, True, False, True], + "notnull": [True, False, True, False], + "reversed": [True, False, True, False], + "text": [True, False, False, False], + } + + def test_view_expression_integer_literal_comparison(self): + table = Table({"a": [1, 2, 3, 4], "b": [1.5, 2.5, 3.5, 4.5]}) + expressions = { + "lt": '"a" < 3', + "lte": '"a" <= 3', + "gt": '"a" > 3', + "gte": '"a" >= 3', + "eq": '"a" == 3', + "ne": '"a" != 3', + "cross": '"a" < "b"', + "cast": '"a" == integer(3)', + "cond": 'if ("a" < 3) 10; else 100', + } + validated = table.validate_expressions(expressions) + assert validated["errors"] == {} + assert validated["expression_schema"] == { + "lt": "boolean", + "lte": "boolean", + "gt": "boolean", + "gte": "boolean", + "eq": "boolean", + "ne": "boolean", + "cross": "boolean", + "cast": "boolean", + "cond": "float", + } + view = table.view(expressions=expressions) + assert view.to_columns() == { + "a": [1, 2, 3, 4], + "b": [1.5, 2.5, 3.5, 4.5], + "lt": [True, True, False, False], + "lte": [True, True, True, False], + "gt": [False, False, False, True], + "gte": [False, False, True, True], + "eq": [False, False, True, False], + "ne": [True, True, False, True], + "cross": [True, True, True, True], + "cast": [False, False, True, False], + "cond": [10, 10, 100, 100], + } + + def test_view_expression_incompatible_comparison_is_type_error(self): + table = Table({"a": [1, 2, 3, 4], "b": ["x", "y", "z", "w"]}) + validated = table.validate_expressions( + { + "str_num": '"b" == 1', + "num_str": "\"a\" < 'x'", + } + ) + assert validated["expression_schema"] == {} + assert validated["errors"] == { + "str_num": { + "column": 8, + "error_message": "Type Error - cannot compare string and float with '=='", + "line": 0, + }, + "num_str": { + "column": 8, + "error_message": "Type Error - cannot compare integer and string with '<'", + "line": 0, + }, + } + + def test_view_expression_boolean_operators_cast(self): + table = Table({"a": [1, None, 0, 4], "b": ["x", None, "", "w"]}) + expressions = { + "cond": 'if ("a") 10; else 100', + "tern": '"b" ? 1 : 0', + "logical": '"a" and "b"', + "negated": 'not("a")', + } + validated = table.validate_expressions(expressions) + assert validated["errors"] == {} + assert validated["expression_schema"] == { + "cond": "float", + "tern": "float", + "logical": "boolean", + "negated": "boolean", + } + view = table.view(expressions=expressions) + assert view.to_columns() == { + "a": [1, None, 0, 4], + "b": ["x", None, "", "w"], + "cond": [10, 100, 100, 10], + "tern": [1, 0, 1, 1], + "logical": [True, False, False, True], + "negated": [False, True, True, False], + } def test_view_expression_string_literal_compare_column(self): table = Table({"a": ["a", "a", "b", "c"]}) diff --git a/rust/perspective-server/cpp/perspective/src/cpp/computed_expression.cpp b/rust/perspective-server/cpp/perspective/src/cpp/computed_expression.cpp index 882e398d98..c92e6495cc 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/computed_expression.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/computed_expression.cpp @@ -13,6 +13,7 @@ #include #include +#include #include namespace perspective { @@ -104,6 +105,98 @@ struct t_validation_check_guard { exprtk::parser& m_parser; }; +/** + * @brief Best-effort line/column of a recorded type error within the parsed + * expression, from the first operator token whose operands carry the + * offending dtypes. + */ +bool +locate_type_error( + const expr::t_expression_type_error& type_error, + const std::string& parsed_expression_string, + const std::function& symbol_dtype, + t_expression_error& error +) { + exprtk::lexer::generator lexer; + if (!lexer.process(parsed_expression_string)) { + return false; + } + + const std::size_t npos = std::numeric_limits::max(); + const std::size_t num_tokens = lexer.size(); + std::size_t fallback = npos; + std::size_t position = npos; + + auto token_dtype = [&](const exprtk::lexer::token& tok) -> t_dtype { + if (tok.type == exprtk::lexer::token::e_number) { + return DTYPE_FLOAT64; + } + if (tok.type == exprtk::lexer::token::e_symbol) { + return symbol_dtype(tok.value); + } + return DTYPE_NONE; + }; + + auto prev_operand = [&](std::size_t idx) -> t_dtype { + while (idx > 0) { + const exprtk::lexer::token& tok = lexer[idx - 1]; + if (tok.type != exprtk::lexer::token::e_rbracket) { + return token_dtype(tok); + } + --idx; + } + return DTYPE_NONE; + }; + + auto next_operand = [&](std::size_t idx) -> t_dtype { + while (idx + 1 < num_tokens) { + const exprtk::lexer::token& tok = lexer[idx + 1]; + if (tok.type != exprtk::lexer::token::e_lbracket) { + return token_dtype(tok); + } + ++idx; + } + return DTYPE_NONE; + }; + + for (std::size_t i = 0; i < num_tokens; ++i) { + const exprtk::lexer::token& tok = lexer[i]; + if (tok.value != type_error.m_op) { + continue; + } + + if (fallback == npos) { + fallback = tok.position; + } + + if (prev_operand(i) == type_error.m_lhs + && next_operand(i) == type_error.m_rhs) { + position = tok.position; + break; + } + } + + if (position == npos) { + position = fallback; + } + + if (position == npos) { + return false; + } + + exprtk::parser_error::type parser_error; + parser_error.token.position = position; + if (!exprtk::parser_error::update_error( + parser_error, parsed_expression_string + )) { + return false; + } + + error.m_line = parser_error.line_no; + error.m_column = parser_error.column_no; + return true; +} + } // namespace computed_function::bucket t_computed_expression_parser::BUCKET_FN = @@ -174,6 +267,8 @@ t_tscalar t_computed_expression_parser::TRUE_SCALAR = mktscalar(true); t_tscalar t_computed_expression_parser::FALSE_SCALAR = mktscalar(false); +t_tscalar t_computed_expression_parser::NONE_SCALAR = mknone(); + /****************************************************************************** * * t_computed_expression @@ -467,7 +562,12 @@ t_computed_expression_parser::precompute( PSP_COMPLAIN_AND_ABORT(ss.str()); } - t_tscalar v = expr_definition.value(); + expr::t_expression_type_check_sink type_errors; + t_tscalar v; + { + const expr::t_expression_type_check_scope type_check_scope(type_errors); + v = expr_definition.value(); + } function_store.clear_computed_function_state(); if (vector_check.m_violation || loop_check.m_violation) { @@ -482,6 +582,14 @@ t_computed_expression_parser::precompute( PSP_COMPLAIN_AND_ABORT(ss.str()); } + if (!type_errors.m_errors.empty()) { + std::stringstream ss; + ss << "[t_computed_expression_parser::precompute] " + << expr::describe_type_error(type_errors.m_errors.front()) + << " in expression: `" << parsed_expression_string << "`\n"; + PSP_COMPLAIN_AND_ABORT(ss.str()); + } + return std::make_shared( expression_alias, expression_string, @@ -596,7 +704,12 @@ t_computed_expression_parser::get_dtype( return DTYPE_NONE; } - t_tscalar v = expr_definition.value(); + expr::t_expression_type_check_sink type_errors; + t_tscalar v; + { + const expr::t_expression_type_check_scope type_check_scope(type_errors); + v = expr_definition.value(); + } t_dtype dtype = v.get_dtype(); function_store.clear_computed_function_state(); @@ -616,6 +729,31 @@ t_computed_expression_parser::get_dtype( return DTYPE_NONE; } + if (!type_errors.m_errors.empty()) { + const expr::t_expression_type_error& type_error = + type_errors.m_errors.front(); + error.m_error_message = expr::describe_type_error(type_error); + error.m_line = 0; + error.m_column = 0; + + auto symbol_dtype = [&](const std::string& symbol) -> t_dtype { + if (symbol == "True" || symbol == "False") { + return DTYPE_BOOL; + } + for (const auto& column_id : column_ids) { + if (column_id.first == symbol) { + return schema.get_dtype(column_id.second); + } + } + return DTYPE_NONE; + }; + + locate_type_error( + type_error, parsed_expression_string, symbol_dtype, error + ); + return DTYPE_NONE; + } + if (v.m_status == STATUS_CLEAR || dtype == DTYPE_NONE) { error.m_error_message = "Type Error - inputs do not resolve to a valid expression."; @@ -802,6 +940,7 @@ t_computed_function_store::register_computed_functions( // And scalar constants sym_table.add_constant("True", t_computed_expression_parser::TRUE_SCALAR); sym_table.add_constant("False", t_computed_expression_parser::FALSE_SCALAR); + sym_table.add_constant("None", t_computed_expression_parser::NONE_SCALAR); } void diff --git a/rust/perspective-server/cpp/perspective/src/cpp/computed_function.cpp b/rust/perspective-server/cpp/perspective/src/cpp/computed_function.cpp index 0512039df6..c5a38ed825 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/computed_function.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/computed_function.cpp @@ -1589,34 +1589,10 @@ inrange_fn::~inrange_fn() = default; t_tscalar inrange_fn::operator()(t_parameter_list parameters) { - t_tscalar rval; - rval.clear(); - rval.m_type = DTYPE_BOOL; - t_scalar_view _low(parameters[0]); t_scalar_view _val(parameters[1]); t_scalar_view _high(parameters[2]); - - t_tscalar low = _low(); - t_tscalar val = _val(); - t_tscalar high = _high(); - - // make sure we are comparing items of the same type, otherwise - // comparisons will fail. - t_dtype val_dtype = val.get_dtype(); - - if (low.get_dtype() != val_dtype || val_dtype != high.get_dtype()) { - rval.m_status = STATUS_CLEAR; - return rval; - } - - // no need to type check - just check validity - if (!low.is_valid() || !val.is_valid() || !high.is_valid()) { - return rval; - } - - rval.set((low <= val) && (val <= high)); - return rval; + return expr::inrange(_low(), _val(), _high()); } min_fn::min_fn() = default; diff --git a/rust/perspective-server/cpp/perspective/src/cpp/scalar.cpp b/rust/perspective-server/cpp/perspective/src/cpp/scalar.cpp index b096eab424..1f8f0301e0 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/scalar.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/scalar.cpp @@ -37,6 +37,9 @@ operator>(const std::size_t& lhs, const t_tscalar& rhs) { t_tscalar rval; \ rval.clear(); \ rval.m_type = DTYPE_FLOAT64; \ + if (is_none() || other.is_none()) { \ + return rval; \ + } \ if (!is_numeric() || !other.is_numeric()) { \ rval.m_status = STATUS_CLEAR; \ } \ @@ -146,6 +149,10 @@ t_tscalar::operator+() const { rval.clear(); rval.m_type = m_type; + if (is_none()) { + return rval; + } + if (!is_numeric()) { rval.m_status = STATUS_CLEAR; } @@ -198,6 +205,10 @@ t_tscalar::operator-() const { rval.clear(); rval.m_type = m_type; + if (is_none()) { + return rval; + } + if (!is_numeric()) { rval.m_status = STATUS_CLEAR; } @@ -258,6 +269,10 @@ t_tscalar t_tscalar::operator/(const t_tscalar& other) const { rval.clear(); rval.m_type = DTYPE_FLOAT64; + if (is_none() || other.is_none()) { + return rval; + } + if (!is_numeric() || !other.is_numeric()) { rval.m_status = STATUS_CLEAR; } @@ -281,6 +296,10 @@ t_tscalar::operator%(const t_tscalar& other) const { rval.clear(); rval.m_type = DTYPE_FLOAT64; + if (is_none() || other.is_none()) { + return rval; + } + if (!is_numeric() || !other.is_numeric()) { rval.m_status = STATUS_CLEAR; } diff --git a/rust/perspective-server/cpp/perspective/src/cpp/server.cpp b/rust/perspective-server/cpp/perspective/src/cpp/server.cpp index 92530efd17..e3b5d19b0b 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/server.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/server.cpp @@ -25,6 +25,7 @@ #include "perspective/view.h" #include "perspective/view_config.h" #include "re2/re2.h" +#include #include #include #include @@ -345,6 +346,66 @@ re_column_name_to_id(std::string&& expression, ValidatedExpr& validated_expr) { return std::tuple(parsed_expression_string, column_id_map); } +/** + * @brief Rewrite the bare `null` keyword (outside string literals) to the + * `None` symbol-table constant, so ExprTK never applies its own `null` rules. + */ +static std::string +re_null_literal(std::string&& expression) { + static const std::string keyword = "null"; + static const std::string replacement = "None"; + std::string out; + out.reserve(expression.size()); + bool in_string = false; + std::size_t i = 0; + + auto is_word = [](char c) { + return (std::isalnum(static_cast(c)) != 0) || c == '_'; + }; + + while (i < expression.size()) { + const char c = expression[i]; + + if (in_string) { + out += c; + if (c == '\\' && i + 1 < expression.size()) { + out += expression[i + 1]; + i += 2; + continue; + } + if (c == '\'') { + in_string = false; + } + ++i; + continue; + } + + if (c == '\'') { + in_string = true; + out += c; + ++i; + continue; + } + + const bool at_word_start = i == 0 || !is_word(expression[i - 1]); + const bool matches = + at_word_start && expression.compare(i, keyword.size(), keyword) == 0 + && (i + keyword.size() == expression.size() + || !is_word(expression[i + keyword.size()])); + + if (matches) { + out += replacement; + i += keyword.size(); + continue; + } + + out += c; + ++i; + } + + return out; +} + static auto re_intern_strings(std::string&& expression) { static const RE2 intern_string("('.*?[^\\\\]')"); @@ -404,6 +465,10 @@ parse_expression_strings(const F& column_expr) { validated_expr ); + validated_expr.parse_expression_string = re_null_literal( + std::move(validated_expr.parse_expression_string) + ); + validated_expr.parse_expression_string = re_intern_strings(std::move(validated_expr.parse_expression_string) ); diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/computed_expression.h b/rust/perspective-server/cpp/perspective/src/include/perspective/computed_expression.h index 3e9e0963d2..c62f97fa3f 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/computed_expression.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/computed_expression.h @@ -142,6 +142,12 @@ class PERSPECTIVE_EXPORT t_computed_expression_parser { // constants for True and False as DTYPE_BOOL scalars static t_tscalar TRUE_SCALAR; static t_tscalar FALSE_SCALAR; + + /** + * @brief The `null` literal, bound to the `None` symbol that + * `re_null_literal` substitutes for ExprTK's `null` keyword. + */ + static t_tscalar NONE_SCALAR; }; /** diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/expression_compare.h b/rust/perspective-server/cpp/perspective/src/include/perspective/expression_compare.h new file mode 100644 index 0000000000..81e5c32592 --- /dev/null +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/expression_compare.h @@ -0,0 +1,432 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +/** + * Comparison, equality and boolean semantics for scalars inside expressions: + * numeric dtypes promote, other dtypes must match, and mismatches poison the + * validator with a recorded reason. + */ +namespace perspective { +namespace expr { + + enum class t_cmp_op : std::uint8_t { LT, LTE, GT, GTE, EQ, NE }; + + enum class t_cmp_class : std::uint8_t { + NUMERIC, + BOOL, + STR, + DATE, + TIME, + NULL_RESULT, + INCOMPATIBLE + }; + + enum class t_ordering : std::uint8_t { LESS, EQUAL, GREATER, UNORDERED }; + + struct t_expression_type_error { + std::string m_op; + t_dtype m_lhs; + t_dtype m_rhs; + }; + + struct t_expression_type_check_sink { + std::vector m_errors; + }; + + /** + * @brief The active validation sink, non-null only inside a + * `t_expression_type_check_scope`. + */ + inline thread_local t_expression_type_check_sink* g_type_check_sink = + nullptr; + + struct t_expression_type_check_scope { + t_expression_type_check_scope(const t_expression_type_check_scope&) = + delete; + t_expression_type_check_scope& + operator=(const t_expression_type_check_scope&) = delete; + + explicit t_expression_type_check_scope( + t_expression_type_check_sink& sink + ) : + m_previous(g_type_check_sink) { + g_type_check_sink = &sink; + } + + ~t_expression_type_check_scope() { g_type_check_sink = m_previous; } + + t_expression_type_check_sink* m_previous; + }; + + inline void + report_type_error(const char* op, t_dtype lhs, t_dtype rhs) { + if (g_type_check_sink != nullptr) { + g_type_check_sink->m_errors.push_back( + t_expression_type_error{op, lhs, rhs} + ); + } + } + + inline const char* + cmp_op_name(t_cmp_op op) { + switch (op) { + case t_cmp_op::LT: + return "<"; + case t_cmp_op::LTE: + return "<="; + case t_cmp_op::GT: + return ">"; + case t_cmp_op::GTE: + return ">="; + case t_cmp_op::EQ: + return "=="; + case t_cmp_op::NE: + return "!="; + } + return "?"; + } + + inline bool + is_comparison_op_name(const std::string& op) { + return op == "<" || op == "<=" || op == ">" || op == ">=" || op == "==" + || op == "!="; + } + + /** + * @brief Human-readable validation message for a recorded type error. + */ + inline std::string + describe_type_error(const t_expression_type_error& err) { + const std::string lhs = dtype_to_str(err.m_lhs); + const std::string rhs = dtype_to_str(err.m_rhs); + + if (is_comparison_op_name(err.m_op)) { + return "Type Error - cannot compare " + lhs + " and " + rhs + + " with '" + err.m_op + "'"; + } + + return "Type Error - '" + err.m_op + "' cannot be applied to " + lhs + + " and " + rhs; + } + + /** + * @brief Resolve the comparison class of an operand dtype pair. + */ + inline t_cmp_class + classify(t_dtype lhs, t_dtype rhs) { + if (lhs == DTYPE_NONE || rhs == DTYPE_NONE) { + return t_cmp_class::NULL_RESULT; + } + + if (is_numeric_type(lhs) && is_numeric_type(rhs)) { + return t_cmp_class::NUMERIC; + } + + if (lhs != rhs) { + return t_cmp_class::INCOMPATIBLE; + } + + switch (lhs) { + case DTYPE_BOOL: + return t_cmp_class::BOOL; + case DTYPE_STR: + return t_cmp_class::STR; + case DTYPE_DATE: + return t_cmp_class::DATE; + case DTYPE_TIME: + return t_cmp_class::TIME; + default: + return t_cmp_class::INCOMPATIBLE; + } + } + + template + inline t_ordering + order_of(T x, T y) { + if (x < y) { + return t_ordering::LESS; + } + if (y < x) { + return t_ordering::GREATER; + } + return t_ordering::EQUAL; + } + + inline t_ordering + order_double(double x, double y) { + if (std::isnan(x) || std::isnan(y)) { + return t_ordering::UNORDERED; + } + return order_of(x, y); + } + + /** + * @brief Order two numeric scalars by value, exactly for integers and + * through `double` when either side is floating point. + */ + inline t_ordering + order_numeric(const t_tscalar& a, const t_tscalar& b) { + if (a.is_floating_point() || b.is_floating_point()) { + return order_double(a.to_double(), b.to_double()); + } + + const bool a_signed = a.is_signed(); + const bool b_signed = b.is_signed(); + + if (a_signed && b_signed) { + return order_of(a.to_int64(), b.to_int64()); + } + + if (!a_signed && !b_signed) { + return order_of(a.to_uint64(), b.to_uint64()); + } + + if (a_signed) { + const std::int64_t x = a.to_int64(); + if (x < 0) { + return t_ordering::LESS; + } + return order_of(static_cast(x), b.to_uint64()); + } + + const std::int64_t y = b.to_int64(); + if (y < 0) { + return t_ordering::GREATER; + } + return order_of(a.to_uint64(), static_cast(y)); + } + + inline t_ordering + order_scalar(const t_tscalar& a, const t_tscalar& b, t_cmp_class cls) { + switch (cls) { + case t_cmp_class::NUMERIC: + return order_numeric(a, b); + case t_cmp_class::BOOL: + return order_of(a.get(), b.get()); + case t_cmp_class::STR: + return order_of( + std::strcmp(a.get_char_ptr(), b.get_char_ptr()), 0 + ); + case t_cmp_class::DATE: + return order_of(a.m_data.m_uint32, b.m_data.m_uint32); + case t_cmp_class::TIME: + return order_of(a.m_data.m_int64, b.m_data.m_int64); + default: + return t_ordering::UNORDERED; + } + } + + inline bool + apply_op(t_cmp_op op, t_ordering ord) { + switch (op) { + case t_cmp_op::EQ: + return ord == t_ordering::EQUAL; + case t_cmp_op::NE: + return ord != t_ordering::EQUAL; + case t_cmp_op::LT: + return ord == t_ordering::LESS; + case t_cmp_op::LTE: + return ord == t_ordering::LESS || ord == t_ordering::EQUAL; + case t_cmp_op::GT: + return ord == t_ordering::GREATER; + case t_cmp_op::GTE: + return ord == t_ordering::GREATER || ord == t_ordering::EQUAL; + } + return false; + } + + inline t_tscalar + make_bool_result() { + t_tscalar rval; + rval.clear(); + rval.m_type = DTYPE_BOOL; + return rval; + } + + /** + * @brief Whether `v` carries a validation-time type error, which is only + * distinguishable from a null cell while a sink is installed. + */ + inline bool + is_poisoned(const t_tscalar& v) { + return g_type_check_sink != nullptr && v.m_status == STATUS_CLEAR; + } + + /** + * @brief `a b` as a `DTYPE_BOOL` scalar, poisoned for incompatible + * dtypes, with `==` / `!=` against the `null` literal acting as a null + * test. + */ + inline t_tscalar + compare(const t_tscalar& a, const t_tscalar& b, t_cmp_op op) { + t_tscalar rval = make_bool_result(); + const t_dtype lhs = a.get_dtype(); + const t_dtype rhs = b.get_dtype(); + + const t_cmp_class cls = classify(lhs, rhs); + + if (cls == t_cmp_class::INCOMPATIBLE) { + rval.m_status = STATUS_CLEAR; + report_type_error(cmp_op_name(op), lhs, rhs); + return rval; + } + + if (is_poisoned(a) || is_poisoned(b)) { + rval.m_status = STATUS_CLEAR; + return rval; + } + + if (cls == t_cmp_class::NULL_RESULT) { + if (op == t_cmp_op::EQ || op == t_cmp_op::NE) { + const t_tscalar& other = a.is_none() ? b : a; + const bool is_null = other.is_none() || !other.is_valid(); + rval.set(op == t_cmp_op::EQ ? is_null : !is_null); + } + return rval; + } + + const bool a_valid = a.is_valid(); + const bool b_valid = b.is_valid(); + + if (!a_valid && !b_valid) { + rval.set( + op == t_cmp_op::EQ || op == t_cmp_op::LTE + || op == t_cmp_op::GTE + ); + return rval; + } + + if (!a_valid || !b_valid) { + rval.set(op == t_cmp_op::NE); + return rval; + } + + rval.set(apply_op(op, order_scalar(a, b, cls))); + return rval; + } + + /** + * @brief `low <= val <= high`; null if any operand is null. + */ + inline t_tscalar + inrange(const t_tscalar& low, const t_tscalar& val, const t_tscalar& high) { + t_tscalar rval = make_bool_result(); + const t_tscalar lo = compare(low, val, t_cmp_op::LTE); + const t_tscalar hi = compare(val, high, t_cmp_op::LTE); + + if (is_poisoned(lo) || is_poisoned(hi)) { + rval.m_status = STATUS_CLEAR; + return rval; + } + + if (!lo.is_valid() || !hi.is_valid() || !low.is_valid() + || !val.is_valid() || !high.is_valid()) { + return rval; + } + + rval.set(lo.get() && hi.get()); + return rval; + } + + /** + * @brief The boolean cast used by every boolean context: null is `false`, + * numbers are `true` when non-zero, strings when non-null. + */ + inline bool + to_bool(const t_tscalar& v) { + return v.as_bool(); + } + + template + inline t_tscalar + logical(const t_tscalar& a, const t_tscalar& b, F fn) { + t_tscalar rval = make_bool_result(); + + if (is_poisoned(a) || is_poisoned(b)) { + rval.m_status = STATUS_CLEAR; + return rval; + } + + rval.set(fn(to_bool(a), to_bool(b))); + return rval; + } + + inline t_tscalar + logical_and(const t_tscalar& a, const t_tscalar& b) { + return logical(a, b, [](bool x, bool y) { return x && y; }); + } + + inline t_tscalar + logical_or(const t_tscalar& a, const t_tscalar& b) { + return logical(a, b, [](bool x, bool y) { return x || y; }); + } + + inline t_tscalar + logical_nand(const t_tscalar& a, const t_tscalar& b) { + return logical(a, b, [](bool x, bool y) { return !(x && y); }); + } + + inline t_tscalar + logical_nor(const t_tscalar& a, const t_tscalar& b) { + return logical(a, b, [](bool x, bool y) { return !(x || y); }); + } + + inline t_tscalar + logical_xor(const t_tscalar& a, const t_tscalar& b) { + return logical(a, b, [](bool x, bool y) { return x != y; }); + } + + inline t_tscalar + logical_xnor(const t_tscalar& a, const t_tscalar& b) { + return logical(a, b, [](bool x, bool y) { return x == y; }); + } + + inline t_tscalar + logical_not(const t_tscalar& v) { + t_tscalar rval = make_bool_result(); + + if (is_poisoned(v)) { + rval.m_status = STATUS_CLEAR; + return rval; + } + + rval.set(!to_bool(v)); + return rval; + } + + /** + * @brief Truthiness of a condition, with a null condition selecting the + * consequent while validating so the `if` is typed by that branch. + */ + inline bool + truthy(const t_tscalar& v) { + if (g_type_check_sink != nullptr && !v.is_valid()) { + return true; + } + + return to_bool(v); + } + +} // namespace expr +} // namespace perspective diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/exprtk.h b/rust/perspective-server/cpp/perspective/src/include/perspective/exprtk.h index ae248a7fc0..fa7b9c7a61 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/exprtk.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/exprtk.h @@ -13,6 +13,7 @@ #pragma once #include +#include namespace exprtk { @@ -193,6 +194,9 @@ namespace details { t_tscalar rval; \ rval.clear(); \ rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; \ + if (v.is_none()) { \ + return rval; \ + } \ if (!v.is_numeric()) { \ rval.m_status = perspective::t_status::STATUS_CLEAR; \ } \ @@ -209,6 +213,9 @@ namespace details { t_tscalar rval; \ rval.clear(); \ rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; \ + if (v.is_none()) { \ + return rval; \ + } \ if (!v.is_numeric()) { \ rval.m_status = perspective::t_status::STATUS_CLEAR; \ } \ @@ -487,6 +494,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_INT32; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -614,6 +625,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -660,6 +675,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -691,6 +710,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -714,6 +737,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -737,6 +764,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -757,6 +788,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -772,7 +807,7 @@ namespace details { template <> inline t_tscalar notl_impl(const t_tscalar v, t_tscalar_type_tag) { - return mknone(); + return perspective::expr::logical_not(v); } template <> @@ -782,6 +817,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -821,6 +860,10 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_INT64; + if (v.is_none()) { + return rval; + } + if (!v.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -901,34 +944,18 @@ namespace details { equal_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.m_type = perspective::t_dtype::DTYPE_BOOL; - - if (!v0.is_valid() || !v1.is_valid() || v0.is_none() - || v1.is_none()) { - rval.m_status = perspective::t_status::STATUS_INVALID; - return rval; - } - - rval.set(v0 == v1); - return rval; + return perspective::expr::compare( + v0, v1, perspective::expr::t_cmp_op::EQ + ); } template <> inline t_tscalar nequal_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.m_type = perspective::t_dtype::DTYPE_BOOL; - - if (!v0.is_valid() || !v1.is_valid() || v0.is_none() - || v1.is_none()) { - rval.m_status = perspective::t_status::STATUS_INVALID; - return rval; - } - - rval.set(v0 != v1); - return rval; + return perspective::expr::compare( + v0, v1, perspective::expr::t_cmp_op::NE + ); } template <> @@ -948,7 +975,11 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; - if (!v1.is_numeric() || !v1.is_numeric()) { + if (v0.is_none() || v1.is_none()) { + return rval; + } + + if (!v0.is_numeric() || !v1.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -970,7 +1001,11 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; - if (!v1.is_numeric() || !v1.is_numeric()) { + if (v0.is_none() || v1.is_none()) { + return rval; + } + + if (!v0.is_numeric() || !v1.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -999,7 +1034,11 @@ namespace details { rval.clear(); rval.m_type = perspective::t_dtype::DTYPE_FLOAT64; - if (!v1.is_numeric() || !v1.is_numeric()) { + if (v0.is_none() || v1.is_none()) { + return rval; + } + + if (!v0.is_numeric() || !v1.is_numeric()) { rval.m_status = perspective::t_status::STATUS_CLEAR; } @@ -1062,9 +1101,7 @@ namespace details { and_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.set(v0.as_bool() && v1.as_bool()); - return rval; + return perspective::expr::logical_and(v0, v1); } template <> @@ -1072,9 +1109,7 @@ namespace details { or_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.set(v0.as_bool() || v1.as_bool()); - return rval; + return perspective::expr::logical_or(v0, v1); } template <> @@ -1082,9 +1117,7 @@ namespace details { xor_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.set(!v0.as_bool() != !v1.as_bool()); - return rval; + return perspective::expr::logical_xor(v0, v1); } template <> @@ -1092,9 +1125,7 @@ namespace details { nand_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.set(!(v0.as_bool() && v1.as_bool())); - return rval; + return perspective::expr::logical_nand(v0, v1); } template <> @@ -1102,9 +1133,7 @@ namespace details { nor_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.set(!(v0.as_bool() || v1.as_bool())); - return rval; + return perspective::expr::logical_nor(v0, v1); } template <> @@ -1112,9 +1141,7 @@ namespace details { xnor_impl( const t_tscalar v0, const t_tscalar v1, t_tscalar_type_tag ) { - t_tscalar rval; - rval.set(v0.as_bool() == v1.as_bool()); - return rval; + return perspective::expr::logical_xnor(v0, v1); } template <> @@ -1190,36 +1217,30 @@ namespace details { return logn_impl( arg0, arg1, scalar_type_tag ); - case e_lt: { - perspective::t_tscalar rval; - rval.set(arg0 < arg1); - return rval; - }; - case e_lte: { - perspective::t_tscalar rval; - rval.set(arg0 <= arg1); - return rval; - }; - case e_eq: { - perspective::t_tscalar rval; - rval.set(std::equal_to()(arg0, arg1)); - return rval; - }; - case e_ne: { - perspective::t_tscalar rval; - rval.set(std::not_equal_to()(arg0, arg1)); - return rval; - }; - case e_gte: { - perspective::t_tscalar rval; - rval.set(arg0 >= arg1); - return rval; - }; - case e_gt: { - perspective::t_tscalar rval; - rval.set(arg0 > arg1); - return rval; - }; + case e_lt: + return perspective::expr::compare( + arg0, arg1, perspective::expr::t_cmp_op::LT + ); + case e_lte: + return perspective::expr::compare( + arg0, arg1, perspective::expr::t_cmp_op::LTE + ); + case e_eq: + return perspective::expr::compare( + arg0, arg1, perspective::expr::t_cmp_op::EQ + ); + case e_ne: + return perspective::expr::compare( + arg0, arg1, perspective::expr::t_cmp_op::NE + ); + case e_gte: + return perspective::expr::compare( + arg0, arg1, perspective::expr::t_cmp_op::GTE + ); + case e_gt: + return perspective::expr::compare( + arg0, arg1, perspective::expr::t_cmp_op::GT + ); case e_and: return and_impl(arg0, arg1, scalar_type_tag); case e_nand: @@ -1275,9 +1296,9 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(t1 < t2); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::LT + ); } static inline t_tscalar @@ -1303,9 +1324,9 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(t1 <= t2); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::LTE + ); } static inline t_tscalar process(const std::string& t1, const std::string& t2) { @@ -1329,9 +1350,9 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(t1 > t2); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::GT + ); } static inline t_tscalar @@ -1357,9 +1378,9 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(t1 >= t2); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::GTE + ); } static inline t_tscalar @@ -1384,9 +1405,9 @@ namespace details { typedef typename opr_base::Type Type; static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(std::equal_to()(t1, t2)); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::EQ + ); } static inline t_tscalar process(const std::string& t1, const std::string& t2) { @@ -1410,9 +1431,9 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(std::equal_to()(t1, t2)); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::EQ + ); } static inline t_tscalar process(const std::string& t1, const std::string& t2) { @@ -1436,9 +1457,9 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(std::not_equal_to()(t1, t2)); - return rval; + return perspective::expr::compare( + t1, t2, perspective::expr::t_cmp_op::NE + ); } static inline t_tscalar process(const std::string& t1, const std::string& t2) { @@ -1462,9 +1483,7 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(is_true(t1) && is_true(t2)); - return rval; + return perspective::expr::logical_and(t1, t2); } static inline typename expression_node::node_type type() { @@ -1482,9 +1501,7 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(!(is_true(t1) && is_true(t2))); - return rval; + return perspective::expr::logical_nand(t1, t2); } static inline typename expression_node::node_type type() { @@ -1502,9 +1519,7 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(is_true(t1) || is_true(t2)); - return rval; + return perspective::expr::logical_or(t1, t2); } static inline typename expression_node::node_type type() { @@ -1522,9 +1537,7 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - t_tscalar rval; - rval.set(!(is_true(t1) || is_true(t2))); - return rval; + return perspective::expr::logical_nor(t1, t2); } static inline typename expression_node::node_type type() { @@ -1542,7 +1555,7 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - return numeric::xor_opr(t1, t2); + return perspective::expr::logical_xor(t1, t2); } static inline typename expression_node::node_type @@ -1561,7 +1574,7 @@ namespace details { static inline t_tscalar process(Type t1, Type t2) { - return numeric::xnor_opr(t1, t2); + return perspective::expr::logical_xnor(t1, t2); } static inline typename expression_node::node_type type() { @@ -1652,9 +1665,7 @@ namespace details { static inline t_tscalar process(const t_tscalar& t0, const t_tscalar& t1, const t_tscalar& t2) { - t_tscalar rval; - rval.set((t0 <= t1) && (t1 <= t2)); - return rval; + return perspective::expr::inrange(t0, t1, t2); } static inline t_tscalar process( @@ -1777,29 +1788,25 @@ namespace details { template <> inline bool is_true(const expression_node* node) { - return std::not_equal_to()(mktscalar(false), node->value()); + return perspective::expr::truthy(node->value()); } template <> inline bool is_true(const std::pair*, bool>& node) { - return std::not_equal_to()( - mktscalar(false), node.first->value() - ); + return perspective::expr::truthy(node.first->value()); } template <> inline bool is_false(const expression_node* node) { - return std::equal_to()(mktscalar(false), node->value()); + return !perspective::expr::truthy(node->value()); } template <> inline bool is_false(const std::pair*, bool>& node) { - return std::equal_to()( - mktscalar(false), node.first->value() - ); + return !perspective::expr::truthy(node.first->value()); } /** @@ -1811,7 +1818,7 @@ namespace details { */ inline bool is_true(const t_tscalar& v) { - return v.as_bool(); + return perspective::expr::truthy(v); } inline bool From 43c01f7d241907f16f4514d5dfe867d4cefc078a Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Sat, 5 Sep 2026 00:14:34 -0400 Subject: [PATCH 2/3] Fix drag/drop focus issue in `perspective-viewer` Signed-off-by: Andrew Stein --- .../column_selector/active_column.rs | 11 ++-- .../column_selector/filter_column.rs | 9 ++- .../column_selector/inactive_column.rs | 6 +- .../column_selector/pivot_column.rs | 5 +- .../components/column_selector/sort_column.rs | 9 ++- .../src/rust/components/window_editor.rs | 5 +- .../src/rust/presentation.rs | 66 +++++++++++++++++-- .../src/rust/presentation/drag_helpers.rs | 28 ++++++++ rust/perspective-viewer/src/themes/pro.css | 1 + 9 files changed, 119 insertions(+), 21 deletions(-) diff --git a/rust/perspective-viewer/src/rust/components/column_selector/active_column.rs b/rust/perspective-viewer/src/rust/components/column_selector/active_column.rs index 5c800d65a6..efbe8b2520 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector/active_column.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector/active_column.rs @@ -330,11 +330,12 @@ impl Component for ActiveColumn { let event_name = name.to_owned(); let presentation = ctx.props().presentation.clone(); move |event: DragEvent| { - presentation.set_drag_image(&event).unwrap(); - presentation.notify_drag_start( - event_name.to_string(), - DragEffect::Move(DragTarget::Active), - ); + if presentation.set_drag_image(&event) { + presentation.notify_drag_start( + event_name.to_string(), + DragEffect::Move(DragTarget::Active), + ); + } MouseLeave(false) } diff --git a/rust/perspective-viewer/src/rust/components/column_selector/filter_column.rs b/rust/perspective-viewer/src/rust/components/column_selector/filter_column.rs index 4a57dff65b..299afdbc44 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector/filter_column.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector/filter_column.rs @@ -240,9 +240,12 @@ impl Component for FilterColumn { let event_name = ctx.props().filter.column().to_owned(); let presentation = ctx.props().presentation.clone(); move |event: DragEvent| { - presentation.set_drag_image(&event).unwrap(); - presentation - .notify_drag_start(event_name.to_string(), DragEffect::Move(DragTarget::Filter)) + if presentation.set_drag_image(&event) { + presentation.notify_drag_start( + event_name.to_string(), + DragEffect::Move(DragTarget::Filter), + ) + } } }); diff --git a/rust/perspective-viewer/src/rust/components/column_selector/inactive_column.rs b/rust/perspective-viewer/src/rust/components/column_selector/inactive_column.rs index e27cb572b3..9f30e8aa0b 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector/inactive_column.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector/inactive_column.rs @@ -141,8 +141,10 @@ impl Component for InactiveColumn { let event_name = ctx.props().name.to_owned(); let presentation = ctx.props().presentation.clone(); move |event: DragEvent| { - presentation.set_drag_image(&event).unwrap(); - presentation.notify_drag_start(event_name.to_string(), DragEffect::Copy); + if presentation.set_drag_image(&event) { + presentation.notify_drag_start(event_name.to_string(), DragEffect::Copy); + } + MouseLeave(true) } }); diff --git a/rust/perspective-viewer/src/rust/components/column_selector/pivot_column.rs b/rust/perspective-viewer/src/rust/components/column_selector/pivot_column.rs index 9c9929199d..000122361f 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector/pivot_column.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector/pivot_column.rs @@ -73,8 +73,9 @@ impl Component for PivotColumn { let presentation = ctx.props().presentation.clone(); let action = ctx.props().action; move |event: DragEvent| { - presentation.set_drag_image(&event).unwrap(); - presentation.notify_drag_start(event_name.to_string(), DragEffect::Move(action)) + if presentation.set_drag_image(&event) { + presentation.notify_drag_start(event_name.to_string(), DragEffect::Move(action)) + } } }); diff --git a/rust/perspective-viewer/src/rust/components/column_selector/sort_column.rs b/rust/perspective-viewer/src/rust/components/column_selector/sort_column.rs index 6030847de5..c779e64974 100644 --- a/rust/perspective-viewer/src/rust/components/column_selector/sort_column.rs +++ b/rust/perspective-viewer/src/rust/components/column_selector/sort_column.rs @@ -104,9 +104,12 @@ impl Component for SortColumn { let event_name = ctx.props().sort.0.to_owned(); let presentation = ctx.props().presentation.clone(); move |event: DragEvent| { - presentation.set_drag_image(&event).unwrap(); - presentation - .notify_drag_start(event_name.to_string(), DragEffect::Move(DragTarget::Sort)) + if presentation.set_drag_image(&event) { + presentation.notify_drag_start( + event_name.to_string(), + DragEffect::Move(DragTarget::Sort), + ) + } } }); diff --git a/rust/perspective-viewer/src/rust/components/window_editor.rs b/rust/perspective-viewer/src/rust/components/window_editor.rs index 28c34b0d8c..c9a25171bf 100644 --- a/rust/perspective-viewer/src/rust/components/window_editor.rs +++ b/rust/perspective-viewer/src/rust/components/window_editor.rs @@ -479,8 +479,9 @@ impl Component for WindowSlotColumn { let presentation = ctx.props().presentation.clone(); let action = ctx.props().action; move |event: DragEvent| { - presentation.set_drag_image(&event).unwrap(); - presentation.notify_drag_start(column.clone(), DragEffect::Move(action)) + if presentation.set_drag_image(&event) { + presentation.notify_drag_start(column.clone(), DragEffect::Move(action)) + } } }); diff --git a/rust/perspective-viewer/src/rust/presentation.rs b/rust/perspective-viewer/src/rust/presentation.rs index 4271992e0a..78d97bef70 100644 --- a/rust/perspective-viewer/src/rust/presentation.rs +++ b/rust/perspective-viewer/src/rust/presentation.rs @@ -32,6 +32,7 @@ pub use self::column_locator::{ }; use self::drag_helpers::DragTargetState; pub use self::drag_helpers::{DragDropContainer, DragEndCallback}; +use self::drag_helpers::{PointerDownCallback, clear_document_selection, closest_draggable}; pub use self::props::{DragDropProps, PresentationProps}; use crate::config::{CssKind, NamedValue, assign_palette_names}; use crate::utils::*; @@ -126,6 +127,10 @@ pub struct PresentationHandle { /// dragged element from the shadow tree. host_dragend: RefCell>, + /// Host-level `pointerdown` listener that clears a stale page selection + /// before it can turn a row drag into a browser selection drag. + host_pointerdown: RefCell>, + source_dragend: RefCell>, /// IntersectionObserver-based fallback for the drag image, kept alive for @@ -202,10 +207,12 @@ impl Presentation { on_dragstart: Default::default(), on_dragend: Default::default(), host_dragend: Default::default(), + host_pointerdown: Default::default(), source_dragend: Default::default(), drag_target: Default::default(), })); + theme.register_host_pointerdown(); ApiFuture::spawn(theme.clone().init()); theme } @@ -577,18 +584,53 @@ impl Presentation { } } - pub fn set_drag_image(&self, event: &DragEvent) -> ApiResult<()> { + /// Claim a `dragstart` for the column drag machinery, returning `false` + /// (cancelling the native drag) when it did not originate on a + /// `draggable="true"` row or installation failed. + pub fn set_drag_image(&self, event: &DragEvent) -> bool { + match self.try_set_drag_image(event) { + Ok(true) => true, + Ok(false) => { + event.prevent_default(); + if let Err(e) = clear_document_selection() { + web_sys::console::warn_1(&e.into()); + } + + false + }, + Err(e) => { + event.prevent_default(); + web_sys::console::warn_1(&e.into()); + false + }, + } + } + + fn try_set_drag_image(&self, event: &DragEvent) -> ApiResult { event.stop_propagation(); + let Some(original) = closest_draggable(event) else { + return Ok(false); + }; + + let is_row_drag = event + .target() + .and_then(|target| target.dyn_into::().ok()) + .map(|target| original.is_same_node(Some(&target))) + .unwrap_or(false); + + if !is_row_drag { + return Ok(false); + } + self.register_source_dragend(event)?; if let Some(dt) = event.data_transfer() { dt.set_drop_effect("move"); } - let original: HtmlElement = event.target().into_apierror()?.unchecked_into(); let elem: HtmlElement = original .children() .get_with_index(0) - .unwrap() + .into_apierror()? .clone_node_with_deep(true)? .unchecked_into(); @@ -612,7 +654,7 @@ impl Presentation { Ok(()) }); - Ok(()) + Ok(true) } /// Is the drag/drop state currently in `action`? @@ -695,6 +737,22 @@ impl Presentation { Ok(()) } + fn register_host_pointerdown(&self) { + let closure = Closure::wrap(Box::new(move |event: PointerEvent| { + if closest_draggable(&event).is_some() + && let Err(e) = clear_document_selection() + { + web_sys::console::warn_1(&e.into()); + } + }) as Box); + + self.viewer_elem + .add_event_listener_with_callback("pointerdown", closure.as_ref().unchecked_ref()) + .unwrap(); + + *self.host_pointerdown.borrow_mut() = Some(closure); + } + fn register_host_dragend(&self) { if let Some(prev) = self.host_dragend.borrow_mut().take() { let _ = self diff --git a/rust/perspective-viewer/src/rust/presentation/drag_helpers.rs b/rust/perspective-viewer/src/rust/presentation/drag_helpers.rs index e992322946..5c87bf3bef 100644 --- a/rust/perspective-viewer/src/rust/presentation/drag_helpers.rs +++ b/rust/perspective-viewer/src/rust/presentation/drag_helpers.rs @@ -22,6 +22,34 @@ use yew::prelude::*; use crate::js::{IntersectionObserver, IntersectionObserverEntry}; pub type DragEndCallback = Closure; +pub type PointerDownCallback = Closure; + +/// The `draggable="true"` row enclosing `event`'s composed-path target, if +/// any. +pub fn closest_draggable(event: &Event) -> Option { + event + .composed_path() + .get(0) + .dyn_into::() + .ok()? + .closest("[draggable=\"true\"]") + .ok() + .flatten()? + .dyn_into::() + .ok() +} + +/// Collapse the page's text selection so a `draggable="true"` row cannot lose +/// to a browser selection drag. +pub fn clear_document_selection() -> ApiResult<()> { + if let Some(selection) = global::window().get_selection()? + && !selection.is_collapsed() + { + selection.remove_all_ranges()?; + } + + Ok(()) +} /// Safari does not set `relatedTarget` on `"dragleave"`, which makes it /// impossible to determine whether a logical drag leave has happened with just diff --git a/rust/perspective-viewer/src/themes/pro.css b/rust/perspective-viewer/src/themes/pro.css index bd43dde60a..3ead49abc7 100644 --- a/rust/perspective-viewer/src/themes/pro.css +++ b/rust/perspective-viewer/src/themes/pro.css @@ -55,6 +55,7 @@ perspective-viewer [theme="Pro Light"] { --psp-charts--gridline--color: #eaedef; --psp-charts--axis-ticks--color: #161616; --psp-charts--axis-lines--color: #c5c9d0; + --psp-charts--legend--color: #161616; --psp-charts--legend--background: var(--psp--background-color); --psp-charts--series--color: rgba(31, 119, 180, 0.8); --psp-charts--series-1--color: #0366d6; From 8016aeadd0948896533f6ba15623e62ce8c86b23 Mon Sep 17 00:00:00 2001 From: Andrew Stein Date: Sat, 5 Sep 2026 11:28:46 -0400 Subject: [PATCH 3/3] Add `View::on_remove` Signed-off-by: Andrew Stein --- .gitignore | 1 + docs/build.config.mjs | 53 +++- .../explanation/architecture/client_server.md | 4 +- docs/md/explanation/view/advanced.md | 40 +++ docs/md/how_to/python/callbacks.md | 12 + .../jupyterlab/test/jupyter/widget.spec.mjs | 82 ++++++ pnpm-lock.yaml | 12 +- pnpm-workspace.yaml | 2 +- rust/metadata/main.rs | 5 +- rust/perspective-client/build.rs | 11 +- rust/perspective-client/perspective.proto | 17 ++ rust/perspective-client/src/rust/client.rs | 42 ++- rust/perspective-client/src/rust/lib.rs | 3 +- rust/perspective-client/src/rust/session.rs | 2 +- rust/perspective-client/src/rust/table.rs | 34 ++- rust/perspective-client/src/rust/table_ref.rs | 6 +- rust/perspective-client/src/rust/view.rs | 95 ++++++- .../src/rust/virtual_server/server.rs | 15 +- rust/perspective-js/src/rust/lib.rs | 5 + rust/perspective-js/src/rust/view.rs | 55 +++- .../test/js/constructors.spec.js | 146 ++++++++++ .../test/js/multi_server.spec.js | 28 ++ rust/perspective-js/test/js/on_remove.spec.js | 250 ++++++++++++++++++ .../perspective/tests/table/test_on_remove.py | 140 ++++++++++ .../src/client/client_async.rs | 57 +++- .../src/client/client_sync.rs | 32 +++ .../cpp/perspective/src/cpp/arrow_writer.cpp | 126 +++++++++ .../cpp/perspective/src/cpp/gnode.cpp | 123 +++++++++ .../cpp/perspective/src/cpp/pool.cpp | 13 + .../cpp/perspective/src/cpp/server.cpp | 138 +++++++++- .../cpp/perspective/src/cpp/table.cpp | 34 ++- .../src/include/perspective/arrow_writer.h | 4 + .../src/include/perspective/gnode.h | 15 ++ .../src/include/perspective/pool.h | 2 + .../src/include/perspective/server.h | 12 + .../src/include/perspective/table.h | 1 + rust/perspective-viewer/src/rust/lib.rs | 19 +- .../src/rust/presentation.rs | 5 +- 38 files changed, 1584 insertions(+), 57 deletions(-) create mode 100644 rust/perspective-js/test/js/on_remove.spec.js create mode 100644 rust/perspective-python/perspective/tests/table/test_on_remove.py diff --git a/.gitignore b/.gitignore index 1a11791125..b99c083c9b 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,7 @@ __pycache__/ .vscode/* *.so *~ +/.claude /.emacs.desktop /.emacs.desktop.lock /.idea diff --git a/docs/build.config.mjs b/docs/build.config.mjs index 555d2d3abf..aa9d17d1df 100644 --- a/docs/build.config.mjs +++ b/docs/build.config.mjs @@ -234,6 +234,54 @@ function copyDocsBundle() { } } +/** + * Stub the optional Memory64 engine binary when `@perspective-dev/server` + * was built without `PSP_WASM64`, so the docs bundle without that compile. + * The stub throws when imported, which rejects `engines.ts`'s `wasm64` + * thunk and lets `init_server` fall back to the wasm32 binary at runtime. + */ +function optionalWasm64Plugin() { + const NAMESPACE = "optional-wasm64"; + return { + name: NAMESPACE, + setup(build) { + build.onResolve( + { filter: /perspective-server\.memory64\.wasm$/ }, + async (args) => { + if (args.pluginData === NAMESPACE) { + return; + } + + const resolved = await build.resolve(args.path, { + kind: args.kind, + importer: args.importer, + resolveDir: args.resolveDir, + pluginData: NAMESPACE, + }); + + if (resolved.errors.length === 0) { + return resolved; + } + + console.warn( + `No ${path.basename(args.path)} (PSP_WASM64 unset); ` + + "perspective-server will run as wasm32.", + ); + + return { path: args.path, namespace: NAMESPACE }; + }, + ); + + build.onLoad({ filter: /.*/, namespace: NAMESPACE }, (args) => ({ + loader: "js", + contents: `throw new Error(${JSON.stringify( + `${args.path} was not built (set PSP_WASM64=1)`, + )});`, + })); + }, + }; +} + function esbuildOptions() { return { entryPoints: [path.join(__dirname, "src/index.ts")], @@ -251,6 +299,7 @@ function esbuildOptions() { ".wasm": "file", ".arrow": "file", }, + plugins: [optionalWasm64Plugin()], }; } @@ -301,9 +350,11 @@ async function watch() { copyStatic(); copyDocsBundle(); + const options = esbuildOptions(); const ctx = await esbuild.context({ - ...esbuildOptions(), + ...options, plugins: [ + ...options.plugins, { name: "livereload", setup(build) { diff --git a/docs/md/explanation/architecture/client_server.md b/docs/md/explanation/architecture/client_server.md index cd7e60a3ad..be30ff4dd3 100644 --- a/docs/md/explanation/architecture/client_server.md +++ b/docs/md/explanation/architecture/client_server.md @@ -42,7 +42,9 @@ loop.start() ## Javascript client Perspective's websocket client interfaces with the Python server, then -_replicates_ the server-side Table. +_replicates_ the server-side Table. When the server-side `Table` has an `index`, +the replica inherits it, and both `update()` and `remove()` on the server are +mirrored in the browser. ```javascript const websocket = await perspective.websocket("ws://localhost:8080"); diff --git a/docs/md/explanation/view/advanced.md b/docs/md/explanation/view/advanced.md index a5631e6ca9..a757223886 100644 --- a/docs/md/explanation/view/advanced.md +++ b/docs/md/explanation/view/advanced.md @@ -182,6 +182,39 @@ When `mode` is set to `"row"`, the callback receives a delta of only the rows that changed (as Apache Arrow), which is useful for efficiently synchronizing tables across clients. +## Remove Callbacks + +Register a callback to be notified whenever rows are removed from the underlying +`Table` by `remove()`, which requires an `index`. The callback receives the +`port_id` and the removed `index` column values as an Apache Arrow of a single +column named after the index. It fires once per update step, only for rows which +existed before that step; `replace()` reports the keys it does not re-supply, +and `clear()` reports every key: + +
+ +```javascript +const callback = await view.on_remove(({ indices, port_id }) => { + replica.remove(indices); +}); + +// Later, remove the callback +await view.remove_remove(callback); +``` + +
+
+ +```python +def on_remove(port_id, indices): + replica.remove(indices) + +callback = view.on_remove(on_remove) +view.remove_remove(callback) +``` + +
+ ## Flattening a View into a Table A [`Table`] can be constructed on a [`Table::view`] instance, which will return @@ -192,6 +225,13 @@ particularly useful for implementing a handles the `View` serialization and `on_update` forwarding for you. This pattern is available in JavaScript, Python and Rust. +When the source `Table` has an `index`, and the `View` is unpivoted and includes +the index column, the new `Table` inherits that `index` and subscribes to the +source's `on_remove()`, so in-place updates and `remove()` calls on the source +are mirrored rather than appended. A pivoted `View`, or one which omits the +index column, produces an unindexed, append-only `Table`. A `limit` is inherited +the same way. `replace()` and `clear()` on the source are mirrored too. +
```javascript diff --git a/docs/md/how_to/python/callbacks.md b/docs/md/how_to/python/callbacks.md index cc6b28d12c..7ec4b87fe2 100644 --- a/docs/md/how_to/python/callbacks.md +++ b/docs/md/how_to/python/callbacks.md @@ -32,3 +32,15 @@ view.remove_delete(on_delete_id) Callbacks defined with a lambda function cannot be removed, as lambda functions have no identifier. + +`on_remove` fires when rows are removed from a `View`'s `Table` with an `index`, +and receives the port ID and the removed index values as an Apache Arrow +(`bytes`) of one column named after the index: + +```python +def remove_callback(port_id, indices): + print("Removed", client.table(indices).view().to_records()) + +on_remove_id = view.on_remove(remove_callback) +view.remove_remove(on_remove_id) +``` diff --git a/packages/jupyterlab/test/jupyter/widget.spec.mjs b/packages/jupyterlab/test/jupyter/widget.spec.mjs index b9f1a2eb9c..1b0ec0fc25 100644 --- a/packages/jupyterlab/test/jupyter/widget.spec.mjs +++ b/packages/jupyterlab/test/jupyter/widget.spec.mjs @@ -723,6 +723,88 @@ assert w2.group_by == ["bool"] }, ); + test_jupyter( + "Table.remove propagates to widget", + [ + [ + "server = perspective.Server()", + "client = server.new_local_client()", + "table = client.table({'key': 'string', 'value': 'integer'}, index='key')", + "table.update([{'key': 'What is the answer?', 'value': 42}])", + "w = perspective.widget.PerspectiveWidget(table)", + ].join("\n"), + "w", + ], + async ({ page }) => { + await default_body(page); + const rows = page.locator("regular-table tbody tr"); + await expect(rows).toHaveCount(1); + + await add_and_execute_cell( + page, + "table.update([{'key': 'Hej', 'value': 74}])", + ); + await expect(rows).toHaveCount(2); + + await add_and_execute_cell( + page, + "table.update([{'key': 'Hej', 'value': 75}])", + ); + await expect(rows).toHaveCount(2); + + await add_and_execute_cell(page, "table.remove(['Hej'])"); + await expect(rows).toHaveCount(1); + await expect(rows.first()).toContainText("What is the answer?"); + + await add_and_execute_cell(page, "w"); + const viewers = page.locator( + ".jp-OutputArea-output perspective-viewer", + ); + await expect(viewers).toHaveCount(2); + for (const v of await viewers.all()) { + await v.evaluate(async (viewer) => await viewer.flush()); + await expect( + v.locator("regular-table tbody tr"), + ).toHaveCount(1); + } + }, + ); + + test_jupyter( + "Table.remove propagates to widget in client-server binding mode", + [ + [ + "server = perspective.Server()", + "client = server.new_local_client()", + "table = client.table({'key': 'string', 'value': 'integer'}, index='key')", + "table.update([{'key': 'What is the answer?', 'value': 42}])", + "w = perspective.widget.PerspectiveWidget(table, binding_mode='client-server')", + ].join("\n"), + "w", + ], + async ({ page }) => { + await default_body(page); + const rows = page.locator("regular-table tbody tr"); + await expect(rows).toHaveCount(1); + + await add_and_execute_cell( + page, + "table.update([{'key': 'Hej', 'value': 74}])", + ); + await expect(rows).toHaveCount(2); + + await add_and_execute_cell( + page, + "table.update([{'key': 'Hej', 'value': 75}])", + ); + await expect(rows).toHaveCount(2); + + await add_and_execute_cell(page, "table.remove(['Hej'])"); + await expect(rows).toHaveCount(1); + await expect(rows.first()).toContainText("What is the answer?"); + }, + ); + // Traits mutated after construction but before the widget is displayed // must be applied by the initial `restore()` (the restore-before- // load-complete path). diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bd6a0a60d6..894fb77f4f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -124,8 +124,8 @@ catalogs: specifier: '=0.6.1' version: 0.6.1 regular-table: - specifier: '=0.9.0' - version: 0.9.0 + specifier: '=0.9.1' + version: 0.9.1 stoppable: specifier: '=1.1.0' version: 1.1.0 @@ -797,7 +797,7 @@ importers: version: link:../../rust/perspective-viewer regular-table: specifier: 'catalog:' - version: 0.9.0 + version: 0.9.1 devDependencies: '@perspective-dev/esbuild-plugin': specifier: 'workspace:' @@ -4412,8 +4412,8 @@ packages: regular-layout@0.6.1: resolution: {integrity: sha512-vGDEFACgFbS/d5kLWJ6AMWY/3DYaZoF1P8+y4KIDnPUFqUQgV5bV8avX7836izmexfmz6ik8JZ4V0Xo4FhaZGQ==} - regular-table@0.9.0: - resolution: {integrity: sha512-SY3gZvEQqN0PdMsg77qJrWHxAWWx3kJsG4+PlskcL1zAPEczo42/95SdlDXe/S0KckqOJrChrpDjPE7RrRf5jQ==} + regular-table@0.9.1: + resolution: {integrity: sha512-3I/2I3NEhmyScCevW/r1AE9hyUkqrPRMDsgo/nDnGXhpXVZOxUddJQGBmEMLgyc7OSaQSzl9BtXllnMqw1MwCA==} engines: {node: '>=16'} relateurl@0.2.7: @@ -9195,7 +9195,7 @@ snapshots: regular-layout@0.6.1: {} - regular-table@0.9.0: {} + regular-table@0.9.1: {} relateurl@0.2.7: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index ea490093de..16cf19a287 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -37,7 +37,7 @@ catalog: "react-dom": ">17 <20" "react": ">17 <20" "regular-layout": "=0.6.1" - "regular-table": "=0.9.0" + "regular-table": "=0.9.1" "stoppable": "=1.1.0" "ws": "^8.17.0" diff --git a/rust/metadata/main.rs b/rust/metadata/main.rs index f7f0ea61e7..8ccabc66af 100644 --- a/rust/metadata/main.rs +++ b/rust/metadata/main.rs @@ -33,8 +33,8 @@ use std::fs; use perspective_client::config::*; use perspective_client::virtual_server::Features; use perspective_client::{ - ColumnWindow, DeleteOptions, JoinOptions, OnUpdateData, OnUpdateOptions, SystemInfo, - TableInitOptions, UpdateOptions, ViewWindow, + ColumnWindow, DeleteOptions, JoinOptions, OnRemoveData, OnUpdateData, OnUpdateOptions, + SystemInfo, TableInitOptions, UpdateOptions, ViewWindow, }; use perspective_js::TypedArrayWindow; use perspective_viewer::config::{ @@ -109,6 +109,7 @@ pub fn generate_type_bindings_js() -> Result<(), Box> { DeleteOptions::export_all_to(&path)?; Features::export_all_to(&path)?; JoinOptions::export_all_to(&path)?; + OnRemoveData::export_all_to(&path)?; OnUpdateData::export_all_to(&path)?; OnUpdateOptions::export_all_to(&path)?; SystemInfo::::export_all_to(&path)?; diff --git a/rust/perspective-client/build.rs b/rust/perspective-client/build.rs index b1674b6504..5b7246d1fc 100644 --- a/rust/perspective-client/build.rs +++ b/rust/perspective-client/build.rs @@ -54,8 +54,17 @@ fn prost_build() -> Result<()> { prost_build::Config::new() // .bytes(["ViewToArrowResp.arrow", "from_arrow"]) .type_attribute("ViewOnUpdateResp", "#[derive(ts_rs::TS)]") - .field_attribute("ViewOnUpdateResp.delta", "#[ts(as = \"Vec::\")]") + .field_attribute( + "ViewOnUpdateResp.delta", + "#[ts(type = \"Uint8Array | undefined\")]", + ) .field_attribute("ViewOnUpdateResp.delta", "#[serde(with = \"serde_bytes\")]") + .type_attribute("ViewOnRemoveResp", "#[derive(ts_rs::TS)]") + .field_attribute("ViewOnRemoveResp.indices", "#[ts(type = \"Uint8Array\")]") + .field_attribute( + "ViewOnRemoveResp.indices", + "#[serde(with = \"serde_bytes\")]", + ) .type_attribute("ColumnType", "#[derive(ts_rs::TS)]") .type_attribute( "JoinType", diff --git a/rust/perspective-client/perspective.proto b/rust/perspective-client/perspective.proto index fac18573e2..c13a2032ca 100644 --- a/rust/perspective-client/perspective.proto +++ b/rust/perspective-client/perspective.proto @@ -159,6 +159,8 @@ message Request { ViewOnDeleteReq view_on_delete_req = 34; ViewRemoveDeleteReq view_remove_delete_req = 35; MakeJoinTableReq make_join_table_req = 38; + ViewOnRemoveReq view_on_remove_req = 39; + ViewRemoveOnRemoveReq view_remove_on_remove_req = 40; } } @@ -202,6 +204,8 @@ message Response { ViewOnDeleteResp view_on_delete_resp = 34; ViewRemoveDeleteResp view_remove_delete_resp = 35; MakeJoinTableResp make_join_table_resp = 38; + ViewOnRemoveResp view_on_remove_resp = 39; + ViewRemoveOnRemoveResp view_remove_on_remove_resp = 40; ServerError server_error = 50; } } @@ -416,6 +420,19 @@ message TableRemoveDeleteReq { } message TableRemoveDeleteResp {} +// `View::on_remove` +message ViewOnRemoveReq {} +message ViewOnRemoveResp { + optional bytes indices = 1; + uint32 port_id = 2; +} + +// `View::remove_remove` +message ViewRemoveOnRemoveReq { + uint32 id = 1; +} +message ViewRemoveOnRemoveResp {} + // `Table::update` message TableUpdateReq { MakeTableData data = 1; diff --git a/rust/perspective-client/src/rust/client.rs b/rust/perspective-client/src/rust/client.rs index a9dfed0376..4d5f7d8b29 100644 --- a/rust/perspective-client/src/rust/client.rs +++ b/rust/perspective-client/src/rust/client.rs @@ -29,11 +29,11 @@ use crate::proto::{ HostedTable, JoinType, MakeJoinTableReq, MakeTableReq, RemoveHostedTablesUpdateReq, Request, Response, ServerError, ServerSystemInfoReq, }; -use crate::table::{JoinOptions, Table, TableInitOptions, TableOptions}; +use crate::table::{JoinOptions, Table, TableInitOptions, TableOptions, ViewBinding}; use crate::table_data::{TableData, UpdateData}; use crate::table_ref::TableRef; use crate::utils::*; -use crate::view::{OnUpdateData, ViewWindow}; +use crate::view::{OnRemoveData, OnUpdateData, ViewWindow}; use crate::{OnUpdateMode, OnUpdateOptions, asyncfn, clone}; /// Metadata about the engine runtime (such as total heap utilization). @@ -571,6 +571,21 @@ impl Client { }; if let TableData::View(view) = &input { + let mut options = options; + let source_index = view.source.as_ref().and_then(|x| x.options.index.clone()); + if let (None, Some(index)) = (&options.index, &source_index) { + let config = view.get_config().await?; + let is_flat = config.group_by.is_empty() && config.split_by.is_empty(); + let has_index = config.columns.iter().flatten().any(|x| x == index); + if is_flat && has_index { + options.index = Some(index.clone()); + } + } + + if options.index.is_none() && options.limit.is_none() { + options.limit = view.source.as_ref().and_then(|x| x.options.limit); + } + let window = ViewWindow::default(); let arrow = view.to_arrow(window).await?; let mut table = self @@ -588,8 +603,27 @@ impl Client { mode: Some(OnUpdateMode::Row), }; - let on_update_token = view.on_update(callback, options).await?; - table.view_update_token = Some(on_update_token); + let update_token = view.on_update(callback, options).await?; + let remove_token = if source_index.is_some() && source_index == table.get_index() { + let table_ = table.clone(); + let callback = asyncfn!(table_, async move |removed: OnRemoveData| { + if let Some(indices) = removed.indices.as_ref().filter(|x| !x.is_empty()) { + let indices = UpdateData::Arrow(indices.clone().into()); + table_.remove(indices).await.unwrap_or_log(); + } + }); + + Some(view.on_remove(callback).await?) + } else { + None + }; + + table.view_binding = Some(ViewBinding { + view: view.clone(), + update_token, + remove_token, + }); + Ok(table) } else { self.crate_table_inner(input, options.into(), entity_id) diff --git a/rust/perspective-client/src/rust/lib.rs b/rust/perspective-client/src/rust/lib.rs index faaf031cc5..cde1b27f63 100644 --- a/rust/perspective-client/src/rust/lib.rs +++ b/rust/perspective-client/src/rust/lib.rs @@ -61,7 +61,7 @@ pub use crate::table::{ pub use crate::table_data::{TableData, UpdateData}; pub use crate::table_ref::TableRef; pub use crate::view::{ - ColumnWindow, OnUpdateData, OnUpdateMode, OnUpdateOptions, View, ViewWindow, + ColumnWindow, OnRemoveData, OnUpdateData, OnUpdateMode, OnUpdateOptions, View, ViewWindow, }; pub type ClientError = utils::ClientError; @@ -135,6 +135,7 @@ macro_rules! assert_view_api { &$x::num_rows, // &$x::on_update, &$x::remove_update, + &$x::remove_remove, &$x::on_delete, &$x::remove_delete, &$x::schema, diff --git a/rust/perspective-client/src/rust/session.rs b/rust/perspective-client/src/rust/session.rs index cc37a2af30..337836101a 100644 --- a/rust/perspective-client/src/rust/session.rs +++ b/rust/perspective-client/src/rust/session.rs @@ -85,7 +85,7 @@ impl Session for ProxySession { let req = Request::decode(request)?; let callback = self.callback.clone(); match req.client_req.as_ref() { - Some(ClientReq::ViewOnUpdateReq(_)) => { + Some(ClientReq::ViewOnUpdateReq(_)) | Some(ClientReq::ViewOnRemoveReq(_)) => { let on_update = asyncfn!(callback, async move |response| encode(response, callback)); self.parent.subscribe(&req, on_update).await? diff --git a/rust/perspective-client/src/rust/table.rs b/rust/perspective-client/src/rust/table.rs index 805aeabc85..e7108453a0 100644 --- a/rust/perspective-client/src/rust/table.rs +++ b/rust/perspective-client/src/rust/table.rs @@ -26,7 +26,7 @@ use crate::proto::response::ClientResp; use crate::proto::*; use crate::table_data::UpdateData; use crate::utils::*; -use crate::view::View; +use crate::view::{View, ViewSource}; pub type Schema = HashMap; @@ -151,6 +151,15 @@ pub(crate) struct TableOptions { pub list_flatten: Option, } +/// The source [`View`] of a replica [`Table`] built by [`Client::table`], +/// with the subscription tokens to release when the replica is deleted. +#[derive(Clone)] +pub(crate) struct ViewBinding { + pub view: View, + pub update_token: u32, + pub remove_token: Option, +} + impl From for TableOptions { fn from(value: TableInitOptions) -> Self { TableOptions { @@ -218,11 +227,7 @@ pub struct Table { name: String, client: Client, options: TableOptions, - - /// If this table is constructed from a View, the view's on_update callback - /// is wired into this table. So, we store the token to clean it up properly - /// on destruction. - pub(crate) view_update_token: Option, + pub(crate) view_binding: Option, } assert_table_api!(Table); @@ -239,7 +244,7 @@ impl Table { name, client, options, - view_update_token: None, + view_binding: None, } } @@ -338,6 +343,13 @@ impl Table { /// # Ok(()) } /// ``` pub async fn delete(&self, options: DeleteOptions) -> ClientResult<()> { + if let Some(binding) = &self.view_binding { + binding.view.remove_update(binding.update_token).await?; + if let Some(token) = binding.remove_token { + binding.view.remove_remove(token).await?; + } + } + let msg = self.client_message(ClientReq::TableDeleteReq(TableDeleteReq { is_immediate: !options.lazy, })); @@ -625,7 +637,13 @@ impl Table { ClientResp::TableMakeViewResp(TableMakeViewResp { view_id }) if view_id == view_name => { - Ok(View::new(view_name, self.client.clone())) + Ok(View::new_with_source( + view_name, + self.client.clone(), + ViewSource { + options: self.options.clone(), + }, + )) }, resp => Err(resp.into()), } diff --git a/rust/perspective-client/src/rust/table_ref.rs b/rust/perspective-client/src/rust/table_ref.rs index bf0b1697ca..8e03c57209 100644 --- a/rust/perspective-client/src/rust/table_ref.rs +++ b/rust/perspective-client/src/rust/table_ref.rs @@ -15,7 +15,7 @@ use crate::Table; /// A reference to a table, either by handle or by name. #[derive(Clone)] pub enum TableRef { - Table(Table), + Table(Box), Name(String), } @@ -30,13 +30,13 @@ impl TableRef { impl From<&Table> for TableRef { fn from(table: &Table) -> Self { - TableRef::Table(table.clone()) + TableRef::Table(table.clone().into()) } } impl From
for TableRef { fn from(table: Table) -> Self { - TableRef::Table(table) + TableRef::Table(table.into()) } } diff --git a/rust/perspective-client/src/rust/view.rs b/rust/perspective-client/src/rust/view.rs index e8248f1088..aa54509aea 100644 --- a/rust/perspective-client/src/rust/view.rs +++ b/rust/perspective-client/src/rust/view.rs @@ -156,6 +156,18 @@ impl Deref for OnUpdateData { } } +/// Removed index values and port ID corresponding to a remove batch, provided +/// to the callback argument to [`View::on_remove`]. +#[derive(TS)] +pub struct OnRemoveData(crate::proto::ViewOnRemoveResp); + +impl Deref for OnRemoveData { + type Target = crate::proto::ViewOnRemoveResp; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} /// The [`View`] struct is Perspective's query and serialization interface. It /// represents a query on the `Table`'s dataset and is always created from an /// existing `Table` instance via the [`Table::view`] method. @@ -268,13 +280,33 @@ impl Deref for OnUpdateData { pub struct View { pub name: String, client: Client, + pub(crate) source: Option, +} + +/// The options of the [`Table`] a [`View`] was created from, as known to the +/// [`Client`] which created it; a [`View`] opened by name alone has no source. +#[derive(Clone, Debug)] +pub(crate) struct ViewSource { + pub options: crate::table::TableOptions, } assert_view_api!(View); impl View { pub fn new(name: String, client: Client) -> Self { - View { name, client } + View { + name, + client, + source: None, + } + } + + pub(crate) fn new_with_source(name: String, client: Client, source: ViewSource) -> Self { + View { + name, + client, + source: Some(source), + } } fn client_message(&self, req: ClientReq) -> Request { @@ -578,6 +610,67 @@ impl View { } } + /// Register a callback which is invoked whenever rows are removed from + /// this [`View`]'s [`Table`] by [`Table::remove`], with the removed `index` + /// column values as an Apache Arrow of one column named after the index. + /// + /// [`Table::replace`] reports the keys it does not re-supply and + /// [`Table::clear`] reports every key. It never fires for a + /// [`Table`] without an `index`. + /// + /// # Examples + /// + /// ```no_run + /// # use perspective_client::{View, OnRemoveData}; + /// # async fn run() -> Result<(), Box> { + /// # let view: View = todo!(); + /// let callback = |removed: OnRemoveData| async move { println!("{:?}", removed.port_id) }; + /// let cid = view.on_remove(callback).await?; + /// view.remove_remove(cid).await?; + /// # Ok(()) } + /// ``` + pub async fn on_remove(&self, on_remove: T) -> ClientResult + where + T: Fn(OnRemoveData) -> U + Send + Sync + 'static, + U: Future + Send + 'static, + { + let on_remove = Arc::new(on_remove); + let callback = move |resp: Response| { + let on_remove = on_remove.clone(); + async move { + match resp.client_resp { + Some(ClientResp::ViewOnRemoveResp(resp)) => { + on_remove(OnRemoveData(resp)).await; + Ok(()) + }, + resp => Err(resp.into()), + } + } + }; + + let msg = self.client_message(ClientReq::ViewOnRemoveReq(ViewOnRemoveReq {})); + self.client.subscribe(&msg, callback).await?; + Ok(msg.msg_id) + } + + /// Unregister a previously registered [`View::on_remove`] callback. + /// + /// # Arguments + /// + /// - `callback_id` - A callback `id` as returned by a reciprocal call to + /// [`View::on_remove`]. + pub async fn remove_remove(&self, callback_id: u32) -> ClientResult<()> { + let msg = self.client_message(ClientReq::ViewRemoveOnRemoveReq(ViewRemoveOnRemoveReq { + id: callback_id, + })); + + self.client.unsubscribe(callback_id).await?; + match self.client.oneshot(&msg).await? { + ClientResp::ViewRemoveOnRemoveResp(_) => Ok(()), + resp => Err(resp.into()), + } + } + /// Register a callback with this [`View`]. Whenever the [`View`] is /// deleted, this callback will be invoked. pub async fn on_delete( diff --git a/rust/perspective-client/src/rust/virtual_server/server.rs b/rust/perspective-client/src/rust/virtual_server/server.rs index 0900cda36f..3b4642808d 100644 --- a/rust/perspective-client/src/rust/virtual_server/server.rs +++ b/rust/perspective-client/src/rust/virtual_server/server.rs @@ -28,9 +28,9 @@ use crate::proto::{ ServerError, TableMakePortResp, TableMakeViewResp, TableOnDeleteResp, TableRemoveDeleteResp, TableSchemaResp, TableSizeResp, TableValidateExprResp, ViewColumnPathsResp, ViewDeleteResp, ViewDimensionsResp, ViewExpressionSchemaResp, ViewGetConfigResp, ViewGetMinMaxResp, - ViewOnDeleteResp, ViewOnUpdateResp, ViewRemoveDeleteResp, ViewRemoveOnUpdateResp, - ViewSchemaResp, ViewToArrowResp, ViewToColumnsStringResp, ViewToCsvResp, - ViewToNdjsonStringResp, ViewToRowsStringResp, + ViewOnDeleteResp, ViewOnRemoveResp, ViewOnUpdateResp, ViewRemoveDeleteResp, + ViewRemoveOnRemoveResp, ViewRemoveOnUpdateResp, ViewSchemaResp, ViewToArrowResp, + ViewToColumnsStringResp, ViewToCsvResp, ViewToNdjsonStringResp, ViewToRowsStringResp, }; macro_rules! respond { @@ -450,6 +450,15 @@ impl VirtualServer { TableOnDeleteReq(_) => { respond!(msg, TableOnDeleteResp {}) }, + ViewOnRemoveReq(_) => { + respond!(msg, ViewOnRemoveResp { + indices: None, + port_id: 0 + }) + }, + ViewRemoveOnRemoveReq(_) => { + respond!(msg, ViewRemoveOnRemoveResp {}) + }, ViewOnUpdateReq(_) => { respond!(msg, ViewOnUpdateResp { delta: None, diff --git a/rust/perspective-js/src/rust/lib.rs b/rust/perspective-js/src/rust/lib.rs index 21cf7a4e47..a636d6e2e3 100644 --- a/rust/perspective-js/src/rust/lib.rs +++ b/rust/perspective-js/src/rust/lib.rs @@ -53,6 +53,9 @@ export type * from "../../src/ts/ts-rs/ColumnWindow.d.ts"; export type * from "../../src/ts/ts-rs/TableInitOptions.d.ts"; export type * from "../../src/ts/ts-rs/ViewConfigUpdate.d.ts"; export type * from "../../src/ts/ts-rs/ViewOnUpdateResp.d.ts"; +export type * from "../../src/ts/ts-rs/ViewOnRemoveResp.d.ts"; +export type * from "../../src/ts/ts-rs/OnRemoveData.d.ts"; +export type * from "../../src/ts/ts-rs/OnUpdateData.d.ts"; export type * from "../../src/ts/ts-rs/OnUpdateOptions.d.ts"; export type * from "../../src/ts/ts-rs/UpdateOptions.d.ts"; export type * from "../../src/ts/ts-rs/DeleteOptions.d.ts"; @@ -77,6 +80,8 @@ import type {JoinOptions} from "../../src/ts/ts-rs/JoinOptions.ts"; import type {JoinType} from "../../src/ts/ts-rs/JoinType.ts"; import type {ViewConfigUpdate} from "../../src/ts/ts-rs/ViewConfigUpdate.d.ts"; import type * as on_update_args from "../../src/ts/ts-rs/ViewOnUpdateResp.d.ts"; +import type {OnRemoveData} from "../../src/ts/ts-rs/OnRemoveData.d.ts"; +import type {OnUpdateData} from "../../src/ts/ts-rs/OnUpdateData.d.ts"; import type {OnUpdateOptions} from "../../src/ts/ts-rs/OnUpdateOptions.d.ts"; import type {UpdateOptions} from "../../src/ts/ts-rs/UpdateOptions.d.ts"; import type {DeleteOptions} from "../../src/ts/ts-rs/DeleteOptions.d.ts"; diff --git a/rust/perspective-js/src/rust/view.rs b/rust/perspective-js/src/rust/view.rs index 9ba8a70a56..a9bebf8919 100644 --- a/rust/perspective-js/src/rust/view.rs +++ b/rust/perspective-js/src/rust/view.rs @@ -12,7 +12,7 @@ use js_sys::{Array, ArrayBuffer, Function, Object}; use perspective_client::{ - ColumnWindow, OnUpdateData, OnUpdateOptions, ViewWindow, assert_view_api, + ColumnWindow, OnRemoveData, OnUpdateData, OnUpdateOptions, ViewWindow, assert_view_api, }; use wasm_bindgen::prelude::*; use wasm_bindgen_futures::spawn_local; @@ -289,7 +289,7 @@ impl View { /// table emits an update, this callback will be invoked with an object /// containing `port_id`, indicating which port the update fired on, and /// optionally `delta`, which is the new data that was updated for each - /// cell or each row. + /// cell or each row (as an Arrow). /// /// # Arguments /// @@ -297,24 +297,22 @@ impl View { /// object with two keys: `port_id`, indicating which port the update was /// triggered on, and `delta`, whose value is dependent on the mode /// parameter. - /// - `options` - If this is provided as `OnUpdateOptions { mode: - /// Some(OnUpdateMode::Row) }`, then `delta` is an Arrow of the updated - /// rows. Otherwise `delta` will be [`Option::None`]. + /// - `options` - If this is provided as `{mode: "row"}`, then `delta` is an + /// Arrow of the updated rows. Otherwise `delta` will be `null`. /// /// # JavaScript Examples /// /// ```javascript - /// // Attach an `on_update` callback /// view.on_update((updated) => console.log(updated.port_id)); /// ``` /// /// ```javascript - /// // `on_update` with row deltas /// view.on_update((updated) => console.log(updated.delta), { mode: "row" }); /// ``` #[wasm_bindgen] pub fn on_update( &self, + #[wasm_bindgen(unchecked_param_type = "(data: OnUpdateData) => void")] on_update_js: Function, options: Option, ) -> ApiFuture { @@ -346,6 +344,49 @@ impl View { Ok(self.0.remove_update(callback_id).await?) } + /// Register a callback which is invoked whenever rows are removed from + /// this [`View`]'s [`Table`] by [`Table::remove`], with an object + /// containing `port_id` and `indices`, the removed `index` column + /// values as an Arrow of one column named after the index. + /// + /// [`Table::replace`] reports the keys it does not re-supply and + /// [`Table::clear`] reports every key. `on_remove` never fires for a + /// [`Table`] without an `index`. + /// + /// # JavaScript Examples + /// + /// ```javascript + /// const id = await view.on_remove(({ indices, port_id }) => { + /// replica.remove(indices); + /// }); + /// ``` + #[wasm_bindgen] + pub fn on_remove( + &self, + #[wasm_bindgen(unchecked_param_type = "(data: OnRemoveData) => void")] + on_remove_js: Function, + ) -> ApiFuture { + let poll_loop = LocalPollLoop::new(move |args: OnRemoveData| { + let js_obj = JsValue::from_serde_ext(&*args)?; + on_remove_js.call1(&JsValue::UNDEFINED, &js_obj) + }); + + let on_remove = Box::new(move |msg| poll_loop.poll(msg)); + let view = self.0.clone(); + ApiFuture::new(async move { Ok(view.on_remove(on_remove).await?) }) + } + + /// Unregister a previously registered [`View::on_remove`] callback. + /// + /// # Arguments + /// + /// - `id` - A callback `id` as returned by a reciprocal call to + /// [`View::on_remove`]. + #[wasm_bindgen] + pub async fn remove_remove(&self, callback_id: u32) -> ApiResult<()> { + Ok(self.0.remove_remove(callback_id).await?) + } + /// Register a callback with this [`View`]. Whenever the [`View`] is /// deleted, this callback will be invoked. #[wasm_bindgen] diff --git a/rust/perspective-js/test/js/constructors.spec.js b/rust/perspective-js/test/js/constructors.spec.js index ad4ca853ff..9fb7604c8b 100644 --- a/rust/perspective-js/test/js/constructors.spec.js +++ b/rust/perspective-js/test/js/constructors.spec.js @@ -678,6 +678,152 @@ function validate_typed_array(typed_array, column_data) { view.delete(); table.delete(); }); + + test("Construct a table from an indexed view inherits the index", async function () { + const table = await perspective.table( + [ + { x: 1, y: "a" }, + { x: 2, y: "b" }, + { x: 3, y: "c" }, + ], + { index: "x" }, + ); + + const view = await table.view(); + const table2 = await perspective.table(view); + expect(await table2.get_index()).toEqual("x"); + const view2 = await table2.view(); + let resolve; + const next = () => + new Promise((x) => { + resolve = x; + }); + + await view2.on_update(() => resolve()); + let promise = next(); + await table.update([{ x: 2, y: "bb" }]); + await promise; + expect(await view2.to_json()).toEqual(await view.to_json()); + expect(await view2.to_json()).toEqual([ + { x: 1, y: "a" }, + { x: 2, y: "bb" }, + { x: 3, y: "c" }, + ]); + + promise = next(); + await table.remove([1]); + await promise; + expect(await view2.to_json()).toEqual(await view.to_json()); + expect(await view2.to_json()).toEqual([ + { x: 2, y: "bb" }, + { x: 3, y: "c" }, + ]); + + await view2.delete(); + await table2.delete(); + await view.delete(); + await table.delete(); + }); + + test("Construct a table from an indexed view mirrors replace and clear", async function () { + const table = await perspective.table( + [ + { x: 1, y: "a" }, + { x: 2, y: "b" }, + { x: 3, y: "c" }, + ], + { index: "x" }, + ); + + const view = await table.view(); + const table2 = await perspective.table(view); + const view2 = await table2.view(); + await table.replace([ + { x: 2, y: "bb" }, + { x: 4, y: "d" }, + ]); + + await expect + .poll(() => view2.to_json()) + .toEqual([ + { x: 2, y: "bb" }, + { x: 4, y: "d" }, + ]); + + await table.clear(); + await expect.poll(() => view2.to_json()).toEqual([]); + await table.update([{ x: 5, y: "e" }]); + await expect + .poll(() => view2.to_json()) + .toEqual([{ x: 5, y: "e" }]); + await view2.delete(); + await table2.delete(); + await view.delete(); + await table.delete(); + }); + + test("Construct a table from a pivoted view stays unindexed", async function () { + const table = await perspective.table( + [ + { x: 1, y: "a" }, + { x: 2, y: "b" }, + ], + { index: "x" }, + ); + + const view = await table.view({ group_by: ["y"] }); + const table2 = await perspective.table(view); + expect(await table2.get_index()).toBeUndefined(); + await table2.delete(); + await view.delete(); + await table.delete(); + }); + + test("Construct a table from a view without the index column stays unindexed", async function () { + const table = await perspective.table( + [ + { x: 1, y: "a" }, + { x: 2, y: "b" }, + ], + { index: "x" }, + ); + + const view = await table.view({ columns: ["y"] }); + const table2 = await perspective.table(view); + expect(await table2.get_index()).toBeUndefined(); + await table2.delete(); + await view.delete(); + await table.delete(); + }); + + test("Construct a table from a limit table's view inherits the limit", async function () { + const table = await perspective.table( + [ + { x: 1, y: "a" }, + { x: 2, y: "b" }, + ], + { limit: 2 }, + ); + + const view = await table.view(); + const table2 = await perspective.table(view); + expect(await table2.get_limit()).toEqual(2); + const view2 = await table2.view(); + let resolve; + const promise = new Promise((x) => { + resolve = x; + }); + + await view2.on_update(() => resolve()); + await table.update([{ x: 3, y: "c" }]); + await promise; + expect(await view2.to_json()).toEqual(await view.to_json()); + expect(await table2.size()).toEqual(2); + await view2.delete(); + await table2.delete(); + await view.delete(); + await table.delete(); + }); }); test.describe("Errors", function () { diff --git a/rust/perspective-js/test/js/multi_server.spec.js b/rust/perspective-js/test/js/multi_server.spec.js index 31ac348f10..af9639b743 100644 --- a/rust/perspective-js/test/js/multi_server.spec.js +++ b/rust/perspective-js/test/js/multi_server.spec.js @@ -62,5 +62,33 @@ import perspective from "./perspective_client"; expect(json0).toEqual(json1); }); + + test("Removes transfer to new table", async function ({ page }) { + await page.goto("/rust/perspective-js/test/html/test.html"); + const [json0, json1] = await page.evaluate(async () => { + let perspective = await import( + "http://localhost:6598/node_modules/@perspective-dev/client/dist/esm/perspective.inline.js" + ); + + const worker0 = await perspective.worker(); + const worker1 = await perspective.worker(); + const table0 = await worker0.table("x,y\n1,2\n3,4", { + index: "x", + }); + const view0 = await table0.view(); + const table1 = await worker1.table(view0); + const view1 = await table1.view(); + const { promise, resolve } = Promise.withResolvers(); + await view1.on_update(() => resolve()); + await table0.remove([1]); + await promise; + const json0 = await view0.to_json(); + const json1 = await view1.to_json(); + return [json0, json1]; + }); + + expect(json0).toEqual([{ x: 3, y: 4 }]); + expect(json0).toEqual(json1); + }); }); })(perspective); diff --git a/rust/perspective-js/test/js/on_remove.spec.js b/rust/perspective-js/test/js/on_remove.spec.js new file mode 100644 index 0000000000..3b968fb179 --- /dev/null +++ b/rust/perspective-js/test/js/on_remove.spec.js @@ -0,0 +1,250 @@ +// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +// ┃ Copyright (c) 2017, the Perspective Authors. ┃ +// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +// ┃ This file is part of the Perspective library, distributed under the terms ┃ +// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +import { test, expect } from "@perspective-dev/test"; +import perspective from "./perspective_client"; + +const data = [ + { x: 1, y: "a", z: 1.5 }, + { x: 2, y: "b", z: 2.5 }, + { x: 3, y: "c", z: 3.5 }, +]; + +async function removed_indices(perspective, indices) { + const table = await perspective.table(indices); + const view = await table.view(); + const result = await view.to_json(); + await view.delete(); + await table.delete(); + return result; +} + +async function subscribe(view) { + const calls = []; + let resolve; + const id = await view.on_remove((removed) => { + calls.push(removed); + if (resolve) { + resolve(removed); + } + }); + + const next = () => + new Promise((x) => { + resolve = x; + }); + + return { calls, next, id }; +} + +async function fixture(perspective, index) { + const table = await perspective.table(data, { index }); + const view = await table.view(); + const cleanup = async () => { + await view.delete(); + await table.delete(); + }; + + return { table, view, cleanup }; +} + +((perspective) => { + test.describe("View.on_remove", function () { + test("fires with the removed index values", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const sub = await subscribe(view); + const promise = sub.next(); + await table.remove([2]); + const removed = await promise; + expect(removed.port_id).toEqual(0); + expect(await removed_indices(perspective, removed.indices)).toEqual( + [{ x: 2 }], + ); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("carries string index values", async function () { + const { table, view, cleanup } = await fixture(perspective, "y"); + const sub = await subscribe(view); + const promise = sub.next(); + await table.remove(["b", "c"]); + const removed = await promise; + expect(await removed_indices(perspective, removed.indices)).toEqual( + [{ y: "b" }, { y: "c" }], + ); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("carries float index values", async function () { + const { table, view, cleanup } = await fixture(perspective, "z"); + const sub = await subscribe(view); + const promise = sub.next(); + await table.remove([2.5]); + const removed = await promise; + expect(await removed_indices(perspective, removed.indices)).toEqual( + [{ z: 2.5 }], + ); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("does not fire for index values which do not exist", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const sub = await subscribe(view); + await table.remove([9]); + await table.size(); + const promise = sub.next(); + await table.remove([1]); + await promise; + expect(sub.calls.length).toEqual(1); + expect( + await removed_indices(perspective, sub.calls[0].indices), + ).toEqual([{ x: 1 }]); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("does not fire for `update`", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const sub = await subscribe(view); + await table.update([ + { x: 1, y: "aa", z: 1.5 }, + { x: 4, y: "d", z: 4.5 }, + ]); + await table.size(); + const promise = sub.next(); + await table.remove([3]); + await promise; + expect(sub.calls.length).toEqual(1); + expect( + await removed_indices(perspective, sub.calls[0].indices), + ).toEqual([{ x: 3 }]); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("fires on every view of the table", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const other = await table.view({ columns: ["y"] }); + const first = await subscribe(view); + const second = await subscribe(other); + const promises = [first.next(), second.next()]; + await table.remove([2]); + await Promise.all(promises); + expect( + await removed_indices(perspective, first.calls[0].indices), + ).toEqual([{ x: 2 }]); + expect( + await removed_indices(perspective, second.calls[0].indices), + ).toEqual([{ x: 2 }]); + await view.remove_remove(first.id); + await other.remove_remove(second.id); + await other.delete(); + await cleanup(); + }); + + test("fires for `replace` with the keys not re-supplied", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const sub = await subscribe(view); + const promise = sub.next(); + await table.replace([ + { x: 2, y: "bb", z: 2.5 }, + { x: 4, y: "d", z: 4.5 }, + ]); + const removed = await promise; + expect(sub.calls.length).toEqual(1); + expect(await removed_indices(perspective, removed.indices)).toEqual( + [{ x: 1 }, { x: 3 }], + ); + expect(await view.to_json()).toEqual([ + { x: 2, y: "bb", z: 2.5 }, + { x: 4, y: "d", z: 4.5 }, + ]); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("fires for `clear` with every key", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const sub = await subscribe(view); + let updates = 0; + await view.on_update(() => { + updates += 1; + }); + const promise = sub.next(); + await table.clear(); + const removed = await promise; + expect(sub.calls.length).toEqual(1); + expect(await removed_indices(perspective, removed.indices)).toEqual( + [{ x: 1 }, { x: 2 }, { x: 3 }], + ); + expect(await view.to_json()).toEqual([]); + await expect.poll(() => updates).toEqual(1); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("`remove_remove` stops delivery", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const first = await subscribe(view); + await view.remove_remove(first.id); + const second = await subscribe(view); + const promise = second.next(); + await table.remove([1]); + await promise; + expect(first.calls.length).toEqual(0); + expect(second.calls.length).toEqual(1); + await view.remove_remove(second.id); + await cleanup(); + }); + + test("`remove` accepts an Arrow of index values", async function () { + const { table, view, cleanup } = await fixture(perspective, "x"); + const keys = await perspective.table({ x: [2, 3] }); + const keys_view = await keys.view(); + const arrow = await keys_view.to_arrow(); + const sub = await subscribe(view); + const promise = sub.next(); + await table.remove(arrow); + const removed = await promise; + expect(await removed_indices(perspective, removed.indices)).toEqual( + [{ x: 2 }, { x: 3 }], + ); + + expect(await view.to_json()).toEqual([{ x: 1, y: "a", z: 1.5 }]); + await keys_view.delete(); + await keys.delete(); + await view.remove_remove(sub.id); + await cleanup(); + }); + + test("`remove` rejects an Arrow without the index column", async function () { + const { table, cleanup } = await fixture(perspective, "x"); + const keys = await perspective.table({ q: [2] }); + const keys_view = await keys.view(); + const arrow = await keys_view.to_arrow(); + let message = ""; + try { + await table.remove(arrow); + } catch (error) { + message = error.message; + } + + expect(message).toContain("missing index column"); + await keys_view.delete(); + await keys.delete(); + await cleanup(); + }); + }); +})(perspective); diff --git a/rust/perspective-python/perspective/tests/table/test_on_remove.py b/rust/perspective-python/perspective/tests/table/test_on_remove.py new file mode 100644 index 0000000000..d56be765ef --- /dev/null +++ b/rust/perspective-python/perspective/tests/table/test_on_remove.py @@ -0,0 +1,140 @@ +# ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓ +# ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃ +# ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃ +# ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃ +# ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃ +# ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫ +# ┃ Copyright (c) 2017, the Perspective Authors. ┃ +# ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃ +# ┃ This file is part of the Perspective library, distributed under the terms ┃ +# ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃ +# ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛ + +import perspective as psp + +client = psp.Server().new_local_client() +Table = client.table + +data = [{"x": 1, "y": "a"}, {"x": 2, "y": "b"}, {"x": 3, "y": "c"}] + + +def removed_indices(indices): + return Table(indices).view().to_records() + + +class TestOnRemove(object): + def test_on_remove_fires_with_removed_index_values(self): + tbl = Table(data, index="x") + view = tbl.view() + calls = [] + cid = view.on_remove(lambda port_id, indices: calls.append((port_id, indices))) + tbl.remove([2]) + assert len(calls) == 1 + assert calls[0][0] == 0 + assert removed_indices(calls[0][1]) == [{"x": 2}] + view.remove_remove(cid) + + def test_on_remove_string_index(self): + tbl = Table(data, index="y") + view = tbl.view() + calls = [] + cid = view.on_remove(lambda port_id, indices: calls.append(indices)) + tbl.remove(["b", "c"]) + assert len(calls) == 1 + assert removed_indices(calls[0]) == [{"y": "b"}, {"y": "c"}] + view.remove_remove(cid) + + def test_on_remove_ignores_unknown_and_update(self): + tbl = Table(data, index="x") + view = tbl.view() + calls = [] + cid = view.on_remove(lambda port_id, indices: calls.append(indices)) + tbl.remove([9]) + tbl.update([{"x": 1, "y": "aa"}, {"x": 4, "y": "d"}]) + assert tbl.size() == 4 + assert calls == [] + tbl.remove([3]) + assert len(calls) == 1 + assert removed_indices(calls[0]) == [{"x": 3}] + view.remove_remove(cid) + + def test_on_remove_replace_reports_keys_not_resupplied(self): + tbl = Table(data, index="x") + view = tbl.view() + calls = [] + cid = view.on_remove(lambda port_id, indices: calls.append(indices)) + tbl.replace([{"x": 2, "y": "bb"}, {"x": 4, "y": "d"}]) + assert len(calls) == 1 + assert removed_indices(calls[0]) == [{"x": 1}, {"x": 3}] + assert view.to_records() == [{"x": 2, "y": "bb"}, {"x": 4, "y": "d"}] + view.remove_remove(cid) + + def test_on_remove_clear_reports_every_key(self): + tbl = Table(data, index="x") + view = tbl.view() + calls = [] + updates = [] + cid = view.on_remove(lambda port_id, indices: calls.append(indices)) + view.on_update(lambda port_id: updates.append(port_id)) + tbl.clear() + assert len(calls) == 1 + assert removed_indices(calls[0]) == [{"x": 1}, {"x": 2}, {"x": 3}] + assert view.to_records() == [] + assert updates == [0] + view.remove_remove(cid) + + def test_remove_remove_stops_delivery(self): + tbl = Table(data, index="x") + view = tbl.view() + first = [] + second = [] + cid = view.on_remove(lambda port_id, indices: first.append(indices)) + view.remove_remove(cid) + cid2 = view.on_remove(lambda port_id, indices: second.append(indices)) + tbl.remove([1]) + assert first == [] + assert len(second) == 1 + view.remove_remove(cid2) + + def test_remove_accepts_arrow(self): + tbl = Table(data, index="x") + arrow = Table({"x": [2, 3]}).view().to_arrow() + tbl.remove(arrow) + assert tbl.view().to_records() == [{"x": 1, "y": "a"}] + + def test_table_from_view_inherits_index_and_replicates_removes(self): + tbl = Table(data, index="x") + view = tbl.view() + tbl2 = Table(view) + assert tbl2.get_index() == "x" + view2 = tbl2.view() + + tbl.update([{"x": 2, "y": "bb"}]) + assert tbl2.size() == 3 + assert view2.to_records() == view.to_records() + + tbl.remove([1]) + assert tbl2.size() == 2 + assert view2.to_records() == [{"x": 2, "y": "bb"}, {"x": 3, "y": "c"}] + assert view2.to_records() == view.to_records() + + def test_table_from_view_mirrors_replace_and_clear(self): + tbl = Table(data, index="x") + view = tbl.view() + tbl2 = Table(view) + view2 = tbl2.view() + + tbl.replace([{"x": 2, "y": "bb"}, {"x": 4, "y": "d"}]) + assert view2.to_records() == [{"x": 2, "y": "bb"}, {"x": 4, "y": "d"}] + + tbl.clear() + assert view2.to_records() == [] + + tbl.update([{"x": 5, "y": "e"}]) + assert view2.to_records() == [{"x": 5, "y": "e"}] + + def test_table_from_pivoted_view_stays_unindexed(self): + tbl = Table(data, index="x") + view = tbl.view(group_by=["y"]) + tbl2 = Table(view) + assert tbl2.get_index() is None diff --git a/rust/perspective-python/src/client/client_async.rs b/rust/perspective-python/src/client/client_async.rs index a9ece51f26..e0b1d1eb53 100644 --- a/rust/perspective-python/src/client/client_async.rs +++ b/rust/perspective-python/src/client/client_async.rs @@ -18,8 +18,8 @@ use std::sync::Arc; use futures::FutureExt; use perspective_client::proto::ListFlatten; use perspective_client::{ - Client, ColumnWindow, DeleteOptions, OnUpdateData, OnUpdateMode, OnUpdateOptions, Table, - TableData, TableInitOptions, TableReadFormat, TableRef, UpdateData, UpdateOptions, View, + Client, ColumnWindow, DeleteOptions, OnRemoveData, OnUpdateData, OnUpdateMode, OnUpdateOptions, + Table, TableData, TableInitOptions, TableReadFormat, TableRef, UpdateData, UpdateOptions, View, ViewWindow, assert_table_api, assert_view_api, asyncfn, }; use pyo3::exceptions::PyValueError; @@ -920,6 +920,59 @@ impl AsyncView { self.view.remove_update(callback_id).await.into_pyerr() } + /// Register a callback which is invoked whenever rows are removed from + /// this [`View`]'s [`Table`] by [`Table::remove`], with two arguments: + /// `port_id`, + /// and `indices`, the removed `index` column values as an Apache Arrow + /// (`bytes`) of one column named after the index. + /// + /// [`Table::replace`] reports the keys it does not re-supply and + /// [`Table::clear`] reports every key. It never fires for a + /// [`Table`] without an `index`. + /// + /// # Python Examples + /// + /// ```python + /// def on_remove(port_id, indices): + /// replica.remove(indices) + /// + /// callback_id = await view.on_remove(on_remove) + /// ``` + pub async fn on_remove(&self, callback: Py) -> PyResult { + let callback = move |x: OnRemoveData| { + let callback = Python::with_gil(|py| Py::clone_ref(&callback, py)); + async move { + let aggregate_errors: PyResult<()> = Python::with_gil(|py| { + match &x.indices { + None => callback.call1(py, (x.port_id,))?, + Some(indices) => { + callback.call1(py, (x.port_id, PyBytes::new(py, indices)))? + }, + }; + + Ok(()) + }); + + if let Err(err) = aggregate_errors { + tracing::warn!("Error in on_remove callback: {:?}", err); + } + } + .boxed() + }; + + self.view.on_remove(Box::new(callback)).await.into_pyerr() + } + + /// Unregister a previously registered [`View::on_remove`] callback. + /// + /// # Arguments + /// + /// - `id` - A callback `id` as returned by a reciprocal call to + /// [`View::on_remove`]. + pub async fn remove_remove(&self, callback_id: u32) -> PyResult<()> { + self.view.remove_remove(callback_id).await.into_pyerr() + } + #[pyo3(signature=(**window))] pub async fn to_dataframe(&self, window: Option>) -> PyResult> { let window: ViewWindow = Python::with_gil(|py| window.map(|x| depythonize(x.bind(py)))) diff --git a/rust/perspective-python/src/client/client_sync.rs b/rust/perspective-python/src/client/client_sync.rs index b847d25a21..f96b2f9683 100644 --- a/rust/perspective-python/src/client/client_sync.rs +++ b/rust/perspective-python/src/client/client_sync.rs @@ -825,4 +825,36 @@ impl View { pub fn remove_update(&self, py: Python<'_>, callback_id: u32) -> PyResult<()> { self.0.remove_update(callback_id).py_block_on(py) } + + /// Register a callback which is invoked whenever rows are removed from + /// this [`View`]'s [`Table`] by [`Table::remove`], with two arguments: + /// `port_id`, + /// and `indices`, the removed `index` column values as an Apache Arrow + /// (`bytes`) of one column named after the index. + /// + /// [`Table::replace`] reports the keys it does not re-supply and + /// [`Table::clear`] reports every key. It never fires for a + /// [`Table`] without an `index`. + /// + /// # Python Examples + /// + /// ```python + /// def on_remove(port_id, indices): + /// replica.remove(indices) + /// + /// callback_id = view.on_remove(on_remove) + /// ``` + pub fn on_remove(&self, py: Python<'_>, callback: Py) -> PyResult { + self.0.on_remove(callback).py_block_on(py) + } + + /// Unregister a previously registered [`View::on_remove`] callback. + /// + /// # Arguments + /// + /// - `id` - A callback `id` as returned by a reciprocal call to + /// [`View::on_remove`]. + pub fn remove_remove(&self, py: Python<'_>, callback_id: u32) -> PyResult<()> { + self.0.remove_remove(callback_id).py_block_on(py) + } } diff --git a/rust/perspective-server/cpp/perspective/src/cpp/arrow_writer.cpp b/rust/perspective-server/cpp/perspective/src/cpp/arrow_writer.cpp index 536f355e53..987ae0c61a 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/arrow_writer.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/arrow_writer.cpp @@ -78,4 +78,130 @@ get_scalar(t_tscalar& t) { // return (ridx - extents.m_srow) * stride + (cidx - extents.m_scol); // } + +std::shared_ptr +column_to_arrow_ipc( + const t_column& col, const std::string& name, t_uindex nrows +) { + t_get_data_extents extents{ + 0, static_cast(nrows), 0, 1 + }; + auto get = [&col](t_uindex ridx) { return col.get_scalar(ridx); }; + std::shared_ptr field; + std::shared_ptr array; + switch (col.get_dtype()) { + case DTYPE_INT8: { + field = arrow::field(name, arrow::int8()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_UINT8: { + field = arrow::field(name, arrow::uint8()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_INT16: { + field = arrow::field(name, arrow::int16()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_UINT16: { + field = arrow::field(name, arrow::uint16()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_INT32: { + field = arrow::field(name, arrow::int32()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_UINT32: { + field = arrow::field(name, arrow::uint32()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_INT64: { + field = arrow::field(name, arrow::int64()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_UINT64: { + field = arrow::field(name, arrow::uint64()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_FLOAT32: { + field = arrow::field(name, arrow::float32()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_FLOAT64: { + field = arrow::field(name, arrow::float64()); + array = numeric_col_to_array( + extents, get + ); + } break; + case DTYPE_DATE: { + field = arrow::field(name, arrow::date32()); + array = date_col_to_array(extents, get); + } break; + case DTYPE_TIME: { + field = arrow::field( + name, arrow::timestamp(arrow::TimeUnit::MILLI) + ); + array = timestamp_col_to_array(extents, get); + } break; + case DTYPE_BOOL: { + field = arrow::field(name, arrow::boolean()); + array = boolean_col_to_array(extents, get); + } break; + case DTYPE_STR: { + field = arrow::field( + name, arrow::dictionary(arrow::int32(), arrow::utf8()) + ); + array = string_col_to_dictionary_array(extents, get); + } break; + default: { + std::stringstream ss; + ss << "Cannot serialize column `" << name << "` of type `" + << get_dtype_descr(col.get_dtype()) << "` to Arrow format." + << std::endl; + PSP_COMPLAIN_AND_ABORT(ss.str()); + } + } + + auto schema = arrow::schema({field}); + auto batch = arrow::RecordBatch::Make( + schema, static_cast(nrows), {array} + ); + arrow::Result> allocated = + arrow::AllocateResizableBuffer(0); + if (!allocated.ok()) { + std::stringstream ss; + ss << "Failed to allocate buffer: " << allocated.status().message() + << std::endl; + PSP_COMPLAIN_AND_ABORT(ss.str()); + } + + std::shared_ptr buffer = *allocated; + arrow::io::BufferOutputStream sink(buffer); + auto options = arrow::ipc::IpcWriteOptions::Defaults(); + options.use_threads = false; + auto res = arrow::ipc::MakeStreamWriter(&sink, schema, options); + std::shared_ptr writer = *res; + PSP_CHECK_ARROW_STATUS(writer->WriteRecordBatch(*batch)); + PSP_CHECK_ARROW_STATUS(writer->Close()); + PSP_CHECK_ARROW_STATUS(sink.Close()); + return std::make_shared(buffer->ToString()); +} + } // namespace perspective::apachearrow \ No newline at end of file diff --git a/rust/perspective-server/cpp/perspective/src/cpp/gnode.cpp b/rust/perspective-server/cpp/perspective/src/cpp/gnode.cpp index 3be3eaff85..cef10fc6a9 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/gnode.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/gnode.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -223,6 +224,30 @@ t_gnode::calc_transition( return trans; } +static t_column* +append_removed_pkey( + std::shared_ptr& removed, + t_column* removed_col, + t_dtype dtype, + t_uindex reserve, + const t_tscalar& pkey +) { + if (removed_col == nullptr) { + if (!removed) { + t_schema removed_schema({"psp_pkey"}, {dtype}); + removed = std::make_shared(removed_schema); + removed->init(); + removed->reserve(reserve); + } + removed_col = removed->_get_column("psp_pkey"); + } + + const t_uindex idx = removed->size(); + removed->extend(idx + 1); + removed_col->set_scalar(idx, pkey); + return removed_col; +} + t_mask t_gnode::_process_mask_existed_rows(t_process_state& process_state) { // Make sure `existed_data_table` has enough space to write without resizing @@ -243,6 +268,8 @@ t_gnode::_process_mask_existed_rows(t_process_state& process_state) { t_tscalar prev_pkey; prev_pkey.clear(); + t_column* removed_col = nullptr; + t_column* existed_column = process_state.m_existed_data_table->_get_column("psp_existed"); @@ -284,6 +311,15 @@ t_gnode::_process_mask_existed_rows(t_process_state& process_state) { existed_column->set_nth(added_count, row_pre_existed); widened_column->set_nth(added_count, false); ++added_count; + if (m_removes_enabled) { + removed_col = append_removed_pkey( + m_removed_pkeys, + removed_col, + pkey_col->get_dtype(), + flattened_num_rows, + pkey + ); + } } else { mask.set(idx, false); } @@ -303,6 +339,7 @@ t_gnode::_process_mask_existed_rows(t_process_state& process_state) { t_process_table_result t_gnode::_process_table(t_uindex port_id) { m_was_updated = false; + m_removed_pkeys = nullptr; t_process_table_result result; result.m_flattened_data_table = nullptr; @@ -318,7 +355,18 @@ t_gnode::_process_table(t_uindex port_id) { std::shared_ptr& input_port = m_input_ports[port_id]; + // A `reset` with no rows queued behind it (a bare `clear`) still + // produces one step, so listeners see the table empty and its keys gone. if (input_port->get_table()->size() == 0) { + if (!m_reset_pending) { + return result; + } + + m_reset_pending = false; + m_removed_pkeys = std::move(m_reset_pkeys); + m_reset_pkeys = nullptr; + m_was_updated = true; + result.m_should_notify_userspace = true; return result; } @@ -339,6 +387,36 @@ t_gnode::_process_table(t_uindex port_id) { row_lookup[idx] = m_gstate->lookup(pkey); } + if (m_reset_pending) { + m_reset_pending = false; + if (m_reset_pkeys) { + tsl::hopscotch_set incoming; + incoming.reserve(flattened_num_rows); + for (t_uindex idx = 0; idx < flattened_num_rows; ++idx) { + incoming.insert(pkey_col->get_scalar(idx)); + } + + const t_column* stash_col = + m_reset_pkeys->_get_column("psp_pkey"); + const t_uindex stash_size = m_reset_pkeys->size(); + t_column* removed_col = nullptr; + for (t_uindex idx = 0; idx < stash_size; ++idx) { + t_tscalar pkey = stash_col->get_scalar(idx); + if (!incoming.contains(pkey)) { + removed_col = append_removed_pkey( + m_removed_pkeys, + removed_col, + stash_col->get_dtype(), + stash_size, + pkey + ); + } + } + + m_reset_pkeys = nullptr; + } + } + // first update - master table is empty if (m_gstate->mapping_size() == 0) { m_gstate->update_master_table(flattened); @@ -1894,6 +1972,32 @@ void t_gnode::reset() { std::vector rval; + m_reset_pending = true; + if (m_removes_enabled && m_gstate->mapping_size() > 0) { + const auto& mapping = m_gstate->get_pkey_map(); + const auto master = m_gstate->get_table(); + t_mask live(master->size()); + for (const auto& kv : mapping) { + live.set(kv.second, true); + } + + const auto pkey_col = master->get_const_column("psp_pkey"); + t_column* stash_col = nullptr; + for (t_uindex idx = 0; idx < master->size(); ++idx) { + if (!live.get(idx)) { + continue; + } + + stash_col = append_removed_pkey( + m_reset_pkeys, + stash_col, + pkey_col->get_dtype(), + mapping.size(), + pkey_col->get_scalar(idx) + ); + } + } + for (const auto& kv : m_contexts) { auto ctxh = kv.second; switch (ctxh.m_ctx_type) { @@ -1945,6 +2049,25 @@ t_gnode::clear_output_ports() { for (const auto& m_oport : m_oports) { m_oport->get_table()->clear(); } + m_removed_pkeys = nullptr; +} + +std::shared_ptr +t_gnode::get_removed_pkeys() const { + return m_removed_pkeys; +} + +void +t_gnode::set_removes_enabled(bool enabled) { + m_removes_enabled = enabled; + if (!enabled) { + m_reset_pkeys = nullptr; + } +} + +bool +t_gnode::get_removes_enabled() const { + return m_removes_enabled; } void diff --git a/rust/perspective-server/cpp/perspective/src/cpp/pool.cpp b/rust/perspective-server/cpp/perspective/src/cpp/pool.cpp index 914b1b3c81..131c6f23c6 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/pool.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/pool.cpp @@ -105,6 +105,19 @@ t_pool::unregister_gnode(t_uindex idx) { m_gnodes[idx] = nullptr; } +void +t_pool::reset_gnode(t_uindex gnode_id) { + { +#ifdef PSP_PARALLEL_FOR + PSP_WRITE_LOCK(*m_lock); +#endif + m_data_remaining.store(true); + if (m_gnodes[gnode_id] != nullptr) { + m_gnodes[gnode_id]->reset(); + } + } +} + void t_pool::send(t_uindex gnode_id, t_uindex port_id, const t_data_table& table) { { diff --git a/rust/perspective-server/cpp/perspective/src/cpp/server.cpp b/rust/perspective-server/cpp/perspective/src/cpp/server.cpp index e3b5d19b0b..272cfd828a 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/server.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/server.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -588,9 +589,10 @@ ServerResources::delete_view(const std::uint32_t& client_id, const t_id& id) { throw PerspectiveViewNotFoundException(); } + t_id table_id; { PSP_WRITE_LOCK(m_write_lock); - auto table_id = m_view_to_table.at(id); + table_id = m_view_to_table.at(id); if (m_views.find(id) != m_views.end()) { m_views.erase(id); } @@ -613,6 +615,12 @@ ServerResources::delete_view(const std::uint32_t& client_id, const t_id& id) { drop_view_on_update_sub(id); drop_view_on_delete_sub(id); + drop_view_on_remove_sub(id); + if (m_tables.contains(table_id)) { + m_tables.at(table_id)->get_gnode()->set_removes_enabled( + table_has_on_remove_subs(table_id) + ); + } } void @@ -693,6 +701,69 @@ ServerResources::remove_table_on_delete_sub( } } +void +ServerResources::create_view_on_remove_sub( + const t_id& view_id, Subscription sub +) { + PSP_WRITE_LOCK(m_write_lock); + if (!m_view_on_remove_subs.contains(view_id)) { + m_view_on_remove_subs[view_id] = {sub}; + } else { + m_view_on_remove_subs[view_id].push_back(sub); + } +} + +std::vector +ServerResources::get_view_on_remove_sub(const t_id& view_id) { + PSP_READ_LOCK(m_write_lock); + if (!m_view_on_remove_subs.contains(view_id)) { + return {}; + } + return m_view_on_remove_subs.at(view_id); +} + +void +ServerResources::remove_view_on_remove_sub( + const t_id& view_id, + const std::uint32_t sub_id, + const std::uint32_t client_id +) { + PSP_WRITE_LOCK(m_write_lock); + if (!m_view_on_remove_subs.contains(view_id)) { + return; + } + + auto& subs = m_view_on_remove_subs.at(view_id); + for (auto sub = subs.begin(); sub != subs.end();) { + if (sub->id == sub_id && sub->client_id == client_id) { + subs.erase(sub); + break; + } + + ++sub; + } +} + +void +ServerResources::drop_view_on_remove_sub(const t_id& view_id) { + PSP_WRITE_LOCK(m_write_lock); + m_view_on_remove_subs.erase(view_id); +} + +bool +ServerResources::table_has_on_remove_subs(const t_id& table_id) { + PSP_READ_LOCK(m_write_lock); + auto range = m_table_to_view.equal_range(table_id); + for (auto it = range.first; it != range.second; ++it) { + auto subs = m_view_on_remove_subs.find(it->second); + if (subs != m_view_on_remove_subs.end() && !subs->second.empty()) { + return true; + } + } + + return false; +} + void ServerResources::create_view_on_delete_sub( const t_id& view_id, Subscription sub @@ -1180,6 +1251,8 @@ needs_poll(const proto::Request::ClientReqCase proto_case) { return true; case ReqCase::kTableOnDeleteReq: case ReqCase::kViewOnDeleteReq: + case ReqCase::kViewOnRemoveReq: + case ReqCase::kViewRemoveOnRemoveReq: case ReqCase::kViewRemoveDeleteReq: case ReqCase::kTableUpdateReq: case ReqCase::kTableRemoveDeleteReq: @@ -1225,6 +1298,8 @@ entity_type_is_table(const proto::Request::ClientReqCase proto_case) { case ReqCase::kMakeJoinTableReq: return true; case ReqCase::kViewOnDeleteReq: + case ReqCase::kViewOnRemoveReq: + case ReqCase::kViewRemoveOnRemoveReq: case ReqCase::kViewRemoveDeleteReq: case ReqCase::kViewDimensionsReq: case ReqCase::kViewToColumnsStringReq: @@ -2197,7 +2272,10 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) { table->remove_rows(r.data().from_rows()); break; } - case proto::MakeTableData::kFromArrow: + case proto::MakeTableData::kFromArrow: { + table->remove_arrow(r.data().from_arrow()); + break; + } case proto::MakeTableData::kFromCsv: case proto::MakeTableData::kFromSchema: case proto::MakeTableData::DATA_NOT_SET: @@ -2841,6 +2919,31 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) { m_resources.create_table_on_delete_sub(req.entity_id(), sub_info); break; } + case proto::Request::kViewOnRemoveReq: { + Subscription sub_info; + sub_info.id = req.msg_id(); + sub_info.client_id = client_id; + m_resources.create_view_on_remove_sub(req.entity_id(), sub_info); + auto table_id = m_resources.get_table_id_for_view(req.entity_id()); + m_resources.get_table(table_id)->get_gnode()->set_removes_enabled( + true + ); + break; + } + case proto::Request::kViewRemoveOnRemoveReq: { + auto sub_id = req.view_remove_on_remove_req().id(); + m_resources.remove_view_on_remove_sub( + req.entity_id(), sub_id, client_id + ); + auto table_id = m_resources.get_table_id_for_view(req.entity_id()); + m_resources.get_table(table_id)->get_gnode()->set_removes_enabled( + m_resources.table_has_on_remove_subs(table_id) + ); + proto::Response resp; + resp.mutable_view_remove_on_remove_resp(); + push_resp(std::move(resp)); + break; + } case proto::Request::kTableRemoveDeleteReq: { auto sub_id = req.table_remove_delete_req().id(); m_resources.remove_table_on_delete_sub( @@ -3681,7 +3784,12 @@ ProtoServer::_process_table_unchecked( const ServerResources::t_id& table_id, std::vector>& outs ) { - table->get_pool()->_process([this, table_id, &outs](auto port_id) { + table->get_pool()->_process([this, table, table_id, &outs](auto port_id) { + const auto removed = table->get_gnode()->get_removed_pkeys(); + const bool has_removes = + !table->get_index().empty() && removed && removed->size() > 0; + std::shared_ptr removed_indices; + // record changes per port. auto view_ids = m_resources.get_view_ids(table_id); for (const auto& view_id : view_ids) { @@ -3690,6 +3798,30 @@ ProtoServer::_process_table_unchecked( } auto view = m_resources.get_view(view_id); + if (has_removes) { + auto remove_subs = m_resources.get_view_on_remove_sub(view_id); + if (!remove_subs.empty() && !removed_indices) { + removed_indices = apachearrow::column_to_arrow_ipc( + *removed->get_const_column("psp_pkey"), + table->get_index(), + removed->size() + ); + } + + for (auto& subscription : remove_subs) { + Response out; + out.set_msg_id(subscription.id); + out.set_entity_id(view_id); + auto* r = out.mutable_view_on_remove_resp(); + r->set_port_id(port_id); + *r->mutable_indices() = *removed_indices; + ProtoServerResp resp2; + resp2.data = std::move(out); + resp2.client_id = subscription.client_id; + outs.emplace_back(std::move(resp2)); + } + } + auto subscriptions = m_resources.get_view_on_update_sub(view_id); for (auto& subscription : subscriptions) { Response out; diff --git a/rust/perspective-server/cpp/perspective/src/cpp/table.cpp b/rust/perspective-server/cpp/perspective/src/cpp/table.cpp index d4fde0b037..f30f121ba3 100644 --- a/rust/perspective-server/cpp/perspective/src/cpp/table.cpp +++ b/rust/perspective-server/cpp/perspective/src/cpp/table.cpp @@ -232,8 +232,7 @@ Table::unregister_gnode(t_uindex id) const { void Table::reset_gnode(t_uindex id) const { PSP_VERBOSE_ASSERT(m_init, "touching uninited object"); - t_gnode* gnode = m_pool->get_gnode(id); - gnode->reset(); + m_pool->reset_gnode(id); } t_uindex @@ -579,6 +578,37 @@ Table::remove_cols(const std::string_view& data) { m_pool->send(get_gnode()->get_id(), 0, data_table); } +void +Table::remove_arrow(const std::string_view& data) { + if (m_index.empty()) { + PSP_COMPLAIN_AND_ABORT("Cannot remove from unindexed Table\n") + } + + apachearrow::ArrowLoader arrow_loader; + arrow_loader.initialize( + reinterpret_cast(data.data()), + data.size(), + m_list_flatten + ); + + const auto names = arrow_loader.names(); + if (std::find(names.begin(), names.end(), m_index) == names.end()) { + std::stringstream ss; + ss << "Cannot remove: Arrow is missing index column `" << m_index + << "`\n"; + PSP_COMPLAIN_AND_ABORT(ss.str()); + } + + const t_schema& output_schema = get_gnode()->get_output_schema(); + t_schema schema({m_index}, {output_schema.get_dtype(m_index)}); + t_data_table data_table(schema); + data_table.init(); + data_table.extend(arrow_loader.row_count()); + arrow_loader.fill_table(data_table, schema, m_index, m_offset, true); + process_op_column(data_table, OP_DELETE); + m_pool->send(get_gnode()->get_id(), 0, data_table); +} + std::shared_ptr
Table::from_json_loader( json::JsonLoader& loader, diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/arrow_writer.h b/rust/perspective-server/cpp/perspective/src/include/perspective/arrow_writer.h index 6e600c53a2..568f36e594 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/arrow_writer.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/arrow_writer.h @@ -62,6 +62,10 @@ namespace apachearrow { t_get_data_extents extents ); + std::shared_ptr column_to_arrow_ipc( + const t_column& col, const std::string& name, t_uindex nrows + ); + /** * @brief Build an `arrow::Array` from a column typed as `DTYPE_BOOL.` * diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/gnode.h b/rust/perspective-server/cpp/perspective/src/include/perspective/gnode.h index 303286513d..6f94658dfb 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/gnode.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/gnode.h @@ -137,6 +137,17 @@ class PERSPECTIVE_EXPORT t_gnode { */ bool process(t_uindex port_id); + /** + * @brief The primary keys of rows that existed before the most recent + * `process` and were removed by it, as a one-column `psp_pkey` + * `t_data_table`, or `nullptr` when that step removed nothing or removes + * are not enabled. + */ + std::shared_ptr get_removed_pkeys() const; + + void set_removes_enabled(bool enabled); + bool get_removes_enabled() const; + /** * @brief Create a new input port, store it in `m_input_ports`, and * return the integer ID that references the new port. @@ -434,6 +445,10 @@ class PERSPECTIVE_EXPORT t_gnode { std::chrono::high_resolution_clock::time_point m_epoch; std::function m_pool_cleanup; bool m_was_updated; + bool m_removes_enabled = false; + bool m_reset_pending = false; + std::shared_ptr m_removed_pkeys; + std::shared_ptr m_reset_pkeys; std::shared_ptr m_expression_vocab; std::shared_ptr m_expression_regex_mapping; diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/pool.h b/rust/perspective-server/cpp/perspective/src/include/perspective/pool.h index 209857d180..62986d39e9 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/pool.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/pool.h @@ -75,6 +75,8 @@ class PERSPECTIVE_EXPORT t_pool { void send(t_uindex gnode_id, t_uindex port_id, const t_data_table& table); + void reset_gnode(t_uindex gnode_id); + void _process( std::optional> callback = std::nullopt diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/server.h b/rust/perspective-server/cpp/perspective/src/include/perspective/server.h index aecdaa8588..1acc1a4ac3 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/server.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/server.h @@ -581,6 +581,15 @@ namespace server { const t_id& table_id, std::uint32_t sub_id, std::uint32_t client_id ); + // `View::on_remove()` + void create_view_on_remove_sub(const t_id& view_id, Subscription sub); + std::vector get_view_on_remove_sub(const t_id& view_id); + void remove_view_on_remove_sub( + const t_id& view_id, std::uint32_t sub_id, std::uint32_t client_id + ); + void drop_view_on_remove_sub(const t_id& view_id); + bool table_has_on_remove_subs(const t_id& table_id); + // `View::on_delete()` void create_view_on_delete_sub(const t_id& view_id, Subscription sub); std::vector get_view_on_delete_sub(const t_id& view_id); @@ -628,6 +637,9 @@ namespace server { tsl::hopscotch_map> m_table_on_delete_subs; + tsl::hopscotch_map> + m_view_on_remove_subs; + std::vector m_on_hosted_tables_update_subs; tsl::hopscotch_set m_dirty_tables; diff --git a/rust/perspective-server/cpp/perspective/src/include/perspective/table.h b/rust/perspective-server/cpp/perspective/src/include/perspective/table.h index 5e162fcfcf..93d3a09635 100644 --- a/rust/perspective-server/cpp/perspective/src/include/perspective/table.h +++ b/rust/perspective-server/cpp/perspective/src/include/perspective/table.h @@ -220,6 +220,7 @@ class PERSPECTIVE_EXPORT Table { void remove_cols(const std::string_view& data); void remove_rows(const std::string_view& data); + void remove_arrow(const std::string_view& data); void update_arrow(const std::string_view& data, std::uint32_t port_id); void update_csv(const std::string_view& data, std::uint32_t port_id); diff --git a/rust/perspective-viewer/src/rust/lib.rs b/rust/perspective-viewer/src/rust/lib.rs index 73bc4dfeab..b5cf7b8f7a 100644 --- a/rust/perspective-viewer/src/rust/lib.rs +++ b/rust/perspective-viewer/src/rust/lib.rs @@ -71,19 +71,21 @@ use crate::utils::define_web_component; const TS_APPEND_CONTENT: &'static str = r#" import type { ColumnType, - TableInitOptions, ColumnWindow, - ViewWindow, - TypedArrayWindow, - OnUpdateOptions, + DeleteOptions, + Features, JoinOptions, + OnRemoveData, + OnUpdateData, + OnUpdateOptions, + Scalar, + SystemInfo, + TableInitOptions, + TypedArrayWindow, UpdateOptions, - DeleteOptions, ViewConfig, ViewConfigUpdate, - SystemInfo, - Scalar, - Features, + ViewWindow, } from "@perspective-dev/client"; export type * from "../../src/ts/ts-rs/ViewerConfig.d.ts"; @@ -107,6 +109,7 @@ export type * from "../../src/ts/ts-rs/DatetimeFormatType.d.ts"; export type * from "../../src/ts/ts-rs/StringColorMode.d.ts"; export type * from "../../src/ts/ts-rs/DatetimeColorMode.d.ts"; export type * from "../../src/ts/ts-rs/FormatMode.d.ts"; + import type {GetTableOptions} from "../../src/ts/ts-rs/GetTableOptions.d.ts"; import type {PanelOptions} from "../../src/ts/ts-rs/PanelOptions.d.ts"; import type {RestoreOptions} from "../../src/ts/ts-rs/RestoreOptions.d.ts"; diff --git a/rust/perspective-viewer/src/rust/presentation.rs b/rust/perspective-viewer/src/rust/presentation.rs index 78d97bef70..834f8fe3b1 100644 --- a/rust/perspective-viewer/src/rust/presentation.rs +++ b/rust/perspective-viewer/src/rust/presentation.rs @@ -30,9 +30,10 @@ use yew::prelude::*; pub use self::column_locator::{ ColumnLocator, ColumnSettingsTab, ColumnSettingsTarget, ColumnTab, OpenColumnSettings, }; -use self::drag_helpers::DragTargetState; pub use self::drag_helpers::{DragDropContainer, DragEndCallback}; -use self::drag_helpers::{PointerDownCallback, clear_document_selection, closest_draggable}; +use self::drag_helpers::{ + DragTargetState, PointerDownCallback, clear_document_selection, closest_draggable, +}; pub use self::props::{DragDropProps, PresentationProps}; use crate::config::{CssKind, NamedValue, assign_palette_names}; use crate::utils::*;