feat: Create UI to Display Partitioned Tables #1663

georgecwan · 2023-11-29T23:22:44Z

PartitionedTable objects can now be opened and displayed with a new UI
- Supports switching between partitions and viewing the merged or key table
Partition aware parquet tables will also share this new UI
- The new UI will no longer allow users to enter invalid partitions
Closes UI for PartitionedTable #1143
Depends on the following changes to core:

Testing Instructions

PartitionedTable

Run the code and open the table pt

from deephaven import empty_table

_t = empty_table(100).update(["verylongcolumn=(int)Math.floor(i/5)", "veryveryverylongcolumn=i"])
pt = _t.partition_by(["verylongcolumn", "veryveryverylongcolumn"])

Check that the features specified in the spec are present:
- Resizing the panel horizontally wraps the dropdown without wrapping the '>'
- Hovering over the 'Key' and 'Merge' buttons displays the correct labels
- The initial partition should be the first valid partition available when all columns are sorted in descending order
- Options in the dropdown are displayed in descending order
Clicking the 'Key' and 'Merge' tables should correctly display the respective table and the button should visually indicate if one of them is being displayed. All the dropdowns should show empty values while one of the buttons is active.
- While one of the toggle buttons is active, only the leftmost dropdown should be enabled
After clearing the dropdowns by clicking either the 'Key' or 'Merge' button, selecting any value on any dropdown should automatically set the remaining dropdowns and display a valid partition.
Dropdowns should only contain values that are valid with respect to the selected values of all the dropdowns left of it
Changing the value of a dropdown should try to preserve dropdowns to the right of it. If this is not possible, the values of the dropdowns right of it should be changed so that a valid partition can be displayed.

Parquet Tables

Run the following code and verify that all tables display and function correctly for every data type

from deephaven import empty_table
part = empty_table(4).update("II=ii")

from deephaven.parquet import write, read

write(part, "/tmp/pt-test/intCol=0/part.parquet")
write(part, "/tmp/pt-test/intCol=1/part.parquet")
int_partition = read("/tmp/pt-test")

write(part, "/tmp/string-test/stringCol=hello/part.parquet")
write(part, "/tmp/string-test/stringCol=world/part.parquet")
string_partition = read("/tmp/string-test")

write(part, "/tmp/double-test/doubleCol=1.5/part.parquet")
write(part, "/tmp/double-test/doubleCol=2.5/part.parquet")
double_partition = read("/tmp/double-test")

write(part, "/tmp/char-test/charCol=a/part.parquet")
write(part, "/tmp/char-test/charCol=b/part.parquet")
char_partition = read("/tmp/char-test")

write(part, "/tmp/long-test/longCol=2147483648/part.parquet")
write(part, "/tmp/long-test/longCol=2147483650/part.parquet")
long_partition = read("/tmp/long-test")

write(part, "/tmp/bool-test/boolCol=true/part.parquet")
write(part, "/tmp/bool-test/boolCol=false/part.parquet")
bool_partition = read("/tmp/bool-test")

write(part, "/tmp/multi_test/x=0/y=0/part.parquet")
write(part, "/tmp/multi_test/x=0/y=1/part.parquet")
write(part, "/tmp/multi_test/x=1/y=0/part.parquet")
write(part, "/tmp/multi_test/x=1/y=1/part.parquet")
write(part, "/tmp/multi_test/x=1/y=2/part.parquet")
multi_partition = read("/tmp/multi_test")

…o 1143-partitioned-table-ui

…tioned-table-ui

…o 1143-partitioned-table-ui

codecov · 2023-11-30T16:32:08Z

Codecov Report

Attention: 407 lines in your changes are missing coverage. Please review.

Comparison is base (c6a099d) 46.48% compared to head (681bc24) 46.05%.

Files	Patch %	Lines
packages/iris-grid/src/EmptyIrisGridModel.ts	0.00%	91 Missing ⚠️
...ckages/iris-grid/src/IrisGridPartitionSelector.tsx	30.95%	87 Missing ⚠️
packages/iris-grid/src/IrisGrid.tsx	22.03%	46 Missing ⚠️
packages/jsapi-components/src/TableDropdown.tsx	0.00%	41 Missing ⚠️
packages/iris-grid/src/IrisGridProxyModel.ts	31.57%	39 Missing ⚠️
...ges/iris-grid/src/IrisGridPartitionedTableModel.ts	0.00%	30 Missing ⚠️
packages/iris-grid/src/IrisGridTableModel.ts	61.11%	21 Missing ⚠️
packages/iris-grid/src/IrisGridUtils.ts	25.00%	21 Missing ⚠️
packages/jsapi-utils/src/TableUtils.ts	0.00%	9 Missing ⚠️
...ode-studio/src/styleguide/MockIrisGridTreeModel.ts	0.00%	6 Missing ⚠️
... and 9 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1663      +/-   ##
==========================================
- Coverage   46.48%   46.05%   -0.44%     
==========================================
  Files         617      621       +4     
  Lines       37289    37430     +141     
  Branches     9378     9399      +21     
==========================================
- Hits        17335    17237      -98     
- Misses      19900    20139     +239     
  Partials       54       54

Flag	Coverage Δ
unit	`46.05% <23.78%> (-0.44%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

packages/iris-grid/src/IrisGridPartitionSelector.tsx

packages/iris-grid/src/IrisGridPartitionedTableModel.ts

packages/iris-grid/src/IrisGridProxyModel.ts

packages/iris-grid/src/IrisGridPartitionedTableModel.ts

packages/iris-grid/src/IrisGridProxyModel.ts

packages/iris-grid/src/IrisGridPartitionedTableModel.ts

packages/iris-grid/src/IrisGridPartitionSelector.tsx

dsmmcken

Boolean and Char columns don't seem to work in the UI as parition keys, even though they are supported by the engine:

georgecwan · 2023-12-07T20:38:36Z

Boolean and Char columns don't seem to work in the UI as parition keys, even though they are supported by the engine:

Currently breaks when partition columns are not in order, looking into it

Also seems to not handle empty and null well

…o 1143-partitioned-table-ui

dsmmcken

Something awful is happening with the performance here. I can only replicate it with the parition viewer, but it might be something in core. I don't know.

Steps to reproduce:

run this, time_table, creating new a parition on each tick

from deephaven import empty_table, time_table

simple_ticking = time_table("PT00:00:01").update([
        "MyString=new String(`a`+i)",
        "MyInt=new Integer(i)",
        "MyLong=new Long(i)",
        "MyDouble=new Double(i+i/10)",
        "MyFloat=new Float(i+i/10)",
        "MyBoolean=new Boolean(i%2==0)",
        "MyChar= new Character((char) ((i%26)+97))",
        "MyShort=new Short(Integer.toString(i%32767))",
        "MyByte= new java.lang.Byte(Integer.toString(i%127))"])

p = simple_ticking.partition_by("MyString")

Wait like a minute or two then take a performance snapshot. You'll get a million little nano-second locks too the point of it breaking everything.

georgecwan · 2023-12-07T22:09:32Z

The key columns passed in by the js api has incorrect order, going to fix that issue in core first. This should be the cause of dropdowns not showing any options for certain column orderings independent of type. There might still more issues when dealing with null or empty values and some type-dependent issues unrelated to this.

Update: fix opened at deephaven/deephaven-core#4931

- Use EMPTY_ARRAY instead of returning a new array each time from the EmptyGridModel - Reduces re-renders - Save the current viewport in IrisGridProxyModel, and apply it to the new model if the columns haven't changed - Clean up hydration/dehydration - Needed to dehydrate the value correctly

- Return the partitioned tables columns instead of no columns, so rehydration will succeed - Fix char type partitions - needed to create the raw filter value better

bmingles · 2024-01-05T21:46:11Z

packages/iris-grid/src/IrisGridPartitionSelector.test.tsx

+function makeModel(
+  columns = irisGridTestUtils.makeColumns()
+): PartitionedGridModel {
+  const model = {


Not worth changing, but createMockProxy could make this a bit cleaner

…e-ui

packages/iris-grid/src/IrisGridPartitionedTableModel.ts

packages/iris-grid/src/IrisGridTableModel.ts

bmingles

Left 2 minor comments about missing async

- Add async to methods for consistency

bmingles

LGTM

…le (#5009) - Memoizing it is inconsistent with other methods like `WorkerConnection.getTable`, which always returns a new table - Was unclear who the "owner" of the returned table was - Memoization can happen at the client level if they need it - Tested with deephaven/web-client-ui#1663

dsmmcken

String column not working. Boolean column not working.

And I get this error occasionally:

Tried testing using dx.stocks and mostdatatypes queries.

- Was referencing the columns from the original table instead of the actual table - Was causing a mismatch when fetching the viewport, where it would just stall instead of throwing an error

mofojed · 2024-01-16T20:28:51Z

@dsmmcken should be fixed. Also, you'll need to make sure you're on the latest main that includes both deephaven/deephaven-core#5033 and deephaven/deephaven-core#5009

mofojed · 2024-01-18T15:33:43Z

Don's found two errors that are still occurring, both of them are because it is a ticking table starting with no partition. Using this snippet you can reproduce both issues:

from deephaven import empty_table, time_table

simple_ticking = time_table("PT00:00:01").update([
        "MyString=new String(`a`+i)",
        "MyInt=new Integer(i)",
        "MyLong=new Long(i)",
        "MyDouble=new Double(i+i/10)",
        "MyFloat=new Float(i+i/10)",
        "MyBoolean=new Boolean(i%2==0)",
        "MyChar= new Character((char) ((i%26)+97))",
        "MyShort=new Short(Integer.toString(i%32767))",
        "MyByte= new java.lang.Byte(Integer.toString(i%127))"])

p = simple_ticking.partition_by(["MyString", "MyInt", "MyBoolean"])

Run the snippet above. Wait a couple of ticks, then run the snippet again. Sometimes you get stuck with a "Rebuilding Filters..." message in the bottom corner.

In the logs there is a "Table does not have any data" message. The issue is we're trying to load the first partition key, but there are no partition keys. The fix is to instead wait for an "updated" event to get the first key.

Run the snippet above and let it go a few ticks. Then change partitions, wait a second, then run the snippet above again. You'll get an error message saying layoutHints is null:

IrisGrid.tsx:2885 [IrisGrid] request failed: TypeError: Cannot read properties of null (reading 'layoutHints')
    at get layoutHints [as layoutHints] (IrisGridTableModel.ts:124:23)
    at new IrisGridTableModelTemplate (IrisGridTableModelTemplate.ts:251:12)
    at new IrisGridTableModel (IrisGridTableModel.ts:57:5)
    at makeModel (IrisGridProxyModel.ts:65:10)
    at IrisGridProxyModel.ts:551:26

The JsPartitionedTable.getTable API can return null instead of rejecting. I have a PR to fix that: fix: Mark JsPartitionedTable.getTable as nullable deephaven-core#5050
The main issue is that we re-open the table with the previously selected key, which is no longer valid (since the table started from scratch). We could handle that error more gracefully.

Workaround for both issues is to re-open the table.

- Wait for a key to be added to the keys table if one does not initially exist - If the reloaded partition config is invalid, will reload the initial partition config

dsmmcken

Looks good, all working now. Changed a couple color variables to theme better.

georgecwan and others added 10 commits November 2, 2023 15:38

Create IrisGridPartitionedTableModel

9c94210

Merge branch 'main' of https://github.com/deephaven/web-client-ui int…

00c96df

…o 1143-partitioned-table-ui

Handle partitions and partitionColumns with model

12b638c

Update IrisGridPartitionSelector for PartitionedTable

dffd2bb

Fix partially selected partitions

7ead3b5

Merge branch 'main' of github.com:GcWan/web-client-ui into 1143-parti…

ac52c3f

…tioned-table-ui

Fix minor bugs and button formatting

35120eb

Add new partition selector dropdowns

64cfaf4

Merge branch 'main' of https://github.com/deephaven/web-client-ui int…

563a6c5

…o 1143-partitioned-table-ui

Fix bug when no partition is selected

a2cdec0

georgecwan requested review from dsmmcken and mofojed November 29, 2023 23:22

georgecwan self-assigned this Nov 29, 2023

georgecwan added 3 commits November 29, 2023 15:40

Fix error with char and long types

6bcb03a

Fix unit tests

0b578b1

Rename PartitionedTable model file

5f69436

Update key tables to match jsapi

3a5c1fb

georgecwan marked this pull request as ready for review December 1, 2023 21:15

mofojed requested changes Dec 4, 2023

View reviewed changes

dsmmcken reviewed Dec 4, 2023

View reviewed changes

packages/iris-grid/src/IrisGridPartitionSelector.tsx Outdated Show resolved Hide resolved

Remove partitionColumns setter

208dd61

georgecwan requested review from mofojed and dsmmcken December 7, 2023 17:53

Check partition in IrisGridTableModel constructor

3131270

dsmmcken requested changes Dec 7, 2023

View reviewed changes

Merge branch 'main' of https://github.com/deephaven/web-client-ui int…

93ab3bc

…o 1143-partitioned-table-ui

dsmmcken requested changes Dec 7, 2023

View reviewed changes

mofojed added 5 commits January 5, 2024 11:33

Don't throw for getting properties

6726531

Fix bad import

19a1b35

Fix failing test

a9e834b

Fix rehydration and char partitions

dd761a1

- Return the partitioned tables columns instead of no columns, so rehydration will succeed - Fix char type partitions - needed to create the raw filter value better

mofojed mentioned this pull request Jan 5, 2024

fix: Don't memoize retrieval of tables from JsPartitionedTable#getTable deephaven/deephaven-core#5009

Merged

bmingles reviewed Jan 5, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into 1143-partitioned-tabl…

cbd28aa

…e-ui

bmingles reviewed Jan 5, 2024

View reviewed changes

packages/iris-grid/src/IrisGridPartitionedTableModel.ts Outdated Show resolved Hide resolved

bmingles reviewed Jan 5, 2024

View reviewed changes

packages/iris-grid/src/IrisGridTableModel.ts Outdated Show resolved Hide resolved

bmingles requested changes Jan 5, 2024

View reviewed changes

Fix review comments

f4cba3a

- Add async to methods for consistency

mofojed requested a review from bmingles January 8, 2024 14:41

bmingles previously approved these changes Jan 8, 2024

View reviewed changes

mofojed enabled auto-merge (squash) January 8, 2024 15:06

mofojed previously approved these changes Jan 8, 2024

View reviewed changes

dsmmcken requested changes Jan 15, 2024

View reviewed changes

Fix the columns used in the partition selector

02542dd

- Was referencing the columns from the original table instead of the actual table - Was causing a mismatch when fetching the viewport, where it would just stall instead of throwing an error

mofojed requested a review from dsmmcken January 16, 2024 20:27

mofojed dismissed stale reviews from bmingles and themself via 02542dd January 16, 2024 21:20

mofojed and others added 3 commits January 18, 2024 11:42

Fix issues found in review with ticking partition tables

5ce6ff9

- Wait for a key to be added to the keys table if one does not initially exist - If the reloaded partition config is invalid, will reload the initial partition config

Merge remote-tracking branch 'origin/main' into pr/georgecwan/1663

b81cd58

adjust color variables

681bc24

dsmmcken approved these changes Jan 18, 2024

View reviewed changes

mofojed merged commit db219ca into deephaven:main Jan 18, 2024
4 of 5 checks passed

github-actions bot locked and limited conversation to collaborators Jan 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Create UI to Display Partitioned Tables #1663

feat: Create UI to Display Partitioned Tables #1663

georgecwan commented Nov 29, 2023 •

edited by mofojed

Loading

codecov bot commented Nov 30, 2023 •

edited

Loading

dsmmcken left a comment

georgecwan commented Dec 7, 2023 •

edited

Loading

dsmmcken left a comment

georgecwan commented Dec 7, 2023 •

edited

Loading

bmingles Jan 5, 2024

bmingles left a comment

bmingles left a comment

dsmmcken left a comment

mofojed commented Jan 16, 2024

mofojed commented Jan 18, 2024

dsmmcken left a comment

feat: Create UI to Display Partitioned Tables #1663

feat: Create UI to Display Partitioned Tables #1663

Conversation

georgecwan commented Nov 29, 2023 • edited by mofojed Loading

Testing Instructions

PartitionedTable

Parquet Tables

codecov bot commented Nov 30, 2023 • edited Loading

Codecov Report

dsmmcken left a comment

Choose a reason for hiding this comment

georgecwan commented Dec 7, 2023 • edited Loading

dsmmcken left a comment

Choose a reason for hiding this comment

georgecwan commented Dec 7, 2023 • edited Loading

bmingles Jan 5, 2024

Choose a reason for hiding this comment

bmingles left a comment

Choose a reason for hiding this comment

bmingles left a comment

Choose a reason for hiding this comment

dsmmcken left a comment

Choose a reason for hiding this comment

mofojed commented Jan 16, 2024

mofojed commented Jan 18, 2024

dsmmcken left a comment

Choose a reason for hiding this comment

georgecwan commented Nov 29, 2023 •

edited by mofojed

Loading

codecov bot commented Nov 30, 2023 •

edited

Loading

georgecwan commented Dec 7, 2023 •

edited

Loading

georgecwan commented Dec 7, 2023 •

edited

Loading