Skip to content

Commit 6878de1

Browse files
authored
fix: DH-20656: Don't provide getRowId func unless necessary (#1269)
- When updating to AG Grid v34, seems there was a change in behaviour in how the generated row ID was being used - Previously for regular tables, we were just returning an empty string. It doesn't seem to like that anymore - Now it does not provide the getRowId function unless it's a tree or pivot table. Regular tables just map the row index to the row ID automatically - Tested with tables of various sizes (scrolling gets screwy with over a million rows, that's AG Grid's issue) and rollup tables: ``` from deephaven import empty_table from deephaven import time_table from deephaven.ag_grid import AgGrid def make_simple_table(row_count): """ Make a basic table with the specified number of rows """ return empty_table(row_count).update_view("x=i") thousand_rows = make_simple_table(1_000) hundred_thousand_rows = make_simple_table(100_000) million_rows = make_simple_table(1_000_000) ten_million_rows = make_simple_table(10_000_000) hundred_million_rows = make_simple_table(100_000_000) billion_rows = make_simple_table(1_000_000_000) ag_thousand_rows = AgGrid(thousand_rows) ag_hundred_thousand_rows = AgGrid(hundred_thousand_rows) ag_million_rows = AgGrid(million_rows) ag_ten_million_rows = AgGrid(ten_million_rows) ag_hundred_million_rows = AgGrid(hundred_million_rows) ag_billion_rows = AgGrid(billion_rows) from deephaven import agg from deephaven.ag_grid import AgGrid source = empty_table(10000).update( [ "Group = i % 100", "Subgroup = i % 3", "Value = i * 10", ] ) agg_list = [agg.avg(cols="AvgValue=Value"), agg.std(cols="StdValue=Value")] by_list = ["Group", "Subgroup"] rollup = source.rollup(aggs=agg_list, by=by_list) ag_rollup = AgGrid(rollup) ```
1 parent ddd341e commit 6878de1

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

plugins/ag-grid/src/js/src/components/AgGridView.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
getColumnDefs,
1818
getSideBar,
1919
isPivotTable,
20+
isTable,
2021
toGroupKeyString,
2122
TREE_NODE_KEY,
2223
TreeNode,
@@ -137,8 +138,8 @@ export function AgGridView({
137138
(params: GetRowIdParams): string => {
138139
const { data } = params;
139140
if (data == null) {
140-
log.warn('getRowId called with null data', params);
141-
return '';
141+
log.error('getRowId called with null data', params);
142+
throw new Error('getRowId called with null data');
142143
}
143144

144145
if (isPivotTable(table)) {
@@ -153,7 +154,11 @@ export function AgGridView({
153154
}
154155

155156
const treeNode: TreeNode | undefined = data?.[TREE_NODE_KEY];
156-
return `${treeNode?.index ?? ''}`;
157+
if (treeNode == null) {
158+
log.error('getRowId called with missing tree node info', params);
159+
throw new Error('Tree node info missing from row data');
160+
}
161+
return `${treeNode.index}`;
157162
},
158163
[table]
159164
);
@@ -170,7 +175,8 @@ export function AgGridView({
170175
dataTypeDefinitions={formatter.cellDataTypeDefinitions}
171176
viewportDatasource={datasource}
172177
rowModelType="viewport"
173-
getRowId={getRowId}
178+
// With a regular table, the row IDs are just the row indices, so we don't need to specify getRowId
179+
getRowId={isTable(table) ? undefined : getRowId}
174180
sideBar={sideBar}
175181
/>
176182
);

0 commit comments

Comments
 (0)