From 765934bc02f17651158db76f1fb58b706617ad80 Mon Sep 17 00:00:00 2001 From: Mike Bender Date: Tue, 2 Apr 2024 16:48:04 -0400 Subject: [PATCH] fix: Use correct schema when opening transformed partitioned tables (#5305) - We were using the schema from the constituent table when trying to reference the key columns instead of the keys table - Was not a problem when the schemas matched, but if there was a transform on the table removing one of the key columns, it would throw an error - Fixes #5304 - Tested using the snippet in the ticket - Added unit tests --- .../web/client/api/JsPartitionedTable.java | 14 +- .../deephaven/web/client/ide/IdeSession.java | 5 + .../web/ClientIntegrationTestSuite.java | 2 + .../client/api/AbstractAsyncGwtTestCase.java | 4 + .../client/api/PartitionedTableTestGwt.java | 173 ++++++++++++++++++ 5 files changed, 192 insertions(+), 6 deletions(-) create mode 100644 web/client-api/src/test/java/io/deephaven/web/client/api/PartitionedTableTestGwt.java diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java b/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java index 63298207f72..70365e57c30 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/api/JsPartitionedTable.java @@ -83,6 +83,10 @@ public Promise refetch() { return widget.refetch().then(w -> { descriptor = PartitionedTableDescriptor.deserializeBinary(w.getDataAsU8()); + return w.getExportedObjects()[0].fetch(); + }).then(result -> { + keys = (JsTable) result; + keyColumnTypes = new ArrayList<>(); InitialTableDefinition tableDefinition = WebBarrageUtils.readTableDefinition( WebBarrageUtils.readSchemaMessage(descriptor.getConstituentDefinitionSchema_asU8())); @@ -97,15 +101,13 @@ public Promise refetch() { JsArray keyColumnNames = descriptor.getKeyColumnNamesList(); for (int i = 0; i < keyColumnNames.length; i++) { String name = keyColumnNames.getAt(i); - ColumnDefinition columnDefinition = tableDefinition.getColumnsByName().get(false).get(name); - keyColumnTypes.add(columnDefinition.getType()); - keyColumns[keyColumns.length] = columns[columnDefinition.getColumnIndex()]; + Column keyColumn = keys.findColumn(name); + keyColumnTypes.add(keyColumn.getType()); + keyColumns[keyColumns.length] = keyColumn; } this.columns = JsObject.freeze(columns); this.keyColumns = JsObject.freeze(keyColumns); - return w.getExportedObjects()[0].fetch(); - }).then(result -> { - keys = (JsTable) result; + // TODO(deephaven-core#3604) in case of a new session, we should do a full refetch keys.addEventListener(JsTable.EVENT_DISCONNECT, event -> fireEvent(EVENT_DISCONNECT)); keys.addEventListener(JsTable.EVENT_RECONNECT, event -> { diff --git a/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java b/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java index c3a2714fc05..314daf265a9 100644 --- a/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java +++ b/web/client-api/src/main/java/io/deephaven/web/client/ide/IdeSession.java @@ -138,6 +138,11 @@ public Promise getHierarchicalTable(String name) { .then(connection::getHierarchicalTable); } + public Promise getPartitionedTable(String name) { + return connection.getVariableDefinition(name, JsVariableType.PARTITIONEDTABLE) + .then(connection::getPartitionedTable); + } + public Promise getObject(@TsTypeRef(JsVariableDescriptor.class) JsPropertyMap definitionObject) { return connection.getJsObject(definitionObject); } diff --git a/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java b/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java index 994f2413047..961f4b7cb22 100644 --- a/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java +++ b/web/client-api/src/test/java/io/deephaven/web/ClientIntegrationTestSuite.java @@ -6,6 +6,7 @@ import com.google.gwt.junit.tools.GWTTestSuite; import io.deephaven.web.client.api.HierarchicalTableTestGwt; import io.deephaven.web.client.api.NullValueTestGwt; +import io.deephaven.web.client.api.PartitionedTableTestGwt; import io.deephaven.web.client.api.subscription.ConcurrentTableTestGwt; import io.deephaven.web.client.api.TableManipulationTestGwt; import io.deephaven.web.client.api.subscription.ViewportTestGwt; @@ -26,6 +27,7 @@ public static Test suite() { suite.addTestSuite(ConcurrentTableTestGwt.class); suite.addTestSuite(NullValueTestGwt.class); suite.addTestSuite(HierarchicalTableTestGwt.class); + suite.addTestSuite(PartitionedTableTestGwt.class); // Unfinished: // suite.addTestSuite(TotalsTableTestGwt.class); diff --git a/web/client-api/src/test/java/io/deephaven/web/client/api/AbstractAsyncGwtTestCase.java b/web/client-api/src/test/java/io/deephaven/web/client/api/AbstractAsyncGwtTestCase.java index b0eab4ffe49..0035fa5ad05 100644 --- a/web/client-api/src/test/java/io/deephaven/web/client/api/AbstractAsyncGwtTestCase.java +++ b/web/client-api/src/test/java/io/deephaven/web/client/api/AbstractAsyncGwtTestCase.java @@ -191,6 +191,10 @@ public IThenable.ThenOnFulfilledCallbackFn treeTable(St return session -> session.getTreeTable(tableName); } + public IThenable.ThenOnFulfilledCallbackFn partitionedTable(String tableName) { + return session -> session.getPartitionedTable(tableName); + } + /** * Utility method to report Promise errors to the unit test framework */ diff --git a/web/client-api/src/test/java/io/deephaven/web/client/api/PartitionedTableTestGwt.java b/web/client-api/src/test/java/io/deephaven/web/client/api/PartitionedTableTestGwt.java new file mode 100644 index 00000000000..0efa47a0ce7 --- /dev/null +++ b/web/client-api/src/test/java/io/deephaven/web/client/api/PartitionedTableTestGwt.java @@ -0,0 +1,173 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.web.client.api; + +import elemental2.core.JsArray; +import elemental2.dom.CustomEvent; +import io.deephaven.web.client.api.subscription.ViewportData; +import io.deephaven.web.client.api.tree.JsTreeTable; + +public class PartitionedTableTestGwt extends AbstractAsyncGwtTestCase { + @Override + public String getModuleName() { + return "io.deephaven.web.DeephavenIntegrationTest"; + } + + private final TableSourceBuilder tables = new TableSourceBuilder() + .script("from deephaven import empty_table") + .script("source = empty_table(100).update(['MyKey=``+i%5', 'x=i'])") + .script("partitioned_source = source.partition_by(by=['MyKey'])") + .script("partitioned_result = partitioned_source.transform(func=lambda t: t.drop_columns('MyKey'))") + .script("constituent_result = partitioned_result.get_constituent(['0'])"); + + private final TableSourceBuilder tickingTables = new TableSourceBuilder() + .script("from deephaven import time_table") + .script("source = time_table('PT0.1s').update(['MyKey=``+i%5', 'x=i'])") + .script("partitioned_source = source.partition_by(by=['MyKey'])") + .script("partitioned_result = partitioned_source.transform(func=lambda t: t.drop_columns('MyKey'))"); + + + public void testPartitionedTable() { + connect(tables) + .then(partitionedTable("partitioned_source")) + .then(partitionedTable -> { + delayTestFinish(1500); + Column[] keyColumns = partitionedTable.getKeyColumns(); + assertEquals(1, keyColumns.length); + assertEquals("MyKey", keyColumns[0].getName()); + + Column[] columns = partitionedTable.getColumns(); + assertEquals(2, columns.length); + assertEquals("MyKey", columns[0].getName()); + assertEquals("x", columns[1].getName()); + + return partitionedTable.getKeyTable().then(keyTable -> { + System.out.println("KeyTable size: " + keyTable.getSize()); + assertEquals(5d, keyTable.getSize()); + + return partitionedTable.getTable("2"); + }).then(constituentTable -> { + assertEquals(20d, constituentTable.getSize()); + partitionedTable.close(); + + return null; + }); + }) + .then(this::finish).catch_(this::report); + } + + public void testTransformedPartitionedTable() { + connect(tables) + .then(partitionedTable("partitioned_result")) + .then(partitionedTable -> { + delayTestFinish(1500); + Column[] keyColumns = partitionedTable.getKeyColumns(); + assertEquals(1, keyColumns.length); + assertEquals("MyKey", keyColumns[0].getName()); + + Column[] columns = partitionedTable.getColumns(); + assertEquals(1, columns.length); + assertEquals("x", columns[0].getName()); + + return partitionedTable.getKeyTable().then(keyTable -> { + assertEquals(5d, keyTable.getSize()); + + return partitionedTable.getTable("2"); + }).then(constituentTable -> { + assertEquals(20d, constituentTable.getSize()); + constituentTable.close(); + partitionedTable.close(); + return null; + }); + }) + .then(this::finish).catch_(this::report); + } + + public void testConstituentResult() { + connect(tables) + .then(table("constituent_result")) + .then(table -> { + delayTestFinish(1500); + + JsArray columns = table.getColumns(); + assertEquals(1, columns.length); + assertEquals("x", columns.getAt(0).getName()); + + table.close(); + return null; + }) + .then(this::finish).catch_(this::report); + } + + public void testTickingPartitionedTable() { + connect(tickingTables) + .then(partitionedTable("partitioned_source")) + .then(partitionedTable -> { + delayTestFinish(20_000); + Column[] keyColumns = partitionedTable.getKeyColumns(); + assertEquals(1, keyColumns.length); + assertEquals("MyKey", keyColumns[0].getName()); + + Column[] columns = partitionedTable.getColumns(); + assertEquals(3, columns.length); + assertEquals("MyKey", columns[1].getName()); + assertEquals("x", columns[2].getName()); + + return partitionedTable.getKeyTable().then(keyTable -> { + keyTable.setViewport(0, 99, keyTable.getColumns(), null); + return keyTable.getViewportData().then(data -> { + assertEquals(0d, keyTable.getSize()); + + return waitForEventWhere(keyTable, JsTable.EVENT_UPDATED, + (CustomEvent d) -> d.detail.getRows().length == 5, 20004); + }); + }).then(event -> partitionedTable.getTable("2")).then(constituentTable -> { + assertEquals(3, constituentTable.getColumns().length); + assertEquals(2d, constituentTable.getSize()); + + constituentTable.close(); + partitionedTable.close(); + + return null; + }); + }) + .then(this::finish).catch_(this::report); + } + + public void testTickingTransformedPartitionedTable() { + connect(tickingTables) + .then(partitionedTable("partitioned_result")) + .then(partitionedTable -> { + delayTestFinish(20_000); + Column[] keyColumns = partitionedTable.getKeyColumns(); + assertEquals(1, keyColumns.length); + assertEquals("MyKey", keyColumns[0].getName()); + + Column[] columns = partitionedTable.getColumns(); + assertEquals(2, columns.length); + assertEquals("Timestamp", columns[0].getName()); + assertEquals("x", columns[1].getName()); + + return partitionedTable.getKeyTable().then(keyTable -> { + keyTable.setViewport(0, 99, keyTable.getColumns(), null); + return keyTable.getViewportData().then(data -> { + assertEquals(0d, keyTable.getSize()); + + return waitForEventWhere(keyTable, JsTable.EVENT_UPDATED, + (CustomEvent d) -> d.detail.getRows().length == 5, 20004); + }).then(event -> partitionedTable.getTable("2")).then(constituentTable -> { + assertEquals(2, constituentTable.getColumns().length); + assertEquals(2d, constituentTable.getSize()); + + keyTable.close(); + constituentTable.close(); + partitionedTable.close(); + + return null; + }); + }); + }) + .then(this::finish).catch_(this::report); + } +}