Skip to content

Commit

Permalink
fix: Fix null partition filter (#1954)
Browse files Browse the repository at this point in the history
Closes #1867

Tested in Enterprise using steps from DH-16595 - created a partitioned
table and set ACL to `nullFilterGenerator()` to get the `null` value in
the partition column. Creating a partitioned table with a `null` in the
partition column doesn't seem possible with `db.addTablePartition` in
Enterprise.

```
t=db.t("LearnDeephaven", "StockTrades").where()
db.addPartitionedTableDefinition("<namespace>", "partTable", "Date", t.getDefinition())
test = db.t("<namespace>" , "partTable")

// Set Column ACL on `partTable` to `new NullFilterGenerator()` in swing

test = db.t("<namespace>" , "partTable")
```
![Screenshot 2024-04-23 at 5 26
43 PM](https://github.com/deephaven/web-client-ui/assets/2853653/bbd65305-4616-416d-8af3-96dadcc21fa1)


Core+ Groovy console:
```
t = emptyTable(1).update("X=(String)null","Y=ii").partitionBy("X")
```
![Screenshot 2024-04-23 at 5 23
55 PM](https://github.com/deephaven/web-client-ui/assets/2853653/0564a626-97fa-46d9-8920-200eda19c533)

---------

Co-authored-by: Mike Bender <mikebender@deephaven.io>
  • Loading branch information
vbabich and mofojed committed Apr 24, 2024
1 parent 0dbf17d commit 3a1f92b
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 24 deletions.
6 changes: 5 additions & 1 deletion packages/iris-grid/src/IrisGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,11 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
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
// https://github.com/deephaven/deephaven-core/issues/5400
const values = keyTable.columns.map(
column => row.get(column) ?? null
);
const newPartition: PartitionConfig = {
partitions: values,
mode: 'partition',
Expand Down
35 changes: 17 additions & 18 deletions packages/iris-grid/src/IrisGridPartitionSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,20 +172,19 @@ 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
// https://github.com/deephaven/deephaven-core/issues/5400
partitions: t.columns.map(column => data.rows[0].get(column) ?? null),
mode: 'partition',
};
t.close();
Expand Down Expand Up @@ -214,7 +213,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];
Expand Down Expand Up @@ -267,14 +270,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);
}
Expand Down
7 changes: 3 additions & 4 deletions packages/iris-grid/src/IrisGridTableModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,12 +206,11 @@ 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,
const partitionFilter = this.tableUtils.makeNullableEqFilter(
partitionColumn,
partition
);
partitionFilters.push(partitionColumn.filter().eq(partitionFilter));
partitionFilters.push(partitionFilter);
}

const t = await this.table.copy();
Expand Down
5 changes: 4 additions & 1 deletion packages/jsapi-components/src/TableDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ export function TableDropdown({
dh.Table.EVENT_UPDATED,
(event: CustomEvent<DhType.ViewportData>) => {
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
// https://github.com/deephaven/deephaven-core/issues/5400
const newValues = detail.rows.map(row => row.get(tableColumn) ?? null);
setValues(newValues);
}
);
Expand Down
16 changes: 16 additions & 0 deletions packages/jsapi-utils/src/TableUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3a1f92b

Please sign in to comment.