From b2b9d0c96e0738258bc14607e838d65236eb470a Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 22 Apr 2024 07:53:58 -0600 Subject: [PATCH 1/5] Fix null partition filter --- .../iris-grid/src/IrisGridPartitionSelector.tsx | 6 +++++- packages/iris-grid/src/IrisGridTableModel.ts | 15 +++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/iris-grid/src/IrisGridPartitionSelector.tsx b/packages/iris-grid/src/IrisGridPartitionSelector.tsx index f8acc810d7..0c55447503 100644 --- a/packages/iris-grid/src/IrisGridPartitionSelector.tsx +++ b/packages/iris-grid/src/IrisGridPartitionSelector.tsx @@ -214,7 +214,11 @@ class IrisGridPartitionSelector extends Component< getDisplayValue(index: number, value: unknown): string { const { model } = this.props; - if (value == null || value === '') { + if (value === null) { + return 'null'; + } + + if (value === undefined || value === '') { return ''; } const column = model.partitionColumns[index]; diff --git a/packages/iris-grid/src/IrisGridTableModel.ts b/packages/iris-grid/src/IrisGridTableModel.ts index 277a7d868d..3216281040 100644 --- a/packages/iris-grid/src/IrisGridTableModel.ts +++ b/packages/iris-grid/src/IrisGridTableModel.ts @@ -206,12 +206,15 @@ class IrisGridTableModel for (let i = 0; i < this.partitionColumns.length; i += 1) { const partition = partitions[i]; const partitionColumn = this.partitionColumns[i]; - - const partitionFilter = this.tableUtils.makeFilterRawValue( - partitionColumn.type, - partition - ); - partitionFilters.push(partitionColumn.filter().eq(partitionFilter)); + if (partition == null) { + partitionFilters.push(partitionColumn.filter().isNull()); + } else { + const partitionFilter = this.tableUtils.makeFilterRawValue( + partitionColumn.type, + partition + ); + partitionFilters.push(partitionColumn.filter().eq(partitionFilter)); + } } const t = await this.table.copy(); From abb283f88ad6faf21e943282cab355e91c74eb9b Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Mon, 22 Apr 2024 12:41:16 -0600 Subject: [PATCH 2/5] Change null to (null) --- packages/iris-grid/src/IrisGridPartitionSelector.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/iris-grid/src/IrisGridPartitionSelector.tsx b/packages/iris-grid/src/IrisGridPartitionSelector.tsx index 0c55447503..a9d3c781ef 100644 --- a/packages/iris-grid/src/IrisGridPartitionSelector.tsx +++ b/packages/iris-grid/src/IrisGridPartitionSelector.tsx @@ -215,7 +215,7 @@ class IrisGridPartitionSelector extends Component< const { model } = this.props; if (value === null) { - return 'null'; + return '(null)'; } if (value === undefined || value === '') { From d4d0b6554eedee4b2a186d53e9b713681102b1ac Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Tue, 23 Apr 2024 17:18:59 -0600 Subject: [PATCH 3/5] Clean up nullable eq filter, fix undefined/null difference in JSAPI return values --- packages/iris-grid/src/IrisGrid.tsx | 5 +++- .../src/IrisGridPartitionSelector.tsx | 28 ++++++++----------- packages/iris-grid/src/IrisGridTableModel.ts | 14 ++++------ .../jsapi-components/src/TableDropdown.tsx | 4 ++- packages/jsapi-utils/src/TableUtils.ts | 16 +++++++++++ 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index 3bdf4edce9..1c3aa33014 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/packages/iris-grid/src/IrisGrid.tsx @@ -2062,7 +2062,10 @@ class IrisGrid extends Component { return; } const row = data.rows[0]; - const values = keyTable.columns.map(column => row.get(column)); + // Core JSAPI returns undefined for null table values, IrisGridPartitionSelector expects null + const values = keyTable.columns.map( + column => row.get(column) ?? null + ); const newPartition: PartitionConfig = { partitions: values, mode: 'partition', diff --git a/packages/iris-grid/src/IrisGridPartitionSelector.tsx b/packages/iris-grid/src/IrisGridPartitionSelector.tsx index a9d3c781ef..63e85f3e51 100644 --- a/packages/iris-grid/src/IrisGridPartitionSelector.tsx +++ b/packages/iris-grid/src/IrisGridPartitionSelector.tsx @@ -172,20 +172,18 @@ class IrisGridPartitionSelector extends Component< .slice(0, index + 1) .map((partition, i) => { const partitionColumn = t.columns[i]; - return partitionColumn - .filter() - .eq( - this.tableUtils.makeFilterRawValue( - partitionColumn.type, - partition - ) - ); + return this.tableUtils.makeNullableEqFilter( + partitionColumn, + partition + ); }); t.applyFilter(partitionFilters); t.setViewport(0, 0, t.columns); const data = await this.pending.add(t.getViewportData()); const newConfig: PartitionConfig = { - partitions: t.columns.map(column => data.rows[0].get(column)), + // Core JSAPI returns undefined for null table values, + // coalesce with null to differentiate null from no selection in the dropdown + partitions: t.columns.map(column => data.rows[0].get(column) ?? null), mode: 'partition', }; t.close(); @@ -271,14 +269,10 @@ class IrisGridPartitionSelector extends Component< const previousColumn = model.partitionColumns[i - 1]; const partitionFilter = [ ...previousFilter, - previousColumn - .filter() - .eq( - this.tableUtils.makeFilterRawValue( - previousColumn.type, - previousPartition - ) - ), + this.tableUtils.makeNullableEqFilter( + previousColumn, + previousPartition + ), ]; partitionFilters.push(partitionFilter); } diff --git a/packages/iris-grid/src/IrisGridTableModel.ts b/packages/iris-grid/src/IrisGridTableModel.ts index 3216281040..54593e2868 100644 --- a/packages/iris-grid/src/IrisGridTableModel.ts +++ b/packages/iris-grid/src/IrisGridTableModel.ts @@ -206,15 +206,11 @@ class IrisGridTableModel for (let i = 0; i < this.partitionColumns.length; i += 1) { const partition = partitions[i]; const partitionColumn = this.partitionColumns[i]; - if (partition == null) { - partitionFilters.push(partitionColumn.filter().isNull()); - } else { - const partitionFilter = this.tableUtils.makeFilterRawValue( - partitionColumn.type, - partition - ); - partitionFilters.push(partitionColumn.filter().eq(partitionFilter)); - } + const partitionFilter = this.tableUtils.makeNullableEqFilter( + partitionColumn, + partition + ); + partitionFilters.push(partitionFilter); } const t = await this.table.copy(); diff --git a/packages/jsapi-components/src/TableDropdown.tsx b/packages/jsapi-components/src/TableDropdown.tsx index d9762e3766..349f44f2d8 100644 --- a/packages/jsapi-components/src/TableDropdown.tsx +++ b/packages/jsapi-components/src/TableDropdown.tsx @@ -82,7 +82,9 @@ export function TableDropdown({ dh.Table.EVENT_UPDATED, (event: CustomEvent) => { const { detail } = event; - const newValues = detail.rows.map(row => row.get(tableColumn)); + // Core JSAPI returns undefined for null table values, + // coalesce with null to differentiate null from no selection in the dropdown + const newValues = detail.rows.map(row => row.get(tableColumn) ?? null); setValues(newValues); } ); diff --git a/packages/jsapi-utils/src/TableUtils.ts b/packages/jsapi-utils/src/TableUtils.ts index 06d28f6b52..7d7fde3d2a 100644 --- a/packages/jsapi-utils/src/TableUtils.ts +++ b/packages/jsapi-utils/src/TableUtils.ts @@ -1859,6 +1859,22 @@ export class TableUtils { return dh.FilterValue.ofNumber(rawValue as number); } + /** + * Creates an Eq filter for the given column and raw value + * @param column The column to set the filter on + * @param rawValue The nullable value to filter on + * @returns The filter for this column/value combination + */ + makeNullableEqFilter( + column: DhType.Column, + rawValue: unknown + ): DhType.FilterCondition { + if (rawValue == null) { + return column.filter().isNull(); + } + return column.filter().eq(this.makeFilterRawValue(column.type, rawValue)); + } + /** * Converts a string value to a value appropriate for the column * @param columnType The column type to make the value for From c91204a678fcb937fe6b5d193310155db6dc81f3 Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Wed, 24 Apr 2024 11:16:49 -0600 Subject: [PATCH 4/5] Commit review suggestion Co-authored-by: Mike Bender --- packages/iris-grid/src/IrisGrid.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index 1c3aa33014..36ad228a0f 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/packages/iris-grid/src/IrisGrid.tsx @@ -2062,7 +2062,7 @@ class IrisGrid extends Component { return; } const row = data.rows[0]; - // Core JSAPI returns undefined for null table values, IrisGridPartitionSelector expects null + // Core JSAPI returns undefined for null table values, IrisGridPartitionSelector expects null (https://github.com/deephaven/deephaven-core/issues/5400) const values = keyTable.columns.map( column => row.get(column) ?? null ); From 36171e52c1a7d5c0be4275dfa71b877c4db9061f Mon Sep 17 00:00:00 2001 From: Vlad Babich Date: Wed, 24 Apr 2024 11:18:31 -0600 Subject: [PATCH 5/5] Add core issue link --- packages/iris-grid/src/IrisGrid.tsx | 3 ++- packages/iris-grid/src/IrisGridPartitionSelector.tsx | 1 + packages/jsapi-components/src/TableDropdown.tsx | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/iris-grid/src/IrisGrid.tsx b/packages/iris-grid/src/IrisGrid.tsx index 36ad228a0f..6299f53c03 100644 --- a/packages/iris-grid/src/IrisGrid.tsx +++ b/packages/iris-grid/src/IrisGrid.tsx @@ -2062,7 +2062,8 @@ class IrisGrid extends Component { return; } const row = data.rows[0]; - // Core JSAPI returns undefined for null table values, IrisGridPartitionSelector expects null (https://github.com/deephaven/deephaven-core/issues/5400) + // Core JSAPI returns undefined for null table values, IrisGridPartitionSelector expects null + // https://github.com/deephaven/deephaven-core/issues/5400 const values = keyTable.columns.map( column => row.get(column) ?? null ); diff --git a/packages/iris-grid/src/IrisGridPartitionSelector.tsx b/packages/iris-grid/src/IrisGridPartitionSelector.tsx index 63e85f3e51..cd7df309f3 100644 --- a/packages/iris-grid/src/IrisGridPartitionSelector.tsx +++ b/packages/iris-grid/src/IrisGridPartitionSelector.tsx @@ -183,6 +183,7 @@ class IrisGridPartitionSelector extends Component< const newConfig: PartitionConfig = { // Core JSAPI returns undefined for null table values, // coalesce with null to differentiate null from no selection in the dropdown + // https://github.com/deephaven/deephaven-core/issues/5400 partitions: t.columns.map(column => data.rows[0].get(column) ?? null), mode: 'partition', }; diff --git a/packages/jsapi-components/src/TableDropdown.tsx b/packages/jsapi-components/src/TableDropdown.tsx index 349f44f2d8..2e8e342abc 100644 --- a/packages/jsapi-components/src/TableDropdown.tsx +++ b/packages/jsapi-components/src/TableDropdown.tsx @@ -84,6 +84,7 @@ export function TableDropdown({ const { detail } = event; // Core JSAPI returns undefined for null table values, // coalesce with null to differentiate null from no selection in the dropdown + // https://github.com/deephaven/deephaven-core/issues/5400 const newValues = detail.rows.map(row => row.get(tableColumn) ?? null); setValues(newValues); }