Skip to content

Commit

Permalink
fix: JS API rollups with unique aggs must specify null sentinel (deep…
Browse files Browse the repository at this point in the history 
…haven#6202)

Updated the HTML sample to support other missing aggregations.

Fixes deephaven#6200
  • Loading branch information
@niloc132
niloc132 authored Oct 16, 2024
1 parent 064aa6f commit 3692f7a
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 3 deletions.
10 changes: 8 additions & 2 deletions web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsRollupConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import elemental2.core.JsArray;
import elemental2.core.JsObject;
import elemental2.core.JsString;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.Table_pb;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.hierarchicaltable_pb.RollupRequest;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.AggSpec;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.Aggregation;
Expand All @@ -20,6 +21,7 @@
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecLast;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecMax;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecMin;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecNonUniqueSentinel;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecStd;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecSum;
import io.deephaven.javascript.proto.dhinternal.io.deephaven.proto.table_pb.aggspec.AggSpecUnique;
Expand Down Expand Up @@ -58,7 +60,7 @@ public class JsRollupConfig {
* Mapping from each aggregation name to the ordered list of columns it should be applied to in the resulting
* roll-up table.
*/
public JsPropertyMap<JsArray<@TsTypeRef(JsAggregationOperation.class) String>> aggregations =
public JsPropertyMap<JsArray<String>> aggregations =
Js.cast(JsObject.create(null));
/**
* Optional parameter indicating if an extra leaf node should be added at the bottom of the hierarchy, showing the
Expand Down Expand Up @@ -245,7 +247,11 @@ public RollupRequest buildRequest(JsArray<Column> tableColumns) {
}
case JsAggregationOperation.UNIQUE: {
AggSpec spec = new AggSpec();
spec.setUnique(new AggSpecUnique());
AggSpecUnique unique = new AggSpecUnique();
AggSpecNonUniqueSentinel sentinel = new AggSpecNonUniqueSentinel();
sentinel.setNullValue(Table_pb.NullValue.getNULL_VALUE());
unique.setNonUniqueSentinel(sentinel);
spec.setUnique(unique);
columns = new AggregationColumns();
columns.setSpec(spec);
agg.setColumns(columns);
Expand Down
4 changes: 4 additions & 0 deletions web/client-api/src/main/java/io/deephaven/web/public/rollup.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,18 @@ <h3>Rollup Table Configuration</h3>
<label for="operation">Aggregation Operation</label>
<select id="operation">
<option value="Count">Count</option>
<option value="CountDistinct">Count Distinct</option>
<option value="Distinct">Distinct</option>
<option value="Min">Min</option>
<option value="Max">Max</option>
<option value="Sum" selected>Sum</option>
<option value="AbsSum" selected>Absolute Sum</option>
<option value="Var">Var</option>
<option value="Avg">Avg</option>
<option value="Std">Std</option>
<option value="First">First</option>
<option value="Last">Last</option>
<option value="Unique">Unique</option>
</select>
</div>
</div>
Expand Down
57 changes: 56 additions & 1 deletion web/client-api/src/test/java/io/deephaven/web/client/api/HierarchicalTableTestGwt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,19 @@
//
package io.deephaven.web.client.api;

import elemental2.core.JsArray;
import elemental2.dom.CustomEvent;
import elemental2.promise.Promise;
import io.deephaven.web.client.api.tree.JsRollupConfig;
import io.deephaven.web.client.api.tree.JsTreeTable;
import io.deephaven.web.client.api.tree.TreeViewportData;
import io.deephaven.web.client.api.tree.enums.JsAggregationOperation;
import jsinterop.base.Js;
import jsinterop.base.JsPropertyMap;

import java.util.ArrayList;
import java.util.List;
import java.util.function.Supplier;

public class HierarchicalTableTestGwt extends AbstractAsyncGwtTestCase {
private static final Format red = new Format(0x1ff000001e0e0e0L, 0, null, null);
Expand All @@ -23,8 +32,10 @@ public String getModuleName() {
"empty_table(1000).update(['ID=i', 'Parent=i == 0 ? null : (int)(i/10)']).format_columns(['ID=ID>0 ? GREEN : RED']).tree('ID', 'Parent')")
.script("ticking_tree",
"time_table('PT0.1s').update(['ID=i', 'Parent=i == 0 ? null : (int)(i/10)']).format_columns(['ID=ID>0 ? GREEN : RED']).tree('ID', 'Parent')")
.script("table_to_rollup",
"time_table('PT0.1s').update(['Y=Math.sin(i/3)', 'X=i%3']).format_columns(['Y=Y>0 ? GREEN : RED'])")
.script("ticking_rollup",
"time_table('PT0.1s').update(['Y=Math.sin(i/3)', 'X=i%3']).format_columns(['Y=Y>0 ? GREEN : RED']).rollup(aggs=[agg.first('Y')],by=['X'],include_constituents=True)");
"table_to_rollup.rollup(aggs=[agg.first('Y')],by=['X'],include_constituents=True)");

public void testStaticTreeTable() {
connect(tables)
Expand Down Expand Up @@ -268,4 +279,48 @@ public void testTickingRollup() {
.then(this::finish).catch_(this::report);
}

public void testCreateRollup() {
connect(tables)
.then(table("table_to_rollup"))
.then(table -> {
List<Supplier<Promise<JsTreeTable>>> tests = new ArrayList<>();
// For each supported operation, apply it to the numeric column
// Then expand to verify data can load
List<String> count = List.of(
JsAggregationOperation.COUNT,
JsAggregationOperation.COUNT_DISTINCT,
// TODO(deephaven-core#6201) re-enable this line when fixed
// JsAggregationOperation.DISTINCT,
JsAggregationOperation.MIN,
JsAggregationOperation.MAX,
JsAggregationOperation.SUM,
JsAggregationOperation.ABS_SUM,
JsAggregationOperation.VAR,
JsAggregationOperation.AVG,
JsAggregationOperation.STD,
JsAggregationOperation.FIRST,
JsAggregationOperation.LAST,
JsAggregationOperation.UNIQUE);
for (int i = 0; i < count.size(); i++) {
final int step = i;
String operation = count.get(i);
JsRollupConfig cfg = new JsRollupConfig();
cfg.groupingColumns = Js.uncheckedCast(JsArray.of("X"));
cfg.includeConstituents = true;
cfg.aggregations = JsPropertyMap.of(operation, JsArray.of("Y"));
tests.add(() -> table.rollup(cfg).then(r -> {
r.setViewport(0, 99, null, null);
delayTestFinish(15000 + step);

return waitForEventWhere(r, JsTreeTable.EVENT_UPDATED,
(CustomEvent<TreeViewportData> d) -> r.getSize() == 4, 13000 + step)
.then(event -> Promise.resolve(r));
}));
}

return tests.stream().reduce((p1, p2) -> () -> p1.get().then(result -> p2.get())).get().get();
})
.then(this::finish).catch_(this::report);
}

}

0 comments on commit 3692f7a

Please sign in to comment.