From 06878aeec833bcc05bbb460a0ad60aa44b44a77c Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 7 Nov 2024 07:48:32 -0500 Subject: [PATCH] feat: Add withRecomputeOnModifiedRow to SelectColumn. (#6339) --- .../RecomputeOnModifiedRowSelectColumn.java | 44 ++++++ .../table/impl/select/SelectColumn.java | 27 ++++ .../impl/select/StatelessSelectColumn.java | 97 +------------ .../impl/select/WrappedSelectColumn.java | 134 ++++++++++++++++++ .../select/analyzers/ConstantColumnLayer.java | 4 + .../select/analyzers/DependencyLayerBase.java | 3 + .../select/analyzers/ViewColumnLayer.java | 4 + .../impl/QueryTableSelectUpdateTest.java | 124 +++++++++++++++- 8 files changed, 338 insertions(+), 99 deletions(-) create mode 100644 engine/table/src/main/java/io/deephaven/engine/table/impl/select/RecomputeOnModifiedRowSelectColumn.java create mode 100644 engine/table/src/main/java/io/deephaven/engine/table/impl/select/WrappedSelectColumn.java diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/RecomputeOnModifiedRowSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/RecomputeOnModifiedRowSelectColumn.java new file mode 100644 index 00000000000..91a6f079b0c --- /dev/null +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/RecomputeOnModifiedRowSelectColumn.java @@ -0,0 +1,44 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.engine.table.impl.select; + +import io.deephaven.api.ColumnName; +import io.deephaven.api.expression.Expression; +import io.deephaven.engine.rowset.TrackingRowSet; +import io.deephaven.engine.table.ColumnDefinition; +import io.deephaven.engine.table.ColumnSource; +import io.deephaven.engine.table.WritableColumnSource; +import io.deephaven.engine.table.impl.BaseTable; +import io.deephaven.engine.table.impl.MatchPair; +import io.deephaven.engine.table.impl.QueryCompilerRequestProcessor; +import org.jetbrains.annotations.NotNull; + +import java.util.List; +import java.util.Map; + +/** + * {@link SelectColumn} implementation that wraps another {@link SelectColumn} and makes it report that values should be + * recomputed when the row is modified via {@link #recomputeOnModifiedRow()}. + */ +class RecomputeOnModifiedRowSelectColumn extends WrappedSelectColumn { + + RecomputeOnModifiedRowSelectColumn(@NotNull final SelectColumn inner) { + super(inner); + } + + @Override + public SelectColumn copy() { + return new RecomputeOnModifiedRowSelectColumn(inner.copy()); + } + + @Override + public boolean recomputeOnModifiedRow() { + return true; + } + + @Override + public SelectColumn withRecomputeOnModifiedRow() { + return copy(); + } +} diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/SelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/SelectColumn.java index 2af2eedd5ae..249ec9a14f2 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/SelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/SelectColumn.java @@ -67,6 +67,17 @@ static SelectColumn ofStateless(@NotNull final Selectable selectable) { return new StatelessSelectColumn(of(selectable)); } + /** + * Produce a SelectColumn that {@link #recomputeOnModifiedRow()} recomputes values on any modified row} from + * {@code selectable}. + * + * @param selectable The {@link Selectable} to adapt and mark as requiring row-level recomputation + * @return The resulting SelectColumn + */ + static SelectColumn ofRecomputeOnModifiedRow(Selectable selectable) { + return new RecomputeOnModifiedRowSelectColumn(of(selectable)); + } + /** * Convenient static final instance of a zero length Array of SelectColumns for use in toArray calls. */ @@ -232,6 +243,22 @@ default boolean hasVirtualRowVariables() { */ SelectColumn copy(); + /** + * Should we ignore modified column sets, and always re-evaluate this column when the row changes? + * + * @return true if this column should be evaluated on every row modification + */ + default boolean recomputeOnModifiedRow() { + return false; + } + + /** + * Create a copy of this SelectColumn that always re-evaluates itself when a row is modified. + */ + default SelectColumn withRecomputeOnModifiedRow() { + return new RecomputeOnModifiedRowSelectColumn(copy()); + } + class ExpressionAdapter implements Expression.Visitor { private final ColumnName lhs; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/StatelessSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/StatelessSelectColumn.java index 97ca5052ce4..d257ab1ff4e 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/StatelessSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/StatelessSelectColumn.java @@ -21,93 +21,10 @@ * {@link SelectColumn} implementation that wraps another {@link SelectColumn} and makes it report to be * {@link #isStateless() stateless}. */ -class StatelessSelectColumn implements SelectColumn { - - private final SelectColumn inner; +class StatelessSelectColumn extends WrappedSelectColumn { StatelessSelectColumn(@NotNull final SelectColumn inner) { - this.inner = inner; - } - - @Override - public List initInputs( - @NotNull final TrackingRowSet rowSet, - @NotNull final Map> columnsOfInterest) { - return inner.initInputs(rowSet, columnsOfInterest); - } - - @Override - public List initDef(@NotNull final Map> columnDefinitionMap) { - return inner.initDef(columnDefinitionMap); - } - - @Override - public List initDef( - @NotNull final Map> columnDefinitionMap, - @NotNull final QueryCompilerRequestProcessor compilationRequestProcessor) { - return inner.initDef(columnDefinitionMap, compilationRequestProcessor); - } - - @Override - public Class getReturnedType() { - return inner.getReturnedType(); - } - - @Override - public Class getReturnedComponentType() { - return inner.getReturnedComponentType(); - } - - @Override - public List getColumns() { - return inner.getColumns(); - } - - @Override - public List getColumnArrays() { - return inner.getColumnArrays(); - } - - @Override - @NotNull - public ColumnSource getDataView() { - return inner.getDataView(); - } - - @Override - @NotNull - public ColumnSource getLazyView() { - return inner.getLazyView(); - } - - @Override - public String getName() { - return inner.getName(); - } - - @Override - public MatchPair getMatchPair() { - return inner.getMatchPair(); - } - - @Override - public WritableColumnSource newDestInstance(final long size) { - return inner.newDestInstance(size); - } - - @Override - public WritableColumnSource newFlatDestInstance(final long size) { - return inner.newFlatDestInstance(size); - } - - @Override - public boolean isRetain() { - return inner.isRetain(); - } - - @Override - public void validateSafeForRefresh(@NotNull final BaseTable sourceTable) { - inner.validateSafeForRefresh(sourceTable); + super(inner); } @Override @@ -119,14 +36,4 @@ public boolean isStateless() { public SelectColumn copy() { return new StatelessSelectColumn(inner.copy()); } - - @Override - public ColumnName newColumn() { - return inner.newColumn(); - } - - @Override - public Expression expression() { - return inner.expression(); - } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WrappedSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WrappedSelectColumn.java new file mode 100644 index 00000000000..a77bf533d2d --- /dev/null +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WrappedSelectColumn.java @@ -0,0 +1,134 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.engine.table.impl.select; + +import io.deephaven.api.ColumnName; +import io.deephaven.api.expression.Expression; +import io.deephaven.engine.rowset.TrackingRowSet; +import io.deephaven.engine.table.ColumnDefinition; +import io.deephaven.engine.table.ColumnSource; +import io.deephaven.engine.table.WritableColumnSource; +import io.deephaven.engine.table.impl.BaseTable; +import io.deephaven.engine.table.impl.MatchPair; +import io.deephaven.engine.table.impl.QueryCompilerRequestProcessor; +import org.jetbrains.annotations.NotNull; + +import java.util.List; +import java.util.Map; + +/** + * {@link SelectColumn} implementation that wraps another {@link SelectColumn}. + */ +abstract class WrappedSelectColumn implements SelectColumn { + + /** + * The select column that is being wrapped. + */ + protected final SelectColumn inner; + + WrappedSelectColumn(@NotNull final SelectColumn inner) { + this.inner = inner; + } + + @Override + public List initInputs( + @NotNull final TrackingRowSet rowSet, + @NotNull final Map> columnsOfInterest) { + return inner.initInputs(rowSet, columnsOfInterest); + } + + @Override + public List initDef(@NotNull final Map> columnDefinitionMap) { + return inner.initDef(columnDefinitionMap); + } + + @Override + public List initDef( + @NotNull final Map> columnDefinitionMap, + @NotNull final QueryCompilerRequestProcessor compilationRequestProcessor) { + return inner.initDef(columnDefinitionMap, compilationRequestProcessor); + } + + @Override + public Class getReturnedType() { + return inner.getReturnedType(); + } + + @Override + public Class getReturnedComponentType() { + return inner.getReturnedComponentType(); + } + + @Override + public List getColumns() { + return inner.getColumns(); + } + + @Override + public List getColumnArrays() { + return inner.getColumnArrays(); + } + + @Override + @NotNull + public ColumnSource getDataView() { + return inner.getDataView(); + } + + @Override + @NotNull + public ColumnSource getLazyView() { + return inner.getLazyView(); + } + + @Override + public String getName() { + return inner.getName(); + } + + @Override + public MatchPair getMatchPair() { + return inner.getMatchPair(); + } + + @Override + public WritableColumnSource newDestInstance(final long size) { + return inner.newDestInstance(size); + } + + @Override + public WritableColumnSource newFlatDestInstance(final long size) { + return inner.newFlatDestInstance(size); + } + + @Override + public boolean isRetain() { + return inner.isRetain(); + } + + @Override + public void validateSafeForRefresh(@NotNull final BaseTable sourceTable) { + inner.validateSafeForRefresh(sourceTable); + } + + @Override + public boolean isStateless() { + return inner.isStateless(); + } + + @Override + public boolean recomputeOnModifiedRow() { + return inner.recomputeOnModifiedRow(); + } + + @Override + public ColumnName newColumn() { + return inner.newColumn(); + } + + @Override + public Expression expression() { + return inner.expression(); + } +} diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ConstantColumnLayer.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ConstantColumnLayer.java index 4738d944825..fdcb877e752 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ConstantColumnLayer.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ConstantColumnLayer.java @@ -22,6 +22,10 @@ public class ConstantColumnLayer extends SelectOrViewColumnLayer { final String[] deps, final ModifiedColumnSet mcsBuilder) { super(context, sc, ws, null, deps, mcsBuilder); + if (sc.recomputeOnModifiedRow()) { + throw new IllegalArgumentException( + "SelectColumn may not have alwaysEvaluate set for a constant column: " + sc); + } initialize(ws); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/DependencyLayerBase.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/DependencyLayerBase.java index 7d00107d17e..7bd050e9c10 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/DependencyLayerBase.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/DependencyLayerBase.java @@ -30,6 +30,9 @@ public abstract class DependencyLayerBase extends SelectAndViewAnalyzer.Layer { selectColumnHoldsVector = Vector.class.isAssignableFrom(selectColumn.getReturnedType()); this.columnSource = columnSource; context.populateParentDependenciesMCS(mcsBuilder, dependencies); + if (selectColumn.recomputeOnModifiedRow()) { + mcsBuilder.setAll(ModifiedColumnSet.ALL); + } this.myModifiedColumnSet = mcsBuilder; this.myLayerDependencySet = new BitSet(); context.populateLayerDependencySet(myLayerDependencySet, dependencies); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ViewColumnLayer.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ViewColumnLayer.java index 3019b5277b1..890324b77f7 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ViewColumnLayer.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/analyzers/ViewColumnLayer.java @@ -23,6 +23,10 @@ final public class ViewColumnLayer extends SelectOrViewColumnLayer { final String[] deps, final ModifiedColumnSet mcsBuilder) { super(context, sc, checkResultType(cs), null, deps, mcsBuilder); + if (sc.recomputeOnModifiedRow()) { + throw new IllegalArgumentException( + "SelectColumn may not have recomputeOnModifiedRow set for a view column: " + sc); + } } @Override diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableSelectUpdateTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableSelectUpdateTest.java index 35596b75d0e..c8eb9073dbe 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableSelectUpdateTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableSelectUpdateTest.java @@ -11,15 +11,14 @@ import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.liveness.LivenessScope; import io.deephaven.engine.liveness.LivenessScopeStack; -import io.deephaven.engine.rowset.RowSet; -import io.deephaven.engine.rowset.RowSetBuilderSequential; -import io.deephaven.engine.rowset.RowSetFactory; -import io.deephaven.engine.rowset.WritableRowSet; +import io.deephaven.engine.rowset.*; import io.deephaven.engine.table.ColumnSource; import io.deephaven.engine.table.ShiftObliviousListener; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.impl.select.DhFormulaColumn; import io.deephaven.engine.table.impl.select.FormulaCompilationException; +import io.deephaven.engine.table.impl.select.SelectColumn; +import io.deephaven.engine.table.impl.select.SelectColumnFactory; import io.deephaven.engine.table.impl.sources.InMemoryColumnSource; import io.deephaven.engine.table.impl.sources.LongSparseArraySource; import io.deephaven.engine.table.impl.sources.RedirectedColumnSource; @@ -33,6 +32,7 @@ import io.deephaven.engine.testutil.generator.SetGenerator; import io.deephaven.engine.testutil.junit4.EngineCleanup; import io.deephaven.engine.testutil.testcase.RefreshingTableTestCase; +import io.deephaven.engine.util.PrintListener; import io.deephaven.engine.util.TableTools; import io.deephaven.util.SafeCloseable; import io.deephaven.util.mutable.MutableInt; @@ -51,6 +51,7 @@ import static io.deephaven.util.QueryConstants.NULL_INT; import static java.util.Collections.emptyList; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; /** * Test QueryTable select and update operations. @@ -1300,4 +1301,119 @@ public void testPropagationOfAttributes() { Assert.assertTrue(result.isBlink()); } } + + @Test + public void testAlwaysUpdate() { + final MutableInt count = new MutableInt(0); + final MutableInt count2 = new MutableInt(0); + QueryScope.addParam("__COUNT1", count); + QueryScope.addParam("__COUNT2", count2); + final QueryTable base = testRefreshingTable(intCol("Sentinel", 1, 2, 3, 4, 5, 6, 7, 8, 9), + stringCol("Thing", "A", "B", "C", "D", "E", "F", "G", "H", "I")); + + final SelectColumn sc1 = SelectColumnFactory.getExpression("NormalCount=__COUNT1.getAndIncrement()"); + final SelectColumn sc2 = + SelectColumnFactory.getExpression("AlwaysCount=__COUNT2.getAndIncrement()") + .withRecomputeOnModifiedRow(); + + final Table withUpdates = base.update(Arrays.asList(sc1, sc2)); + + final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); + + assertArrayEquals(new int[] {0, 1, 2, 3, 4, 5, 6, 7, 8}, + ColumnVectors.ofInt(withUpdates, "NormalCount").toArray()); + assertArrayEquals(new int[] {0, 1, 2, 3, 4, 5, 6, 7, 8}, + ColumnVectors.ofInt(withUpdates, "AlwaysCount").toArray()); + + updateGraph.runWithinUnitTestCycle(() -> { + addToTable(base, i(9), intCol("Sentinel", 10), stringCol("Thing", "J")); + base.notifyListeners(i(9), i(), i()); + }); + + assertArrayEquals(new int[] {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, + ColumnVectors.ofInt(withUpdates, "NormalCount").toArray()); + assertArrayEquals(new int[] {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, + ColumnVectors.ofInt(withUpdates, "AlwaysCount").toArray()); + + updateGraph.runWithinUnitTestCycle(() -> { + addToTable(base, i(0, 2, 4), intCol("Sentinel", 1, 3, 5), stringCol("Thing", "a", "c", "e")); + base.notifyListeners(i(), i(), i(0, 2, 4)); + }); + + assertArrayEquals(new int[] {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}, + ColumnVectors.ofInt(withUpdates, "NormalCount").toArray()); + assertArrayEquals(new int[] {10, 1, 11, 3, 12, 5, 6, 7, 8, 9}, + ColumnVectors.ofInt(withUpdates, "AlwaysCount").toArray()); + + updateGraph.runWithinUnitTestCycle(() -> { + removeRows(base, i(5)); + base.notifyListeners(i(), i(5), i()); + }); + + assertArrayEquals(new int[] {0, 1, 2, 3, 4, 6, 7, 8, 9}, + ColumnVectors.ofInt(withUpdates, "NormalCount").toArray()); + assertArrayEquals(new int[] {10, 1, 11, 3, 12, 6, 7, 8, 9}, + ColumnVectors.ofInt(withUpdates, "AlwaysCount").toArray()); + } + + @Test + public void testAlwaysUpdateMCS() { + final AtomicInteger a = new AtomicInteger(100); + QueryScope.addParam("a", a); + final AtomicInteger b = new AtomicInteger(200); + QueryScope.addParam("b", b); + final QueryTable base = testRefreshingTable(RowSetFactory.fromKeys(10, 11).toTracking(), + intCol("Sentinel", 1, 2), intCol("B", 10, 11)); + + final SelectColumn x = + SelectColumnFactory.getExpression("X = a.getAndIncrement()").withRecomputeOnModifiedRow(); + + final Table withUpdates = base.update(Arrays.asList(x, SelectColumnFactory.getExpression("Y=1"), + SelectColumnFactory.getExpression("Z=B+b.getAndIncrement()"))); + + final PrintListener pl = new PrintListener("withUpdates", withUpdates, 10); + + final SimpleListener simpleListener = new SimpleListener(withUpdates); + withUpdates.addUpdateListener(simpleListener); + + assertTableEquals(TableTools.newTable(intCol("Sentinel", 1, 2), intCol("B", 10, 11), intCol("X", 100, 101), + intCol("Y", 1, 1), intCol("Z", 210, 212)), withUpdates); + + final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); + + updateGraph.runWithinUnitTestCycle(() -> { + addToTable(base, i(10), intCol("Sentinel", 3), intCol("B", 10)); + base.notifyListeners( + new TableUpdateImpl(i(), i(), i(10), RowSetShiftData.EMPTY, base.newModifiedColumnSet("Sentinel"))); + }); + + assertTableEquals(TableTools.newTable(intCol("Sentinel", 3, 2), intCol("B", 10, 11), intCol("X", 102, 101), + intCol("Y", 1, 1), intCol("Z", 210, 212)), withUpdates); + + assertEquals(1, simpleListener.count); + assertEquals(i(), simpleListener.update.added()); + assertEquals(i(), simpleListener.update.removed()); + assertEquals(i(10), simpleListener.update.modified()); + assertEquals(((QueryTable) withUpdates).newModifiedColumnSet("Sentinel", "X"), + simpleListener.update.modifiedColumnSet()); + + updateGraph.runWithinUnitTestCycle(() -> { + addToTable(base, i(11), intCol("Sentinel", 4), intCol("B", 12)); + base.notifyListeners(new TableUpdateImpl(i(), i(), i(11), RowSetShiftData.EMPTY, + base.newModifiedColumnSet("Sentinel", "B"))); + }); + + assertTableEquals(TableTools.newTable(intCol("Sentinel", 3, 4), intCol("B", 10, 12), intCol("X", 102, 103), + intCol("Y", 1, 1), intCol("Z", 210, 214)), withUpdates); + + assertEquals(2, simpleListener.count); + assertEquals(i(), simpleListener.update.added()); + assertEquals(i(), simpleListener.update.removed()); + assertEquals(i(11), simpleListener.update.modified()); + assertEquals(((QueryTable) withUpdates).newModifiedColumnSet("Sentinel", "B", "X", "Z"), + simpleListener.update.modifiedColumnSet()); + + QueryScope.addParam("a", null); + QueryScope.addParam("b", null); + } }