Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Partitioned Table UI Enhancements #2110

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4caeb92
feat: Partitioned Table UI Enhancements
AkshatJawne Jun 26, 2024
a2f8b9b
modify data extraction and test case
AkshatJawne Jun 26, 2024
831bae4
change name to baseTable and remove unnecessary border
AkshatJawne Jun 26, 2024
45bc6d8
make changes according to review
AkshatJawne Jun 27, 2024
81bdecc
fix formatting issues
AkshatJawne Jun 27, 2024
42dc49d
fix more formatting issues
AkshatJawne Jun 27, 2024
8cff366
small visual changes
AkshatJawne Jun 28, 2024
8ed8875
resolve merge conflicts
AkshatJawne Jul 2, 2024
a97cf27
fix formatting issues
AkshatJawne Jul 2, 2024
01a3225
make changes based on review
AkshatJawne Jul 4, 2024
16db737
temp workaround
AkshatJawne Jul 4, 2024
bc17e40
refactor logic to update partition options
AkshatJawne Jul 4, 2024
9bf8999
WIP fixing rendering issues
AkshatJawne Jul 5, 2024
99096c2
WIP state not working as intended
AkshatJawne Jul 5, 2024
804592b
fix empty table bug
AkshatJawne Jul 8, 2024
292e1ad
WIP still investigating cause of chromium failures
AkshatJawne Jul 9, 2024
dbf69a2
WIP fix formatting issue
AkshatJawne Jul 9, 2024
420bf9e
WIP small changes
AkshatJawne Jul 12, 2024
0762fbb
WIP fix formatter file not getting pushed
AkshatJawne Jul 12, 2024
82f72cb
WIP wanting to test change
AkshatJawne Jul 12, 2024
41bc31a
WIP test context menu change
AkshatJawne Jul 12, 2024
30226d6
WIP test chamhe
AkshatJawne Jul 12, 2024
1601d84
WIP revert old changes
AkshatJawne Jul 12, 2024
a092033
WIP Bender solution
AkshatJawne Jul 16, 2024
7c82824
fix issues with empty table
AkshatJawne Jul 17, 2024
d9fc4ca
remove null check
AkshatJawne Jul 17, 2024
e588409
remove early return
AkshatJawne Jul 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion packages/iris-grid/src/IrisGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ import {
IrisGridContextMenuHandler,
IrisGridCopyCellMouseHandler,
IrisGridDataSelectMouseHandler,
IrisGridPartitionedTableMouseHandler,
IrisGridFilterMouseHandler,
IrisGridRowTreeMouseHandler,
IrisGridSortMouseHandler,
Expand Down Expand Up @@ -734,6 +735,7 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
new IrisGridContextMenuHandler(this, dh),
new IrisGridDataSelectMouseHandler(this),
new PendingMouseHandler(this),
new IrisGridPartitionedTableMouseHandler(this),
];
if (canCopy) {
keyHandlers.push(new CopyKeyHandler(this));
Expand Down Expand Up @@ -2080,7 +2082,7 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
);
const newPartition: PartitionConfig = {
partitions: values,
mode: 'partition',
mode: model.isPartitionAwareSourceTable ? 'partition' : 'keys',
};
keyTable.close();
resolve(newPartition);
Expand All @@ -2093,6 +2095,34 @@ class IrisGrid extends Component<IrisGridProps, IrisGridState> {
});
}

/**
* Selects a partition key from the table based on the provided row index.
*
* @param rowIndex The index of the row from which the partition will be selected.
* @returns A promise that resolves when the partition key has been successfully selected.
*/
selectPartitionKeyFromTable(rowIndex: GridRangeIndex): void {
const { model } = this.props;
assertNotNull(rowIndex);
if (isPartitionedGridModel(model)) {
const data = this.getRowDataMap(rowIndex);
const partitionColumnSet = new Set(
model.partitionColumns.map(column => column.name)
);
const values = Object.entries(data)
.filter(([key, _]) => partitionColumnSet.has(key))
.map(([key, columnData]) => columnData.value);
const newPartition: PartitionConfig = {
partitions: values,
mode: 'partition',
};
this.setState({
isSelectingPartition: true,
partitionConfig: newPartition,
});
}
}

copyCell(
columnIndex: GridRangeIndex,
rowIndex: GridRangeIndex,
Expand Down
3 changes: 1 addition & 2 deletions packages/iris-grid/src/IrisGridPartitionSelector.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@
}
.partition-button-group {
display: flex;
border: 1px var(--dh-color-hr);
border-style: none solid;
border-right: 1px solid var(--dh-color-hr);
padding: 0 $spacer-2;
gap: $spacer-2;
}
Expand Down
3 changes: 3 additions & 0 deletions packages/iris-grid/src/IrisGridPartitionSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ function makeModel(
Promise.resolve(irisGridTestUtils.makeTable())
),
partitionColumns: columns,
partitionBaseTable: jest.fn(() =>
Promise.resolve(irisGridTestUtils.makeTable())
),
} as unknown as PartitionedGridModel;
return model;
}
Expand Down
59 changes: 33 additions & 26 deletions packages/iris-grid/src/IrisGridPartitionSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ interface IrisGridPartitionSelectorProps {
interface IrisGridPartitionSelectorState {
isLoading: boolean;

baseTable: dh.Table | null;

keysTable: dh.Table | null;

partitionTables: dh.Table[] | null;
Expand All @@ -35,7 +37,7 @@ class IrisGridPartitionSelector extends Component<
constructor(props: IrisGridPartitionSelectorProps) {
super(props);

this.handleKeyTableClick = this.handleKeyTableClick.bind(this);
this.handlePartitionTableClick = this.handlePartitionTableClick.bind(this);
this.handleMergeClick = this.handleMergeClick.bind(this);
this.handlePartitionSelect = this.handlePartitionSelect.bind(this);

Expand All @@ -47,6 +49,7 @@ class IrisGridPartitionSelector extends Component<
// We start be loading the partition tables, so we should be in a loading state
isLoading: true,

baseTable: null,
keysTable: null,
partitionFilters: null,
partitionTables: null,
Expand All @@ -68,6 +71,10 @@ class IrisGridPartitionSelector extends Component<
t => t.close()
);

const baseTable = await this.pending.add(model.partitionBaseTable(), t =>
t.close()
);

const partitionTables = await Promise.all(
model.partitionColumns.map(async (_, i) =>
this.pending.add(
Expand All @@ -81,6 +88,7 @@ class IrisGridPartitionSelector extends Component<
this.setState({
isLoading: false,
keysTable,
baseTable,
partitionFilters,
partitionTables,
});
Expand All @@ -105,7 +113,8 @@ class IrisGridPartitionSelector extends Component<
componentWillUnmount(): void {
this.pending.cancel();

const { keysTable, partitionTables } = this.state;
const { keysTable, baseTable, partitionTables } = this.state;
baseTable?.close();
keysTable?.close();
partitionTables?.forEach(table => table.close());
}
Expand All @@ -114,10 +123,11 @@ class IrisGridPartitionSelector extends Component<

tableUtils: TableUtils;

handleKeyTableClick(): void {
log.debug2('handleKeyTableClick');
handlePartitionTableClick(): void {
log.debug2('handlePartitionTableClick');

const { partitionConfig } = this.props;

const newPartitionConfig = { ...partitionConfig };
// Toggle between Keys and Partition mode
newPartitionConfig.mode =
Expand Down Expand Up @@ -145,25 +155,22 @@ class IrisGridPartitionSelector extends Component<
index: number,
selectedValue: unknown
): Promise<void> {
const { model, partitionConfig: prevConfig } = this.props;
const { partitionConfig: prevConfig } = this.props;

log.debug('handlePartitionSelect', index, selectedValue, prevConfig);

const newPartitions = [...prevConfig.partitions];
newPartitions[index] = selectedValue;

// If it's the last partition changed, we know it's already a valid value, just emit it
if (index === model.partitionColumns.length - 1) {
this.sendUpdate({ partitions: newPartitions, mode: 'partition' });
return;
}

const { baseTable } = this.state;
const { keysTable } = this.state;
// Otherwise, we need to get the value from a filtered key table

assertNotNull(baseTable);
assertNotNull(keysTable);
try {
this.setState({ isLoading: true });
const t = await this.pending.add(keysTable.copy(), tCopy =>

const t = await this.pending.add(baseTable.copy(), tCopy =>
tCopy.close()
);

Expand All @@ -178,13 +185,13 @@ class IrisGridPartitionSelector extends Component<
);
});
t.applyFilter(partitionFilters);
t.setViewport(0, 0, t.columns);
t.setViewport(0, 10, t.columns);
const data = await this.pending.add(t.getViewportData());
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),
partitions: keysTable.columns.map(column => data.rows[0].get(column)),
mode: 'partition',
};
t.close();
Expand Down Expand Up @@ -294,11 +301,10 @@ class IrisGridPartitionSelector extends Component<
const { model, partitionConfig } = this.props;
const { isLoading, partitionFilters, partitionTables } = this.state;

const { partitions } = partitionConfig;

if (partitionFilters !== null && partitionTables !== null) {
partitionFilters.forEach((filter, index) => {
partitionTables[index].applyFilter(filter as dh.FilterCondition[]);
partitionTables[index].setViewport(0, 50);
});
}
const partitionSelectors = model.partitionColumns.map((column, index) => (
Expand All @@ -311,13 +317,15 @@ class IrisGridPartitionSelector extends Component<
direction="bottom"
shouldFlip={false}
keyColumn={partitionTables[index].columns[index].name}
placeholder={'Loading...' as string}
selectedKey={
partitionConfig.mode === 'keys'
? null
: (partitionConfig.partitions[index] as ItemKey)
}
placeholder={'Select an option' as string}
labelColumn={partitionTables[index].columns[index].name}
onChange={this.getCachedChangeCallback(index)}
defaultSelectedKey={partitions[index] as ItemKey}
isDisabled={
(index > 0 && partitionConfig.mode !== 'partition') || isLoading
}
isDisabled={isLoading}
/>
)}
{model.partitionColumns.length - 1 === index || (
Expand All @@ -327,17 +335,16 @@ class IrisGridPartitionSelector extends Component<
));
return (
<div className="iris-grid-partition-selector">
<div className="table-name">Partitioned Table</div>
<div className="partition-button-group">
<Button
onClick={this.handleKeyTableClick}
onClick={this.handlePartitionTableClick}
kind="inline"
tooltip="View keys as table"
icon={vsKey}
active={partitionConfig.mode === 'keys'}
disabled={isLoading}
>
Keys
Partitions
</Button>
<Button
onClick={this.handleMergeClick}
Expand All @@ -347,7 +354,7 @@ class IrisGridPartitionSelector extends Component<
active={partitionConfig.mode === 'merged'}
disabled={isLoading}
>
Merge
{model.isPartitionAwareSourceTable ? 'Merge' : 'Coalesce'}
</Button>
</div>
{partitionSelectors}
Expand Down
9 changes: 9 additions & 0 deletions packages/iris-grid/src/IrisGridPartitionedTableModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class IrisGridPartitionedTableModel
return true;
}

get isPartitionAwareSourceTable(): boolean {
return false;
}

get isReversible(): boolean {
return false;
}
Expand Down Expand Up @@ -60,6 +64,11 @@ class IrisGridPartitionedTableModel
return this.partitionedTable.getKeyTable();
}

async partitionBaseTable(): Promise<DhType.Table> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return (this.partitionedTable as any).getBaseTable();
}

async partitionMergedTable(): Promise<DhType.Table> {
return this.partitionedTable.getMergedTable();
}
Expand Down
15 changes: 14 additions & 1 deletion packages/iris-grid/src/IrisGridProxyModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ class IrisGridProxyModel extends IrisGridModel implements PartitionedGridModel {
) {
if (partitionConfig.mode === 'keys') {
modelPromise = this.originalModel
.partitionKeysTable()
.partitionBaseTable()
.then(table => makeModel(this.dh, table, this.formatter));
} else if (partitionConfig.mode === 'merged') {
modelPromise = this.originalModel
Expand All @@ -553,6 +553,13 @@ class IrisGridProxyModel extends IrisGridModel implements PartitionedGridModel {
return this.originalModel.partitionKeysTable();
}

partitionBaseTable(): Promise<DhType.Table> {
if (!isPartitionedGridModelProvider(this.originalModel)) {
throw new Error('Partitions are not available');
}
return this.originalModel.partitionBaseTable();
}

partitionMergedTable(): Promise<DhType.Table> {
if (!isPartitionedGridModelProvider(this.originalModel)) {
throw new Error('Partitions are not available');
Expand Down Expand Up @@ -698,6 +705,12 @@ class IrisGridProxyModel extends IrisGridModel implements PartitionedGridModel {
: false;
}

get isPartitionAwareSourceTable(): boolean {
return isPartitionedGridModelProvider(this.originalModel)
? this.originalModel.isPartitionAwareSourceTable
: false;
}

get isEditable(): boolean {
return isEditableGridModel(this.model) && this.model.isEditable;
}
Expand Down
8 changes: 8 additions & 0 deletions packages/iris-grid/src/IrisGridTableModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,10 @@ class IrisGridTableModel
return t;
}

async partitionBaseTable(): Promise<DhType.Table> {
return this.valuesTable(this.columns);
AkshatJawne marked this conversation as resolved.
Show resolved Hide resolved
}

async partitionTable(partitions: unknown[]): Promise<DhType.Table> {
log.debug('getting partition table for partitions', partitions);

Expand Down Expand Up @@ -296,6 +300,10 @@ class IrisGridTableModel
);
}

get isPartitionAwareSourceTable(): boolean {
return this.isPartitionRequired && true;
AkshatJawne marked this conversation as resolved.
Show resolved Hide resolved
}

isFilterable(columnIndex: ModelIndex): boolean {
return this.getCachedFilterableColumnSet(this.columns).has(columnIndex);
}
Expand Down
5 changes: 5 additions & 0 deletions packages/iris-grid/src/PartitionedGridModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,14 @@ export interface PartitionedGridModelProvider extends IrisGridModel {
/** Whether a partition is required to be set on this model */
get isPartitionRequired(): boolean;

/** Whether the table is a partition-aware source table or a partition table */
get isPartitionAwareSourceTable(): boolean;

/** Get a keys table for the partitions */
AkshatJawne marked this conversation as resolved.
Show resolved Hide resolved
partitionKeysTable: () => Promise<dh.Table>;

partitionBaseTable: () => Promise<dh.Table>;
AkshatJawne marked this conversation as resolved.
Show resolved Hide resolved

/** Get a merged table containing all partitions */
partitionMergedTable: () => Promise<dh.Table>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import './IrisGridContextMenuHandler.scss';
import SHORTCUTS from '../IrisGridShortcuts';
import IrisGrid from '../IrisGrid';
import { QuickFilter } from '../CommonTypes';
import { isPartitionedGridModel } from '../PartitionedGridModel';

const log = Log.module('IrisGridContextMenuHandler');

Expand Down Expand Up @@ -504,6 +505,17 @@ class IrisGridContextMenuHandler extends GridMouseHandler {
},
});

if (isPartitionedGridModel(model) && !model.isPartitionAwareSourceTable) {
actions.push({
title: 'View Constituent Table',
group: IrisGridContextMenuHandler.GROUP_VIEW_CONTENTS,
order: 40,
action: () => {
irisGrid.selectPartitionKeyFromTable(rowIndex);
},
});
}

return actions;
}

Expand Down
Loading
Loading