From 8d4f823bf85b21173bbf24a587ed2083a165e186 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 7 Nov 2024 09:31:20 -0500 Subject: [PATCH 01/18] Allow a Filter to be used as the Expression in a SelectColumn. --- .../table/impl/WouldMatchOperation.java | 15 +- .../impl/select/AbstractConditionFilter.java | 5 + .../table/impl/select/FilterSelectColumn.java | 218 ++++++++++++++++++ .../table/impl/select/SelectColumn.java | 2 +- .../engine/table/impl/select/WhereFilter.java | 9 +- .../impl/QueryTableSelectUpdateTest.java | 102 ++++++++ 6 files changed, 347 insertions(+), 4 deletions(-) create mode 100644 engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java index 43e20c8d9b1..fef43eed902 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java @@ -113,6 +113,17 @@ public SafeCloseable beginOperation(@NotNull final QueryTable parent) { void initializeFilters(@NotNull QueryTable parent) { final QueryCompilerRequestProcessor.BatchProcessor compilationProcessor = QueryCompilerRequestProcessor.batch(); Arrays.stream(whereFilters).forEach(filter -> filter.init(parent.getDefinition(), compilationProcessor)); + + final List disallowedRowVariables = Arrays.stream(whereFilters).filter(WhereFilter::hasVirtualRowVariables).collect(Collectors.toList()); + if (!disallowedRowVariables.isEmpty()) { + throw new UncheckedTableException("wouldMatch filters cannot use virtual row variables (i, ii, and k): " + disallowedRowVariables); + } + + final List disallowedColumnVectors = Arrays.stream(whereFilters).filter(wf -> !wf.getColumnArrays().isEmpty()).collect(Collectors.toList()); + if (!disallowedColumnVectors.isEmpty()) { + throw new UncheckedTableException("wouldMatch filters cannot use column Vectors (_ syntax): " + disallowedColumnVectors); + } + compilationProcessor.compile(); } @@ -517,13 +528,13 @@ private RowSet update(RowSet added, RowSet removed, RowSet modified, try (final SafeCloseableList toClose = new SafeCloseableList()) { // Filter and add addeds - final WritableRowSet filteredAdded = toClose.add(filter.filter(added, source, table, false)); + final WritableRowSet filteredAdded = toClose.add(filter.filter(added, table.getRowSet(), table, false)); RowSet keysToRemove = EMPTY_INDEX; // If we were affected, recompute mods and re-add the ones that pass. if (affected) { downstreamModified.setAll(name); - final RowSet filteredModified = toClose.add(filter.filter(modified, source, table, false)); + final RowSet filteredModified = toClose.add(filter.filter(modified, table.getRowSet(), table, false)); // Now apply the additions and remove any non-matching modifieds filteredAdded.insert(filteredModified); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java index 2007eee6526..9b9602843f5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java @@ -311,6 +311,11 @@ public boolean hasConstantArrayAccess() { return getFormulaShiftColPair() != null; } + @Override + public boolean hasVirtualRowVariables() { + return usesI || usesII || usesK; + } + /** * @return a Pair object, consisting of formula string and shift to column MatchPairs, if the filter formula or * expression has Array Access that conforms to "i +/- <constant>" or "ii +/- <constant>". If diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java new file mode 100644 index 00000000000..a05cdde430f --- /dev/null +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -0,0 +1,218 @@ +// +// Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending +// +package io.deephaven.engine.table.impl.select; + +import io.deephaven.chunk.ChunkType; +import io.deephaven.chunk.WritableChunk; +import io.deephaven.chunk.WritableObjectChunk; +import io.deephaven.chunk.attributes.Values; +import io.deephaven.engine.exceptions.UncheckedTableException; +import io.deephaven.engine.rowset.RowSequence; +import io.deephaven.engine.rowset.RowSet; +import io.deephaven.engine.rowset.RowSetFactory; +import io.deephaven.engine.rowset.TrackingRowSet; +import io.deephaven.engine.table.*; +import io.deephaven.engine.table.impl.MatchPair; +import io.deephaven.engine.table.impl.QueryTable; +import io.deephaven.engine.table.impl.chunkfillers.ChunkFiller; +import io.deephaven.engine.table.impl.sources.InMemoryColumnSource; +import io.deephaven.engine.table.impl.sources.SparseArrayColumnSource; +import io.deephaven.engine.table.impl.sources.ViewColumnSource; +import io.deephaven.qst.column.header.ColumnHeader; +import io.deephaven.util.type.TypeUtils; +import org.jetbrains.annotations.NotNull; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +public class FilterSelectColumn implements SelectColumn { + + // We don't actually care to do anything, but cannot return null + private static final Formula.FillContext FILL_CONTEXT_INSTANCE = new Formula.FillContext() { + }; + + @NotNull private final String destName; + @NotNull private final WhereFilter filter; + + private RowSet rowSet; + private Table tableToFilter; + + public FilterSelectColumn(@NotNull final String destName, @NotNull final WhereFilter filter) { + this.destName = destName; + this.filter = filter; + } + + @Override + public String toString() { + return "filter(" + filter + ')'; + } + + @Override + public List initInputs(TrackingRowSet rowSet, Map> columnsOfInterest) { + this.rowSet = rowSet; + this.tableToFilter = new QueryTable(rowSet, columnsOfInterest); + return filter.getColumns(); + } + + @Override + public List initDef(@NotNull final Map> columnDefinitionMap) { + final List> columnHeaders = new ArrayList<>(); + for (Map.Entry> entry : columnDefinitionMap.entrySet()) { + columnHeaders.add(ColumnHeader.of(entry.getKey(), entry.getValue().getDataType())); + } + final TableDefinition fromColumns = TableDefinition.from(columnHeaders); + // TODO: Compilation processor! + filter.init(fromColumns); + + if (!filter.getColumnArrays().isEmpty()) { + throw new UncheckedTableException("Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: " + filter); + } + if (filter.hasVirtualRowVariables()) { + throw new UncheckedTableException("Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: " + filter); + } + + return filter.getColumns(); + } + + @Override + public Class getReturnedType() { + return Boolean.class; + } + + @Override + public Class getReturnedComponentType() { + return null; + } + + @Override + public List getColumns() { + return filter.getColumns(); + } + + @Override + public List getColumnArrays() { + return List.of(); + } + + @Override + public boolean hasVirtualRowVariables() { + return filter.hasVirtualRowVariables(); + } + + @NotNull + @Override + public ColumnSource getDataView() { + return new ViewColumnSource<>(Boolean.class, new Formula(null) { + + @Override + public Boolean getBoolean(long rowKey) { + return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, false).containsRange(rowKey, rowKey); + } + + @Override + public Boolean getPrevBoolean(long rowKey) { + return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, true).containsRange(rowKey, rowKey); + } + + @Override + public Object get(final long rowKey) { + return TypeUtils.box(getBoolean(rowKey)); + } + + @Override + public Object getPrev(final long rowKey) { + return TypeUtils.box(getPrevBoolean(rowKey)); + } + + @Override + public ChunkType getChunkType() { + return ChunkType.Object; + } + + @Override + public FillContext makeFillContext(final int chunkCapacity) { + return FILL_CONTEXT_INSTANCE; + } + + @Override + public void fillChunk( + @NotNull final FillContext fillContext, + @NotNull final WritableChunk destination, + @NotNull final RowSequence rowSequence) { + doFill(rowSequence, destination, false); + } + + private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, boolean usePrev) { + final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); + try (final RowSet inputRowSet = rowSequence.asRowSet(); + final RowSet filtered = filter.filter(inputRowSet, rowSet, tableToFilter, usePrev); + final RowSet.Iterator inputIt = inputRowSet.iterator(); + final RowSet.Iterator trueIt = filtered.iterator()) { + long nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; + int offset = 0; + while (nextTrue >= 0 && inputIt.hasNext()) { + final long nextInput = inputIt.nextLong(); + final boolean found = nextInput == nextTrue; + booleanDestination.set(offset++, found); + if (found) { + nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; + } + } + // fill everything else up with false, because nothing else can match + booleanDestination.fillWithBoxedValue(offset, booleanDestination.size() - offset, false); + } + } + + @Override + public void fillPrevChunk( + @NotNull final FillContext fillContext, + @NotNull final WritableChunk destination, + @NotNull final RowSequence rowSequence) { + doFill(rowSequence, destination, true); + } + }, false); + } + + @NotNull + @Override + public ColumnSource getLazyView() { + return getDataView(); + } + + @Override + public String getName() { + return destName; + } + + @Override + public MatchPair getMatchPair() { + throw new UnsupportedOperationException(); + } + + @Override + public final WritableColumnSource newDestInstance(final long size) { + return SparseArrayColumnSource.getSparseMemoryColumnSource(size, Boolean.class); + } + + @Override + public final WritableColumnSource newFlatDestInstance(final long size) { + return InMemoryColumnSource.getImmutableMemoryColumnSource(size, Boolean.class, null); + } + + @Override + public boolean isRetain() { + return false; + } + + @Override + public boolean isStateless() { + return false; + } + + @Override + public FilterSelectColumn copy() { + return new FilterSelectColumn<>(destName, filter.copy()); + } +} \ No newline at end of file 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..9c3d9fdd66f 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 @@ -251,7 +251,7 @@ public SelectColumn visit(Literal rhs) { @Override public SelectColumn visit(Filter rhs) { - return makeSelectColumn(Strings.of(rhs)); + return new FilterSelectColumn<>(lhs.name(), WhereFilter.of(rhs)); } @Override diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilter.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilter.java index 9dae903ae4e..454cc547020 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilter.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilter.java @@ -301,6 +301,13 @@ default boolean canMemoize() { return false; } + /** + * Returns true if this filter uses row virtual offset columns of {@code i}, {@code ii} or {@code k}. + */ + default boolean hasVirtualRowVariables() { + return false; + } + /** * Create a copy of this WhereFilter. * @@ -331,7 +338,7 @@ default Filter invert() { @Override default T walk(Expression.Visitor visitor) { - throw new UnsupportedOperationException("WhereFilters do not implement walk"); + return visitor.visit(this); } @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..8f3033f56a7 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 @@ -3,12 +3,18 @@ // package io.deephaven.engine.table.impl; +import io.deephaven.api.ColumnName; import io.deephaven.api.JoinMatch; +import io.deephaven.api.Selectable; import io.deephaven.api.TableOperations; +import io.deephaven.api.filter.Filter; +import io.deephaven.api.filter.FilterIn; +import io.deephaven.api.literal.Literal; import io.deephaven.base.testing.BaseArrayTestCase; import io.deephaven.configuration.Configuration; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; +import io.deephaven.engine.exceptions.UncheckedTableException; import io.deephaven.engine.liveness.LivenessScope; import io.deephaven.engine.liveness.LivenessScopeStack; import io.deephaven.engine.rowset.RowSet; @@ -18,8 +24,10 @@ import io.deephaven.engine.table.ColumnSource; import io.deephaven.engine.table.ShiftObliviousListener; import io.deephaven.engine.table.Table; +import io.deephaven.engine.table.WouldMatchPair; import io.deephaven.engine.table.impl.select.DhFormulaColumn; import io.deephaven.engine.table.impl.select.FormulaCompilationException; +import io.deephaven.engine.table.impl.select.WhereFilterFactory; import io.deephaven.engine.table.impl.sources.InMemoryColumnSource; import io.deephaven.engine.table.impl.sources.LongSparseArraySource; import io.deephaven.engine.table.impl.sources.RedirectedColumnSource; @@ -1300,4 +1308,98 @@ public void testPropagationOfAttributes() { Assert.assertTrue(result.isBlink()); } } + + @Test + public void testFilterExpression() { + final Filter filter = FilterIn.of(ColumnName.of("A"), Literal.of(1), Literal.of(3)); + final Table t = TableTools.newTable(intCol("A", 1, 1, 2, 3, 5, 8)); + final Table wm = t.wouldMatch(new WouldMatchPair("AWM", filter)); + TableTools.showWithRowSet(wm); + + // use an update + final Table up = t.update(List.of(Selectable.of(ColumnName.of("AWM"), filter))); + TableTools.showWithRowSet(up); + + assertTableEquals(wm, up); + + // use an updateView + final Table upv = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter))); + TableTools.showWithRowSet(upv); + + assertTableEquals(wm, upv); + + // and now a more generic WhereFilter + + final Filter filter2 = WhereFilterFactory.getExpression("A == 1 || A==3"); + final Table wm2 = t.wouldMatch(new WouldMatchPair("AWM", filter2)); + TableTools.showWithRowSet(wm2); + + // use an update + final Table up2 = t.update(List.of(Selectable.of(ColumnName.of("AWM"), filter2))); + TableTools.showWithRowSet(up); + + assertTableEquals(wm2, up2); + } + + @Test + public void testFilterExpressionArray() { + final Filter filter = WhereFilterFactory.getExpression("A=A_[i-1]"); + final Filter filterArrayOnly = WhereFilterFactory.getExpression("A=A_.size() = 1"); + final Filter filterKonly = WhereFilterFactory.getExpression("A=k+1"); + final QueryTable table = TstUtils.testRefreshingTable(intCol("A", 1, 1, 2, 3, 5, 8, 9, 9)); + + final UncheckedTableException wme = Assert.assertThrows(UncheckedTableException.class, () -> table.wouldMatch(new WouldMatchPair("AWM", filter))); + Assert.assertEquals("wouldMatch filters cannot use virtual row variables (i, ii, and k): [A=A_[i-1]]", wme.getMessage()); + + final UncheckedTableException wme2 = Assert.assertThrows(UncheckedTableException.class, () -> table.wouldMatch(new WouldMatchPair("AWM", filterArrayOnly))); + Assert.assertEquals("wouldMatch filters cannot use column Vectors (_ syntax): [A=A_.size() = 1]", wme2.getMessage()); + + final UncheckedTableException upe = Assert.assertThrows(UncheckedTableException.class, () -> table.update(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); + Assert.assertEquals("Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", upe.getMessage()); + + final UncheckedTableException uve = Assert.assertThrows(UncheckedTableException.class, () -> table.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); + Assert.assertEquals("Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", upe.getMessage()); + + final UncheckedTableException se = Assert.assertThrows(UncheckedTableException.class, () -> table.select(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); + Assert.assertEquals("Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", se.getMessage()); + + final UncheckedTableException ve = Assert.assertThrows(UncheckedTableException.class, () -> table.view(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); + Assert.assertEquals("Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", ve.getMessage()); + + } + + @Test + public void testFilterExpressionTicking() { + for (int seed = 0; seed < 5; ++seed) { + testFilterExpressionTicking(seed, new MutableInt(100)); + } + } + + private void testFilterExpressionTicking(final int seed, final MutableInt numSteps) { + final Random random = new Random(seed); + final ColumnInfo[] columnInfo; + final int size = 25; + final QueryTable queryTable = getTable(size, random, + columnInfo = initColumnInfos(new String[] {"Sym", "intCol", "doubleCol"}, + new SetGenerator<>("a", "b", "c", "d", "e"), + new IntGenerator(10, 100), + new SetGenerator<>(10.1, 20.1, 30.1))); + + final EvalNuggetInterface[] en = new EvalNuggetInterface[] { + new TableComparator(queryTable.wouldMatch("SM=Sym in `b`, `d`"), queryTable.update(List.of(Selectable.of(ColumnName.of("SM"), WhereFilterFactory.getExpression("Sym in `b`, `d`"))))), + new TableComparator(queryTable.wouldMatch("SM=Sym in `b`, `d`"), queryTable.updateView(List.of(Selectable.of(ColumnName.of("SM"), WhereFilterFactory.getExpression("Sym in `b`, `d`"))))), + new TableComparator(queryTable.wouldMatch("IM=intCol < 50"), queryTable.update(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("intCol < 50"))))), + new TableComparator(queryTable.wouldMatch("IM=intCol < 50"), queryTable.updateView(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("intCol < 50"))))), + new TableComparator(queryTable.wouldMatch("IM=Sym= (intCol%2 == 0? `a` : `b`)"), queryTable.update(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("Sym= (intCol%2 == 0? `a` : `b`)"))))), + new TableComparator(queryTable.wouldMatch("IM=Sym= (intCol%2 == 0? `a` : `b`)"), queryTable.updateView(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("Sym= (intCol%2 == 0? `a` : `b`)"))))), + }; + + final int maxSteps = numSteps.get(); + for (numSteps.set(0); numSteps.get() < maxSteps; numSteps.increment()) { + if (RefreshingTableTestCase.printTableUpdates) { + System.out.println("Step = " + numSteps.get()); + } + RefreshingTableTestCase.simulateShiftAwareStep(size, random, queryTable, columnInfo, en); + } + } } From aff0046dd6c7c779ae529bdf6655c03ad9ba3f05 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 7 Nov 2024 09:32:49 -0500 Subject: [PATCH 02/18] spotless --- .../table/impl/WouldMatchOperation.java | 15 ++-- .../table/impl/select/FilterSelectColumn.java | 34 +++++---- .../impl/QueryTableSelectUpdateTest.java | 74 +++++++++++++------ 3 files changed, 82 insertions(+), 41 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java index fef43eed902..91dbc7e323b 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/WouldMatchOperation.java @@ -114,14 +114,18 @@ void initializeFilters(@NotNull QueryTable parent) { final QueryCompilerRequestProcessor.BatchProcessor compilationProcessor = QueryCompilerRequestProcessor.batch(); Arrays.stream(whereFilters).forEach(filter -> filter.init(parent.getDefinition(), compilationProcessor)); - final List disallowedRowVariables = Arrays.stream(whereFilters).filter(WhereFilter::hasVirtualRowVariables).collect(Collectors.toList()); + final List disallowedRowVariables = + Arrays.stream(whereFilters).filter(WhereFilter::hasVirtualRowVariables).collect(Collectors.toList()); if (!disallowedRowVariables.isEmpty()) { - throw new UncheckedTableException("wouldMatch filters cannot use virtual row variables (i, ii, and k): " + disallowedRowVariables); + throw new UncheckedTableException( + "wouldMatch filters cannot use virtual row variables (i, ii, and k): " + disallowedRowVariables); } - final List disallowedColumnVectors = Arrays.stream(whereFilters).filter(wf -> !wf.getColumnArrays().isEmpty()).collect(Collectors.toList()); + final List disallowedColumnVectors = + Arrays.stream(whereFilters).filter(wf -> !wf.getColumnArrays().isEmpty()).collect(Collectors.toList()); if (!disallowedColumnVectors.isEmpty()) { - throw new UncheckedTableException("wouldMatch filters cannot use column Vectors (_ syntax): " + disallowedColumnVectors); + throw new UncheckedTableException( + "wouldMatch filters cannot use column Vectors (_ syntax): " + disallowedColumnVectors); } compilationProcessor.compile(); @@ -534,7 +538,8 @@ private RowSet update(RowSet added, RowSet removed, RowSet modified, // If we were affected, recompute mods and re-add the ones that pass. if (affected) { downstreamModified.setAll(name); - final RowSet filteredModified = toClose.add(filter.filter(modified, table.getRowSet(), table, false)); + final RowSet filteredModified = + toClose.add(filter.filter(modified, table.getRowSet(), table, false)); // Now apply the additions and remove any non-matching modifieds filteredAdded.insert(filteredModified); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index a05cdde430f..77c0f435e19 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -30,11 +30,12 @@ public class FilterSelectColumn implements SelectColumn { // We don't actually care to do anything, but cannot return null - private static final Formula.FillContext FILL_CONTEXT_INSTANCE = new Formula.FillContext() { - }; + private static final Formula.FillContext FILL_CONTEXT_INSTANCE = new Formula.FillContext() {}; - @NotNull private final String destName; - @NotNull private final WhereFilter filter; + @NotNull + private final String destName; + @NotNull + private final WhereFilter filter; private RowSet rowSet; private Table tableToFilter; @@ -67,10 +68,14 @@ public List initDef(@NotNull final Map> colu filter.init(fromColumns); if (!filter.getColumnArrays().isEmpty()) { - throw new UncheckedTableException("Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: " + filter); + throw new UncheckedTableException( + "Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: " + + filter); } if (filter.hasVirtualRowVariables()) { - throw new UncheckedTableException("Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: " + filter); + throw new UncheckedTableException( + "Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: " + + filter); } return filter.getColumns(); @@ -108,12 +113,14 @@ public ColumnSource getDataView() { @Override public Boolean getBoolean(long rowKey) { - return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, false).containsRange(rowKey, rowKey); + return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, false).containsRange(rowKey, + rowKey); } @Override public Boolean getPrevBoolean(long rowKey) { - return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, true).containsRange(rowKey, rowKey); + return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, true).containsRange(rowKey, + rowKey); } @Override @@ -144,12 +151,13 @@ public void fillChunk( doFill(rowSequence, destination, false); } - private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, boolean usePrev) { + private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, + boolean usePrev) { final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); try (final RowSet inputRowSet = rowSequence.asRowSet(); - final RowSet filtered = filter.filter(inputRowSet, rowSet, tableToFilter, usePrev); - final RowSet.Iterator inputIt = inputRowSet.iterator(); - final RowSet.Iterator trueIt = filtered.iterator()) { + final RowSet filtered = filter.filter(inputRowSet, rowSet, tableToFilter, usePrev); + final RowSet.Iterator inputIt = inputRowSet.iterator(); + final RowSet.Iterator trueIt = filtered.iterator()) { long nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; int offset = 0; while (nextTrue >= 0 && inputIt.hasNext()) { @@ -215,4 +223,4 @@ public boolean isStateless() { public FilterSelectColumn copy() { return new FilterSelectColumn<>(destName, filter.copy()); } -} \ No newline at end of file +} 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 8f3033f56a7..8d9152b7c62 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 @@ -1348,23 +1348,39 @@ public void testFilterExpressionArray() { final Filter filterKonly = WhereFilterFactory.getExpression("A=k+1"); final QueryTable table = TstUtils.testRefreshingTable(intCol("A", 1, 1, 2, 3, 5, 8, 9, 9)); - final UncheckedTableException wme = Assert.assertThrows(UncheckedTableException.class, () -> table.wouldMatch(new WouldMatchPair("AWM", filter))); - Assert.assertEquals("wouldMatch filters cannot use virtual row variables (i, ii, and k): [A=A_[i-1]]", wme.getMessage()); - - final UncheckedTableException wme2 = Assert.assertThrows(UncheckedTableException.class, () -> table.wouldMatch(new WouldMatchPair("AWM", filterArrayOnly))); - Assert.assertEquals("wouldMatch filters cannot use column Vectors (_ syntax): [A=A_.size() = 1]", wme2.getMessage()); - - final UncheckedTableException upe = Assert.assertThrows(UncheckedTableException.class, () -> table.update(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); - Assert.assertEquals("Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", upe.getMessage()); - - final UncheckedTableException uve = Assert.assertThrows(UncheckedTableException.class, () -> table.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); - Assert.assertEquals("Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", upe.getMessage()); - - final UncheckedTableException se = Assert.assertThrows(UncheckedTableException.class, () -> table.select(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); - Assert.assertEquals("Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", se.getMessage()); - - final UncheckedTableException ve = Assert.assertThrows(UncheckedTableException.class, () -> table.view(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); - Assert.assertEquals("Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", ve.getMessage()); + final UncheckedTableException wme = Assert.assertThrows(UncheckedTableException.class, + () -> table.wouldMatch(new WouldMatchPair("AWM", filter))); + Assert.assertEquals("wouldMatch filters cannot use virtual row variables (i, ii, and k): [A=A_[i-1]]", + wme.getMessage()); + + final UncheckedTableException wme2 = Assert.assertThrows(UncheckedTableException.class, + () -> table.wouldMatch(new WouldMatchPair("AWM", filterArrayOnly))); + Assert.assertEquals("wouldMatch filters cannot use column Vectors (_ syntax): [A=A_.size() = 1]", + wme2.getMessage()); + + final UncheckedTableException upe = Assert.assertThrows(UncheckedTableException.class, + () -> table.update(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); + Assert.assertEquals( + "Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", + upe.getMessage()); + + final UncheckedTableException uve = Assert.assertThrows(UncheckedTableException.class, + () -> table.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); + Assert.assertEquals( + "Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", + upe.getMessage()); + + final UncheckedTableException se = Assert.assertThrows(UncheckedTableException.class, + () -> table.select(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); + Assert.assertEquals( + "Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", + se.getMessage()); + + final UncheckedTableException ve = Assert.assertThrows(UncheckedTableException.class, + () -> table.view(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); + Assert.assertEquals( + "Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", + ve.getMessage()); } @@ -1386,12 +1402,24 @@ private void testFilterExpressionTicking(final int seed, final MutableInt numSte new SetGenerator<>(10.1, 20.1, 30.1))); final EvalNuggetInterface[] en = new EvalNuggetInterface[] { - new TableComparator(queryTable.wouldMatch("SM=Sym in `b`, `d`"), queryTable.update(List.of(Selectable.of(ColumnName.of("SM"), WhereFilterFactory.getExpression("Sym in `b`, `d`"))))), - new TableComparator(queryTable.wouldMatch("SM=Sym in `b`, `d`"), queryTable.updateView(List.of(Selectable.of(ColumnName.of("SM"), WhereFilterFactory.getExpression("Sym in `b`, `d`"))))), - new TableComparator(queryTable.wouldMatch("IM=intCol < 50"), queryTable.update(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("intCol < 50"))))), - new TableComparator(queryTable.wouldMatch("IM=intCol < 50"), queryTable.updateView(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("intCol < 50"))))), - new TableComparator(queryTable.wouldMatch("IM=Sym= (intCol%2 == 0? `a` : `b`)"), queryTable.update(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("Sym= (intCol%2 == 0? `a` : `b`)"))))), - new TableComparator(queryTable.wouldMatch("IM=Sym= (intCol%2 == 0? `a` : `b`)"), queryTable.updateView(List.of(Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("Sym= (intCol%2 == 0? `a` : `b`)"))))), + new TableComparator(queryTable.wouldMatch("SM=Sym in `b`, `d`"), + queryTable.update(List.of(Selectable.of(ColumnName.of("SM"), + WhereFilterFactory.getExpression("Sym in `b`, `d`"))))), + new TableComparator(queryTable.wouldMatch("SM=Sym in `b`, `d`"), + queryTable.updateView(List.of(Selectable.of(ColumnName.of("SM"), + WhereFilterFactory.getExpression("Sym in `b`, `d`"))))), + new TableComparator(queryTable.wouldMatch("IM=intCol < 50"), + queryTable.update(List.of( + Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("intCol < 50"))))), + new TableComparator(queryTable.wouldMatch("IM=intCol < 50"), + queryTable.updateView(List.of( + Selectable.of(ColumnName.of("IM"), WhereFilterFactory.getExpression("intCol < 50"))))), + new TableComparator(queryTable.wouldMatch("IM=Sym= (intCol%2 == 0? `a` : `b`)"), + queryTable.update(List.of(Selectable.of(ColumnName.of("IM"), + WhereFilterFactory.getExpression("Sym= (intCol%2 == 0? `a` : `b`)"))))), + new TableComparator(queryTable.wouldMatch("IM=Sym= (intCol%2 == 0? `a` : `b`)"), + queryTable.updateView(List.of(Selectable.of(ColumnName.of("IM"), + WhereFilterFactory.getExpression("Sym= (intCol%2 == 0? `a` : `b`)"))))), }; final int maxSteps = numSteps.get(); From 0536d38abfecc1ec2be76840ed880b267b0d8039 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 13 Nov 2024 09:44:00 -0500 Subject: [PATCH 03/18] javadoc, remove useless type parameter. --- .../table/impl/select/FilterSelectColumn.java | 30 ++++++++++++++----- .../table/impl/select/SelectColumn.java | 2 +- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 77c0f435e19..f5f09571aaf 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -14,8 +14,8 @@ import io.deephaven.engine.rowset.TrackingRowSet; import io.deephaven.engine.table.*; import io.deephaven.engine.table.impl.MatchPair; +import io.deephaven.engine.table.impl.QueryCompilerRequestProcessor; import io.deephaven.engine.table.impl.QueryTable; -import io.deephaven.engine.table.impl.chunkfillers.ChunkFiller; import io.deephaven.engine.table.impl.sources.InMemoryColumnSource; import io.deephaven.engine.table.impl.sources.SparseArrayColumnSource; import io.deephaven.engine.table.impl.sources.ViewColumnSource; @@ -24,10 +24,21 @@ import org.jetbrains.annotations.NotNull; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; -public class FilterSelectColumn implements SelectColumn { +/** + * The FilterSelectColumn wraps a {@link io.deephaven.api.filter.Filter} and producing a column of + * true or false Boolean values described by the filter. + * + *

This column is appropriate as an argument to a {@link Table#view(Collection)} or + * {@link Table#updateView(String...)}, lazily evaluating the equivalent of a {@link Table#wouldMatch(String...)} + * operation. Although select and update can also use Filters, the wouldMatch provides a more efficient path + * for realized results as it stores and updates only a {@link RowSet}. The FilterSelectColumn can only evaluate + * the filter one chunk at a time, and must write to an in-memory Boolean column source.

+ */ +class FilterSelectColumn implements SelectColumn { // We don't actually care to do anything, but cannot return null private static final Formula.FillContext FILL_CONTEXT_INSTANCE = new Formula.FillContext() {}; @@ -58,14 +69,19 @@ public List initInputs(TrackingRowSet rowSet, Map initDef(@NotNull final Map> columnDefinitionMap) { + public List initDef(@NotNull Map> columnDefinitionMap) { + return initDef(columnDefinitionMap, QueryCompilerRequestProcessor.immediate()); + } + + @Override + public List initDef(@NotNull final Map> columnDefinitionMap, + @NotNull final QueryCompilerRequestProcessor compilationRequestProcessor) { final List> columnHeaders = new ArrayList<>(); for (Map.Entry> entry : columnDefinitionMap.entrySet()) { columnHeaders.add(ColumnHeader.of(entry.getKey(), entry.getValue().getDataType())); } final TableDefinition fromColumns = TableDefinition.from(columnHeaders); - // TODO: Compilation processor! - filter.init(fromColumns); + filter.init(fromColumns, compilationRequestProcessor); if (!filter.getColumnArrays().isEmpty()) { throw new UncheckedTableException( @@ -220,7 +236,7 @@ public boolean isStateless() { } @Override - public FilterSelectColumn copy() { - return new FilterSelectColumn<>(destName, filter.copy()); + public FilterSelectColumn copy() { + return new FilterSelectColumn(destName, filter.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 e3049beab94..f5b5ec4b9e7 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 @@ -278,7 +278,7 @@ public SelectColumn visit(Literal rhs) { @Override public SelectColumn visit(Filter rhs) { - return new FilterSelectColumn<>(lhs.name(), WhereFilter.of(rhs)); + return new FilterSelectColumn(lhs.name(), WhereFilter.of(rhs)); } @Override From 2821da33d83e758d4a31b9d406b91ff65b77e0ad Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 13 Nov 2024 09:52:45 -0500 Subject: [PATCH 04/18] cleanup. --- .../table/impl/select/FilterSelectColumn.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index f5f09571aaf..d399633b48e 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -141,12 +141,12 @@ public Boolean getPrevBoolean(long rowKey) { @Override public Object get(final long rowKey) { - return TypeUtils.box(getBoolean(rowKey)); + return getBoolean(rowKey); } @Override public Object getPrev(final long rowKey) { - return TypeUtils.box(getPrevBoolean(rowKey)); + return getPrevBoolean(rowKey); } @Override @@ -167,6 +167,14 @@ public void fillChunk( doFill(rowSequence, destination, false); } + @Override + public void fillPrevChunk( + @NotNull final FillContext fillContext, + @NotNull final WritableChunk destination, + @NotNull final RowSequence rowSequence) { + doFill(rowSequence, destination, true); + } + private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, boolean usePrev) { final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); @@ -188,14 +196,6 @@ private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, - @NotNull final RowSequence rowSequence) { - doFill(rowSequence, destination, true); - } }, false); } From e951adff931b26006426d34cb70b739da6b373b5 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 13 Nov 2024 10:00:50 -0500 Subject: [PATCH 05/18] more coverage and remove impossible branch. --- .../table/impl/select/FilterSelectColumn.java | 4 +++- .../table/impl/QueryTableSelectUpdateTest.java | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index d399633b48e..c10e23a6a2a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -184,7 +184,9 @@ private void doFill(@NotNull RowSequence rowSequence, WritableChunk= 0 && inputIt.hasNext()) { + while (nextTrue >= 0) { + // the input iterator is a superset of the true iterator, so we can always find out what + // the next value is without needing to check hasNext final long nextInput = inputIt.nextLong(); final boolean found = nextInput == nextTrue; booleanDestination.set(offset++, found); 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 0406bd7ea6f..06eea59e0c3 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 @@ -1340,6 +1340,20 @@ public void testFilterExpression() { TableTools.showWithRowSet(up); assertTableEquals(wm2, up2); + + // a Filter where nothing is true, to check that state + final Table upvf = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), Filter.ofFalse()))); + assertTableEquals(t.updateView("AWM=false"), upvf); + + // a Filter where everything is true + final Table upvt = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), Filter.ofTrue()))); + assertTableEquals(t.updateView("AWM=true"), upvt); + + // a Filter where the last value in the chunk is true + final Filter filter3 = WhereFilterFactory.getExpression("A in 8"); + final Table wm3 = t.wouldMatch(new WouldMatchPair("AWM", filter3)); + final Table upv3 = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter3))); + assertTableEquals(wm3, upv3); } @Test From 1d1fa931f3ae48822da1e6ec7904d01f98f5d8bf Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 13 Nov 2024 10:08:29 -0500 Subject: [PATCH 06/18] spotless --- .../table/impl/select/FilterSelectColumn.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index c10e23a6a2a..2bc43989367 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -29,14 +29,16 @@ import java.util.Map; /** - * The FilterSelectColumn wraps a {@link io.deephaven.api.filter.Filter} and producing a column of - * true or false Boolean values described by the filter. + * The FilterSelectColumn wraps a {@link io.deephaven.api.filter.Filter} and producing a column of true or false Boolean + * values described by the filter. * - *

This column is appropriate as an argument to a {@link Table#view(Collection)} or - * {@link Table#updateView(String...)}, lazily evaluating the equivalent of a {@link Table#wouldMatch(String...)} - * operation. Although select and update can also use Filters, the wouldMatch provides a more efficient path - * for realized results as it stores and updates only a {@link RowSet}. The FilterSelectColumn can only evaluate - * the filter one chunk at a time, and must write to an in-memory Boolean column source.

+ *

+ * This column is appropriate as an argument to a {@link Table#view(Collection)} or {@link Table#updateView(String...)}, + * lazily evaluating the equivalent of a {@link Table#wouldMatch(String...)} operation. Although select and update can + * also use Filters, the wouldMatch provides a more efficient path for realized results as it stores and updates only a + * {@link RowSet}. The FilterSelectColumn can only evaluate the filter one chunk at a time, and must write to an + * in-memory Boolean column source. + *

*/ class FilterSelectColumn implements SelectColumn { From 7cb94578b82970b8d0e9ff4035e9ae5b74584261 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 13 Nov 2024 22:01:35 -0500 Subject: [PATCH 07/18] cr1 --- .../table/impl/select/FilterSelectColumn.java | 193 ++++++++++-------- .../table/impl/select/SelectColumn.java | 2 +- .../impl/QueryTableSelectUpdateTest.java | 9 - 3 files changed, 107 insertions(+), 97 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 2bc43989367..820b04e2717 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -8,10 +8,7 @@ import io.deephaven.chunk.WritableObjectChunk; import io.deephaven.chunk.attributes.Values; import io.deephaven.engine.exceptions.UncheckedTableException; -import io.deephaven.engine.rowset.RowSequence; -import io.deephaven.engine.rowset.RowSet; -import io.deephaven.engine.rowset.RowSetFactory; -import io.deephaven.engine.rowset.TrackingRowSet; +import io.deephaven.engine.rowset.*; import io.deephaven.engine.table.*; import io.deephaven.engine.table.impl.MatchPair; import io.deephaven.engine.table.impl.QueryCompilerRequestProcessor; @@ -20,7 +17,6 @@ import io.deephaven.engine.table.impl.sources.SparseArrayColumnSource; import io.deephaven.engine.table.impl.sources.ViewColumnSource; import io.deephaven.qst.column.header.ColumnHeader; -import io.deephaven.util.type.TypeUtils; import org.jetbrains.annotations.NotNull; import java.util.ArrayList; @@ -53,7 +49,18 @@ class FilterSelectColumn implements SelectColumn { private RowSet rowSet; private Table tableToFilter; - public FilterSelectColumn(@NotNull final String destName, @NotNull final WhereFilter filter) { + /** + * Create a FilterSelectColumn with the given name and {@link WhereFilter}. + * + * @param destName the name of the result column + * @param filter the filter that is evaluated to true or false for each row of the table + * @return a new FilterSelectColumn representing the provided filter. + */ + static FilterSelectColumn of(@NotNull final String destName, @NotNull final WhereFilter filter) { + return new FilterSelectColumn(destName, filter); + } + + private FilterSelectColumn(@NotNull final String destName, @NotNull final WhereFilter filter) { this.destName = destName; this.filter = filter; } @@ -72,19 +79,24 @@ public List initInputs(TrackingRowSet rowSet, Map initDef(@NotNull Map> columnDefinitionMap) { - return initDef(columnDefinitionMap, QueryCompilerRequestProcessor.immediate()); + filter.init(TableDefinition.of(columnDefinitionMap.values())); + return checkForInvalidFilters(); } @Override public List initDef(@NotNull final Map> columnDefinitionMap, @NotNull final QueryCompilerRequestProcessor compilationRequestProcessor) { - final List> columnHeaders = new ArrayList<>(); - for (Map.Entry> entry : columnDefinitionMap.entrySet()) { - columnHeaders.add(ColumnHeader.of(entry.getKey(), entry.getValue().getDataType())); - } - final TableDefinition fromColumns = TableDefinition.from(columnHeaders); - filter.init(fromColumns, compilationRequestProcessor); + filter.init(TableDefinition.of(columnDefinitionMap.values()), compilationRequestProcessor); + return checkForInvalidFilters(); + } + /** + * Validates the filter to ensure it does not contain invalid filters such as column vectors or virtual row + * variables. Throws an {@link UncheckedTableException} if any invalid filters are found. + * + * @return the list of columns required by the filter. + */ + private List checkForInvalidFilters() { if (!filter.getColumnArrays().isEmpty()) { throw new UncheckedTableException( "Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: " @@ -121,86 +133,14 @@ public List getColumnArrays() { @Override public boolean hasVirtualRowVariables() { + /* This should always be false, because initDef throws when arrays or ii and friends are used. */ return filter.hasVirtualRowVariables(); } @NotNull @Override public ColumnSource getDataView() { - return new ViewColumnSource<>(Boolean.class, new Formula(null) { - - @Override - public Boolean getBoolean(long rowKey) { - return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, false).containsRange(rowKey, - rowKey); - } - - @Override - public Boolean getPrevBoolean(long rowKey) { - return filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, true).containsRange(rowKey, - rowKey); - } - - @Override - public Object get(final long rowKey) { - return getBoolean(rowKey); - } - - @Override - public Object getPrev(final long rowKey) { - return getPrevBoolean(rowKey); - } - - @Override - public ChunkType getChunkType() { - return ChunkType.Object; - } - - @Override - public FillContext makeFillContext(final int chunkCapacity) { - return FILL_CONTEXT_INSTANCE; - } - - @Override - public void fillChunk( - @NotNull final FillContext fillContext, - @NotNull final WritableChunk destination, - @NotNull final RowSequence rowSequence) { - doFill(rowSequence, destination, false); - } - - @Override - public void fillPrevChunk( - @NotNull final FillContext fillContext, - @NotNull final WritableChunk destination, - @NotNull final RowSequence rowSequence) { - doFill(rowSequence, destination, true); - } - - private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, - boolean usePrev) { - final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); - try (final RowSet inputRowSet = rowSequence.asRowSet(); - final RowSet filtered = filter.filter(inputRowSet, rowSet, tableToFilter, usePrev); - final RowSet.Iterator inputIt = inputRowSet.iterator(); - final RowSet.Iterator trueIt = filtered.iterator()) { - long nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; - int offset = 0; - while (nextTrue >= 0) { - // the input iterator is a superset of the true iterator, so we can always find out what - // the next value is without needing to check hasNext - final long nextInput = inputIt.nextLong(); - final boolean found = nextInput == nextTrue; - booleanDestination.set(offset++, found); - if (found) { - nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; - } - } - // fill everything else up with false, because nothing else can match - booleanDestination.fillWithBoxedValue(offset, booleanDestination.size() - offset, false); - } - } - }, false); + return new ViewColumnSource<>(Boolean.class, new FilterFormula(), false); } @NotNull @@ -243,4 +183,83 @@ public boolean isStateless() { public FilterSelectColumn copy() { return new FilterSelectColumn(destName, filter.copy()); } + + private class FilterFormula extends Formula { + public FilterFormula() { + super(null); + } + + @Override + public Boolean getBoolean(long rowKey) { + WritableRowSet filteredIndex = filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, false); + return filteredIndex.isNonempty(); + } + + @Override + public Boolean getPrevBoolean(long rowKey) { + WritableRowSet filteredIndex = filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, true); + return filteredIndex.isNonempty(); + } + + @Override + public Object get(final long rowKey) { + return getBoolean(rowKey); + } + + @Override + public Object getPrev(final long rowKey) { + return getPrevBoolean(rowKey); + } + + @Override + public ChunkType getChunkType() { + return ChunkType.Object; + } + + @Override + public FillContext makeFillContext(final int chunkCapacity) { + return FILL_CONTEXT_INSTANCE; + } + + @Override + public void fillChunk( + @NotNull final FillContext fillContext, + @NotNull final WritableChunk destination, + @NotNull final RowSequence rowSequence) { + doFill(rowSequence, destination, false); + } + + @Override + public void fillPrevChunk( + @NotNull final FillContext fillContext, + @NotNull final WritableChunk destination, + @NotNull final RowSequence rowSequence) { + doFill(rowSequence, destination, true); + } + + private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, + boolean usePrev) { + final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); + booleanDestination.setSize(rowSequence.intSize()); + try (final RowSet inputRowSet = rowSequence.asRowSet(); + final RowSet filtered = filter.filter(inputRowSet, rowSet, tableToFilter, usePrev); + final RowSet.Iterator inputIt = inputRowSet.iterator(); + final RowSet.Iterator trueIt = filtered.iterator()) { + long nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; + int offset = 0; + while (nextTrue >= 0) { + // the input iterator is a superset of the true iterator, so we can always find out what + // the next value is without needing to check hasNext + final long nextInput = inputIt.nextLong(); + final boolean found = nextInput == nextTrue; + booleanDestination.set(offset++, found); + if (found) { + nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; + } + } + // fill everything else up with false, because nothing else can match + booleanDestination.fillWithBoxedValue(offset, booleanDestination.size() - offset, false); + } + } + } } 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 f5b5ec4b9e7..65817eedb8b 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 @@ -278,7 +278,7 @@ public SelectColumn visit(Literal rhs) { @Override public SelectColumn visit(Filter rhs) { - return new FilterSelectColumn(lhs.name(), WhereFilter.of(rhs)); + return FilterSelectColumn.of(lhs.name(), WhereFilter.of(rhs)); } @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 06eea59e0c3..e74e6584332 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 @@ -1315,31 +1315,22 @@ public void testFilterExpression() { final Filter filter = FilterIn.of(ColumnName.of("A"), Literal.of(1), Literal.of(3)); final Table t = TableTools.newTable(intCol("A", 1, 1, 2, 3, 5, 8)); final Table wm = t.wouldMatch(new WouldMatchPair("AWM", filter)); - TableTools.showWithRowSet(wm); // use an update final Table up = t.update(List.of(Selectable.of(ColumnName.of("AWM"), filter))); - TableTools.showWithRowSet(up); - assertTableEquals(wm, up); // use an updateView final Table upv = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter))); - TableTools.showWithRowSet(upv); - assertTableEquals(wm, upv); // and now a more generic WhereFilter final Filter filter2 = WhereFilterFactory.getExpression("A == 1 || A==3"); final Table wm2 = t.wouldMatch(new WouldMatchPair("AWM", filter2)); - TableTools.showWithRowSet(wm2); // use an update final Table up2 = t.update(List.of(Selectable.of(ColumnName.of("AWM"), filter2))); - TableTools.showWithRowSet(up); - - assertTableEquals(wm2, up2); // a Filter where nothing is true, to check that state final Table upvf = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), Filter.ofFalse()))); From a2b799b94b420d780f4690ec7321cb47b7ef85e6 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 14 Nov 2024 05:55:18 -0500 Subject: [PATCH 08/18] cr2. --- .../engine/table/impl/select/FilterSelectColumn.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 820b04e2717..cd407809b2f 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -10,16 +10,15 @@ import io.deephaven.engine.exceptions.UncheckedTableException; import io.deephaven.engine.rowset.*; import io.deephaven.engine.table.*; +import io.deephaven.engine.table.impl.BaseTable; import io.deephaven.engine.table.impl.MatchPair; import io.deephaven.engine.table.impl.QueryCompilerRequestProcessor; import io.deephaven.engine.table.impl.QueryTable; import io.deephaven.engine.table.impl.sources.InMemoryColumnSource; import io.deephaven.engine.table.impl.sources.SparseArrayColumnSource; import io.deephaven.engine.table.impl.sources.ViewColumnSource; -import io.deephaven.qst.column.header.ColumnHeader; import org.jetbrains.annotations.NotNull; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; @@ -176,7 +175,7 @@ public boolean isRetain() { @Override public boolean isStateless() { - return false; + return filter.permitParallelization(); } @Override @@ -184,6 +183,11 @@ public FilterSelectColumn copy() { return new FilterSelectColumn(destName, filter.copy()); } + @Override + public void validateSafeForRefresh(BaseTable sourceTable) { + filter.validateSafeForRefresh(sourceTable); + } + private class FilterFormula extends Formula { public FilterFormula() { super(null); From 263bc021b2951f102f1505383ade0f9fbe85c020 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 14 Nov 2024 11:01:46 -0500 Subject: [PATCH 09/18] FilterSelectColumn to take Filter. --- .../engine/table/impl/select/FilterSelectColumn.java | 5 +++-- .../io/deephaven/engine/table/impl/select/SelectColumn.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index cd407809b2f..efc8e26b6f6 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -3,6 +3,7 @@ // package io.deephaven.engine.table.impl.select; +import io.deephaven.api.filter.Filter; import io.deephaven.chunk.ChunkType; import io.deephaven.chunk.WritableChunk; import io.deephaven.chunk.WritableObjectChunk; @@ -55,8 +56,8 @@ class FilterSelectColumn implements SelectColumn { * @param filter the filter that is evaluated to true or false for each row of the table * @return a new FilterSelectColumn representing the provided filter. */ - static FilterSelectColumn of(@NotNull final String destName, @NotNull final WhereFilter filter) { - return new FilterSelectColumn(destName, filter); + static FilterSelectColumn of(@NotNull final String destName, @NotNull final Filter filter) { + return new FilterSelectColumn(destName, WhereFilter.of(filter)); } private FilterSelectColumn(@NotNull final String destName, @NotNull final WhereFilter filter) { 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 65817eedb8b..63239a0a461 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 @@ -278,7 +278,7 @@ public SelectColumn visit(Literal rhs) { @Override public SelectColumn visit(Filter rhs) { - return FilterSelectColumn.of(lhs.name(), WhereFilter.of(rhs)); + return FilterSelectColumn.of(lhs.name(), rhs); } @Override From 1893ad599071a6a314e74f7d1cb7d143ac9ecec2 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 14 Nov 2024 14:30:05 -0500 Subject: [PATCH 10/18] missing change --- .../deephaven/engine/table/impl/select/FilterSelectColumn.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index efc8e26b6f6..c772235379b 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -128,13 +128,14 @@ public List getColumns() { @Override public List getColumnArrays() { + /* This should always be empty, because initDef throws when arrays or ii and friends are used. */ return List.of(); } @Override public boolean hasVirtualRowVariables() { /* This should always be false, because initDef throws when arrays or ii and friends are used. */ - return filter.hasVirtualRowVariables(); + return false; } @NotNull From 8f7b1e47f7f1433739c61275e702ec3f811d2dc7 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 20 Nov 2024 05:34:02 -0500 Subject: [PATCH 11/18] Apply suggestions from code review Co-authored-by: Ryan Caudy --- .../table/impl/select/FilterSelectColumn.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index c772235379b..3896e0f52ef 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -25,16 +25,14 @@ import java.util.Map; /** - * The FilterSelectColumn wraps a {@link io.deephaven.api.filter.Filter} and producing a column of true or false Boolean - * values described by the filter. - * + * The FilterSelectColumn wraps a {@link Filter} and producing a column of true or false Boolean values described by the + * filter. *

- * This column is appropriate as an argument to a {@link Table#view(Collection)} or {@link Table#updateView(String...)}, - * lazily evaluating the equivalent of a {@link Table#wouldMatch(String...)} operation. Although select and update can - * also use Filters, the wouldMatch provides a more efficient path for realized results as it stores and updates only a - * {@link RowSet}. The FilterSelectColumn can only evaluate the filter one chunk at a time, and must write to an - * in-memory Boolean column source. - *

+ * This SelectColumn is appropriate as an argument to a {@link Table#view(Collection)} or + * {@link Table#updateView(String...)}, lazily evaluating the equivalent of a {@link Table#wouldMatch(String...)} + * operation. Although select and update can also use Filters, wouldMatch provides a more efficient path for realized + * results as it stores and updates only a {@link RowSet}. The FilterSelectColumn can only evaluate the Filter one chunk + * at a time, and must write to an in-memory {@link Boolean} {@link ColumnSource}. */ class FilterSelectColumn implements SelectColumn { @@ -128,7 +126,7 @@ public List getColumns() { @Override public List getColumnArrays() { - /* This should always be empty, because initDef throws when arrays or ii and friends are used. */ + /* This should always be empty, because initDef throws when arrays or virtual row variables are used. */ return List.of(); } @@ -141,7 +139,7 @@ public boolean hasVirtualRowVariables() { @NotNull @Override public ColumnSource getDataView() { - return new ViewColumnSource<>(Boolean.class, new FilterFormula(), false); + return new ViewColumnSource<>(Boolean.class, new FilterFormula(), isStateless()); } @NotNull @@ -203,7 +201,7 @@ public Boolean getBoolean(long rowKey) { @Override public Boolean getPrevBoolean(long rowKey) { - WritableRowSet filteredIndex = filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, true); + WritableRowSet filteredIndex = filter.filter(RowSetFactory.fromKeys(rowKey), rowSet.prev(), tableToFilter, true); return filteredIndex.isNonempty(); } @@ -264,7 +262,7 @@ private void doFill(@NotNull RowSequence rowSequence, WritableChunk Date: Wed, 20 Nov 2024 05:50:51 -0500 Subject: [PATCH 12/18] Final parameters, prev in doFill, compare definitions. --- .../table/impl/select/FilterSelectColumn.java | 45 ++++++++++++------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 3896e0f52ef..d2536f198b9 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -44,8 +44,11 @@ class FilterSelectColumn implements SelectColumn { @NotNull private final WhereFilter filter; - private RowSet rowSet; private Table tableToFilter; + /** + * We store a copy of our table definition, to ensure that it is identical between initDef and initInputs. + */ + private TableDefinition computedDefinition; /** * Create a FilterSelectColumn with the given name and {@link WhereFilter}. @@ -69,22 +72,27 @@ public String toString() { } @Override - public List initInputs(TrackingRowSet rowSet, Map> columnsOfInterest) { - this.rowSet = rowSet; - this.tableToFilter = new QueryTable(rowSet, columnsOfInterest); + public List initInputs(final TrackingRowSet rowSet, + final Map> columnsOfInterest) { + tableToFilter = new QueryTable(rowSet, columnsOfInterest); + if (!computedDefinition.equals(tableToFilter.getDefinition())) { + throw new IllegalStateException( + "Definition changed between initDef and initInputs in FilterSelectColumn: initDef=" + + computedDefinition + ", initInputs" + tableToFilter.getDefinition()); + } return filter.getColumns(); } @Override - public List initDef(@NotNull Map> columnDefinitionMap) { - filter.init(TableDefinition.of(columnDefinitionMap.values())); + public List initDef(@NotNull final Map> columnDefinitionMap) { + filter.init(computedDefinition = TableDefinition.of(columnDefinitionMap.values())); return checkForInvalidFilters(); } @Override public List initDef(@NotNull final Map> columnDefinitionMap, @NotNull final QueryCompilerRequestProcessor compilationRequestProcessor) { - filter.init(TableDefinition.of(columnDefinitionMap.values()), compilationRequestProcessor); + filter.init(computedDefinition = TableDefinition.of(columnDefinitionMap.values()), compilationRequestProcessor); return checkForInvalidFilters(); } @@ -184,7 +192,7 @@ public FilterSelectColumn copy() { } @Override - public void validateSafeForRefresh(BaseTable sourceTable) { + public void validateSafeForRefresh(final BaseTable sourceTable) { filter.validateSafeForRefresh(sourceTable); } @@ -194,15 +202,17 @@ public FilterFormula() { } @Override - public Boolean getBoolean(long rowKey) { - WritableRowSet filteredIndex = filter.filter(RowSetFactory.fromKeys(rowKey), rowSet, tableToFilter, false); - return filteredIndex.isNonempty(); + public Boolean getBoolean(final long rowKey) { + final WritableRowSet filteredRowSet = + filter.filter(RowSetFactory.fromKeys(rowKey), tableToFilter.getRowSet(), tableToFilter, false); + return filteredRowSet.isNonempty(); } @Override - public Boolean getPrevBoolean(long rowKey) { - WritableRowSet filteredIndex = filter.filter(RowSetFactory.fromKeys(rowKey), rowSet.prev(), tableToFilter, true); - return filteredIndex.isNonempty(); + public Boolean getPrevBoolean(final long rowKey) { + final WritableRowSet filteredRowSet = filter.filter(RowSetFactory.fromKeys(rowKey), + tableToFilter.getRowSet().prev(), tableToFilter, true); + return filteredRowSet.isNonempty(); } @Override @@ -241,12 +251,13 @@ public void fillPrevChunk( doFill(rowSequence, destination, true); } - private void doFill(@NotNull RowSequence rowSequence, WritableChunk destination, - boolean usePrev) { + private void doFill(@NotNull final RowSequence rowSequence, final WritableChunk destination, + final boolean usePrev) { final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); booleanDestination.setSize(rowSequence.intSize()); + final RowSet fullSet = usePrev ? tableToFilter.getRowSet().prev() : tableToFilter.getRowSet(); try (final RowSet inputRowSet = rowSequence.asRowSet(); - final RowSet filtered = filter.filter(inputRowSet, rowSet, tableToFilter, usePrev); + final RowSet filtered = filter.filter(inputRowSet, fullSet, tableToFilter, usePrev); final RowSet.Iterator inputIt = inputRowSet.iterator(); final RowSet.Iterator trueIt = filtered.iterator()) { long nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; From 3bbfd97766481f41593f37c6a07e1ccde86cb121 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 20 Nov 2024 06:18:21 -0500 Subject: [PATCH 13/18] Add test for get/getPrev. --- .../table/impl/select/FilterSelectColumn.java | 14 +++-- .../impl/QueryTableSelectUpdateTest.java | 56 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index d2536f198b9..9ad5cec0d06 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -203,16 +203,18 @@ public FilterFormula() { @Override public Boolean getBoolean(final long rowKey) { - final WritableRowSet filteredRowSet = - filter.filter(RowSetFactory.fromKeys(rowKey), tableToFilter.getRowSet(), tableToFilter, false); - return filteredRowSet.isNonempty(); + try (final WritableRowSet filteredRowSet = + filter.filter(RowSetFactory.fromKeys(rowKey), tableToFilter.getRowSet(), tableToFilter, false)) { + return filteredRowSet.isNonempty(); + } } @Override public Boolean getPrevBoolean(final long rowKey) { - final WritableRowSet filteredRowSet = filter.filter(RowSetFactory.fromKeys(rowKey), - tableToFilter.getRowSet().prev(), tableToFilter, true); - return filteredRowSet.isNonempty(); + try (final WritableRowSet filteredRowSet = filter.filter(RowSetFactory.fromKeys(rowKey), + tableToFilter.getRowSet().prev(), tableToFilter, true)) { + return filteredRowSet.isNonempty(); + } } @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 e74e6584332..08503eedf23 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 @@ -1324,6 +1324,10 @@ public void testFilterExpression() { final Table upv = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter))); assertTableEquals(wm, upv); + // Test the getBoolean method + assertEquals(true, upv.getColumnSource("AWM").get(t.getRowSet().get(0))); + assertEquals(false, upv.getColumnSource("AWM").get(t.getRowSet().get(2))); + // and now a more generic WhereFilter final Filter filter2 = WhereFilterFactory.getExpression("A == 1 || A==3"); @@ -1331,6 +1335,7 @@ public void testFilterExpression() { // use an update final Table up2 = t.update(List.of(Selectable.of(ColumnName.of("AWM"), filter2))); + assertTableEquals(wm2, up2); // a Filter where nothing is true, to check that state final Table upvf = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), Filter.ofFalse()))); @@ -1347,6 +1352,57 @@ public void testFilterExpression() { assertTableEquals(wm3, upv3); } + @Test + public void testFilterExpressionGetPrev() { + final Filter filter = FilterIn.of(ColumnName.of("A"), Literal.of(2), Literal.of(4)); + final QueryTable t = TstUtils.testRefreshingTable(i(2, 4, 6, 8).toTracking(), intCol("A", 1, 2, 3, 4)); + // noinspection resource + final TrackingWritableRowSet rs = t.getRowSet().writableCast(); + + // use an updateView + final Table upv = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter))); + + // Test the getBoolean method + assertEquals(false, upv.getColumnSource("AWM").get(rs.get(0))); + assertEquals(true, upv.getColumnSource("AWM").get(rs.get(1))); + assertEquals(false, upv.getColumnSource("AWM").get(rs.get(2))); + assertEquals(true, upv.getColumnSource("AWM").get(rs.get(3))); + + final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); + + updateGraph.runWithinUnitTestCycle(() -> { + assertEquals(false, upv.getColumnSource("AWM").get(t.getRowSet().get(0))); + assertEquals(true, upv.getColumnSource("AWM").get(t.getRowSet().get(1))); + assertEquals(false, upv.getColumnSource("AWM").get(t.getRowSet().get(2))); + assertEquals(true, upv.getColumnSource("AWM").get(t.getRowSet().get(3))); + + // noinspection resource + final RowSet prevRowset = rs.prev(); + assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(0))); + assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(1))); + assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(2))); + assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(3))); + + addToTable(t, i(1, 2, 9), intCol("A", 2, 2, 4)); + removeRows(t, i(8)); + rs.insert(i(1, 9)); + rs.remove(8); + t.notifyListeners(i(1, 9), i(8), i()); + + assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(0))); + assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(1))); + assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(2))); + assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(3))); + + assertEquals(true, upv.getColumnSource("AWM").get(rs.get(0))); + assertEquals(true, upv.getColumnSource("AWM").get(rs.get(1))); + assertEquals(true, upv.getColumnSource("AWM").get(rs.get(2))); + assertEquals(false, upv.getColumnSource("AWM").get(rs.get(3))); + assertEquals(true, upv.getColumnSource("AWM").get(rs.get(4))); + }); + + } + @Test public void testFilterExpressionArray() { final Filter filter = WhereFilterFactory.getExpression("A=A_[i-1]"); From 1f3f3563c2b79115a5a53288395e408be26608fa Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 20 Nov 2024 07:58:57 -0500 Subject: [PATCH 14/18] fillPrevChunk test. --- .../impl/QueryTableSelectUpdateTest.java | 68 ++++++++++++------- 1 file changed, 42 insertions(+), 26 deletions(-) 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 08503eedf23..cce17482150 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,17 +11,18 @@ import io.deephaven.api.filter.FilterIn; import io.deephaven.api.literal.Literal; import io.deephaven.base.testing.BaseArrayTestCase; +import io.deephaven.chunk.Chunk; +import io.deephaven.chunk.ObjectChunk; +import io.deephaven.chunk.attributes.Values; import io.deephaven.configuration.Configuration; import io.deephaven.engine.context.ExecutionContext; import io.deephaven.engine.context.QueryScope; import io.deephaven.engine.exceptions.UncheckedTableException; import io.deephaven.engine.liveness.LivenessScope; import io.deephaven.engine.liveness.LivenessScopeStack; +import io.deephaven.engine.primitive.iterator.CloseableIterator; 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.WouldMatchPair; +import io.deephaven.engine.table.*; import io.deephaven.engine.table.impl.select.DhFormulaColumn; import io.deephaven.engine.table.impl.select.FormulaCompilationException; import io.deephaven.engine.table.impl.select.WhereFilterFactory; @@ -53,6 +54,7 @@ import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; +import java.util.stream.Collectors; import static io.deephaven.engine.testutil.TstUtils.*; import static io.deephaven.engine.util.TableTools.*; @@ -1363,25 +1365,30 @@ public void testFilterExpressionGetPrev() { final Table upv = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter))); // Test the getBoolean method - assertEquals(false, upv.getColumnSource("AWM").get(rs.get(0))); - assertEquals(true, upv.getColumnSource("AWM").get(rs.get(1))); - assertEquals(false, upv.getColumnSource("AWM").get(rs.get(2))); - assertEquals(true, upv.getColumnSource("AWM").get(rs.get(3))); + ColumnSource resultColumn = upv.getColumnSource("AWM"); + assertEquals(false, resultColumn.get(rs.get(0))); + assertEquals(true, resultColumn.get(rs.get(1))); + assertEquals(false, resultColumn.get(rs.get(2))); + assertEquals(true, resultColumn.get(rs.get(3))); + + // and do it with chunks + try (final CloseableIterator awm = upv.columnIterator("AWM")) { + assertEquals(Arrays.asList(false, true, false, true), awm.stream().collect(Collectors.toList())); + } final ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast(); updateGraph.runWithinUnitTestCycle(() -> { - assertEquals(false, upv.getColumnSource("AWM").get(t.getRowSet().get(0))); - assertEquals(true, upv.getColumnSource("AWM").get(t.getRowSet().get(1))); - assertEquals(false, upv.getColumnSource("AWM").get(t.getRowSet().get(2))); - assertEquals(true, upv.getColumnSource("AWM").get(t.getRowSet().get(3))); + assertEquals(false, resultColumn.get(t.getRowSet().get(0))); + assertEquals(true, resultColumn.get(t.getRowSet().get(1))); + assertEquals(false, resultColumn.get(t.getRowSet().get(2))); + assertEquals(true, resultColumn.get(t.getRowSet().get(3))); - // noinspection resource final RowSet prevRowset = rs.prev(); - assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(0))); - assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(1))); - assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(2))); - assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(3))); + assertEquals(false, resultColumn.getPrev(prevRowset.get(0))); + assertEquals(true, resultColumn.getPrev(prevRowset.get(1))); + assertEquals(false, resultColumn.getPrev(prevRowset.get(2))); + assertEquals(true, resultColumn.getPrev(prevRowset.get(3))); addToTable(t, i(1, 2, 9), intCol("A", 2, 2, 4)); removeRows(t, i(8)); @@ -1389,16 +1396,25 @@ public void testFilterExpressionGetPrev() { rs.remove(8); t.notifyListeners(i(1, 9), i(8), i()); - assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(0))); - assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(1))); - assertEquals(false, upv.getColumnSource("AWM").getPrev(prevRowset.get(2))); - assertEquals(true, upv.getColumnSource("AWM").getPrev(prevRowset.get(3))); + // with a chunk + try (final ChunkSource.GetContext fc = resultColumn.makeGetContext(4)) { + final ObjectChunk prevValues = resultColumn.getPrevChunk(fc, prevRowset).asObjectChunk(); + assertEquals(false, prevValues.get(0)); + assertEquals(true, prevValues.get(1)); + assertEquals(false, prevValues.get(2)); + assertEquals(true, prevValues.get(3)); + } + + assertEquals(false, resultColumn.getPrev(prevRowset.get(0))); + assertEquals(true, resultColumn.getPrev(prevRowset.get(1))); + assertEquals(false, resultColumn.getPrev(prevRowset.get(2))); + assertEquals(true, resultColumn.getPrev(prevRowset.get(3))); - assertEquals(true, upv.getColumnSource("AWM").get(rs.get(0))); - assertEquals(true, upv.getColumnSource("AWM").get(rs.get(1))); - assertEquals(true, upv.getColumnSource("AWM").get(rs.get(2))); - assertEquals(false, upv.getColumnSource("AWM").get(rs.get(3))); - assertEquals(true, upv.getColumnSource("AWM").get(rs.get(4))); + assertEquals(true, resultColumn.get(rs.get(0))); + assertEquals(true, resultColumn.get(rs.get(1))); + assertEquals(true, resultColumn.get(rs.get(2))); + assertEquals(false, resultColumn.get(rs.get(3))); + assertEquals(true, resultColumn.get(rs.get(4))); }); } From d432b0a71b760e41e76b4dcb952f073c3ee3a1a3 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Wed, 20 Nov 2024 09:19:08 -0500 Subject: [PATCH 15/18] performance test and stop refreshing filters. --- .../table/impl/select/FilterSelectColumn.java | 66 +++++++++++++++---- .../impl/QueryTableSelectUpdateTest.java | 64 ++++++++++++++---- 2 files changed, 104 insertions(+), 26 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 9ad5cec0d06..58898307b88 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -113,6 +113,18 @@ private List checkForInvalidFilters() { "Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: " + filter); } + if (filter.isRefreshing()) { + // TODO: DH-18052: updateView and view should support refreshing Filter Expressions + // + // This would enable us to use a whereIn or whereNotIn for things like conditional formatting; which could + // be attractive. However, a join or actualy wouldMatch gets you there without the additional complexity. + // + // Supporting this requires SelectColumn dependencies, which have not previously existed. Additionally, + // if we were to support these for select and update (as opposed to view and updateView), then the filter + // could require recomputing the entire result table whenever anything changes. + throw new UncheckedTableException( + "Cannot use a refreshing filter in select, view, update, or updateView: " + filter); + } return filter.getColumns(); } @@ -258,24 +270,50 @@ private void doFill(@NotNull final RowSequence rowSequence, final WritableChunk< final WritableObjectChunk booleanDestination = destination.asWritableObjectChunk(); booleanDestination.setSize(rowSequence.intSize()); final RowSet fullSet = usePrev ? tableToFilter.getRowSet().prev() : tableToFilter.getRowSet(); + try (final RowSet inputRowSet = rowSequence.asRowSet(); - final RowSet filtered = filter.filter(inputRowSet, fullSet, tableToFilter, usePrev); - final RowSet.Iterator inputIt = inputRowSet.iterator(); - final RowSet.Iterator trueIt = filtered.iterator()) { - long nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; + final RowSet filtered = filter.filter(inputRowSet, fullSet, tableToFilter, usePrev)) { + if (filtered.size() == inputRowSet.size()) { + // if everything matches, short circuit the iteration + booleanDestination.fillWithValue(0, booleanDestination.size(), true); + return; + } + int offset = 0; - while (nextTrue >= 0) { - // the input iterator is a superset of the true iterator, so we can always find out what - // the next value is without needing to check hasNext - final long nextInput = inputIt.nextLong(); - final boolean found = nextInput == nextTrue; - booleanDestination.set(offset++, found); - if (found) { - nextTrue = trueIt.hasNext() ? trueIt.nextLong() : -1; + + try (final RowSet.Iterator inputRows = inputRowSet.iterator(); + final RowSet.Iterator trueRows = filtered.iterator()) { + long nextTrue = trueRows.hasNext() ? trueRows.nextLong() : -1; + while (nextTrue >= 0) { + // the input iterator is a superset of the true iterator, so we can always find out what + // the next value is without needing to check hasNext + final long nextInput = inputRows.nextLong(); + final boolean found = nextInput == nextTrue; + booleanDestination.set(offset++, found); + if (found) { + nextTrue = trueRows.hasNext() ? trueRows.nextLong() : -1; + } } } - // fill everything else up with false, because nothing else can match - booleanDestination.fillWithValue(offset, booleanDestination.size() - offset, false); + + /* + * This alternative formulation from Ryan is fairly close in terms of performance. It might be very + * slightly worse on the dense cases, and slightly better on the sparse cases. + */ + /* + * try (final RowSequence.Iterator inputRows = inputRowSet.getRowSequenceIterator(); final + * RowSet.Iterator trueRows = filtered.iterator()) { while (trueRows.hasNext()) { final long nextTrue = + * trueRows.nextLong(); // Find all the false rows between the last consumed input row and the next true + * row final int falsesSkipped = (int) inputRows.advanceAndGetPositionDistance(nextTrue + 1) - 1; if + * (falsesSkipped > 0) { booleanDestination.fillWithValue(offset, falsesSkipped, false); offset += + * falsesSkipped; } booleanDestination.set(offset++, true); } } + */ + + final int remainingFalses = booleanDestination.size() - offset; + // Fill everything else up with false, because we've exhausted the trues + if (remainingFalses > 0) { + booleanDestination.fillWithValue(offset, remainingFalses, false); + } } } } 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 cce17482150..10e1b015cf5 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 @@ -3,15 +3,11 @@ // package io.deephaven.engine.table.impl; -import io.deephaven.api.ColumnName; -import io.deephaven.api.JoinMatch; -import io.deephaven.api.Selectable; -import io.deephaven.api.TableOperations; +import io.deephaven.api.*; import io.deephaven.api.filter.Filter; import io.deephaven.api.filter.FilterIn; import io.deephaven.api.literal.Literal; import io.deephaven.base.testing.BaseArrayTestCase; -import io.deephaven.chunk.Chunk; import io.deephaven.chunk.ObjectChunk; import io.deephaven.chunk.attributes.Values; import io.deephaven.configuration.Configuration; @@ -23,11 +19,7 @@ import io.deephaven.engine.primitive.iterator.CloseableIterator; import io.deephaven.engine.rowset.*; import io.deephaven.engine.table.*; -import io.deephaven.engine.table.impl.select.DhFormulaColumn; -import io.deephaven.engine.table.impl.select.FormulaCompilationException; -import io.deephaven.engine.table.impl.select.WhereFilterFactory; -import io.deephaven.engine.table.impl.select.SelectColumn; -import io.deephaven.engine.table.impl.select.SelectColumnFactory; +import io.deephaven.engine.table.impl.select.*; import io.deephaven.engine.table.impl.sources.InMemoryColumnSource; import io.deephaven.engine.table.impl.sources.LongSparseArraySource; import io.deephaven.engine.table.impl.sources.RedirectedColumnSource; @@ -51,10 +43,12 @@ import org.junit.Rule; import org.junit.Test; +import java.text.DecimalFormat; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; import java.util.stream.Collectors; +import java.util.stream.IntStream; import static io.deephaven.engine.testutil.TstUtils.*; import static io.deephaven.engine.util.TableTools.*; @@ -1398,7 +1392,8 @@ public void testFilterExpressionGetPrev() { // with a chunk try (final ChunkSource.GetContext fc = resultColumn.makeGetContext(4)) { - final ObjectChunk prevValues = resultColumn.getPrevChunk(fc, prevRowset).asObjectChunk(); + final ObjectChunk prevValues = + resultColumn.getPrevChunk(fc, prevRowset).asObjectChunk(); assertEquals(false, prevValues.get(0)); assertEquals(true, prevValues.get(1)); assertEquals(false, prevValues.get(2)); @@ -1416,7 +1411,44 @@ public void testFilterExpressionGetPrev() { assertEquals(false, resultColumn.get(rs.get(3))); assertEquals(true, resultColumn.get(rs.get(4))); }); + } + @Test + public void testFilterExpressionFillChunkPerformance() { + testFilterExpressionFillChunkPerformance(1.0); + testFilterExpressionFillChunkPerformance(.9999); + testFilterExpressionFillChunkPerformance(.999); + testFilterExpressionFillChunkPerformance(.8725); + testFilterExpressionFillChunkPerformance(.75); + testFilterExpressionFillChunkPerformance(.5); + testFilterExpressionFillChunkPerformance(.25); + testFilterExpressionFillChunkPerformance(.125); + testFilterExpressionFillChunkPerformance(0.001); + testFilterExpressionFillChunkPerformance(0.0001); + } + + public void testFilterExpressionFillChunkPerformance(final double density) { + final int numIterations = 1; + final int size = 100_000; + final Filter filter = FilterIn.of(ColumnName.of("A"), Literal.of(1)); + + final Random random = new Random(20241120); + final List values = IntStream.range(0, size).mapToObj(ignored -> random.nextDouble() < density) + .collect(Collectors.toList()); + QueryScope.addParam("values", values); + final Table t = TableTools.emptyTable(size).update("A=(Boolean)(values[i]) ? 1: 0"); + QueryScope.addParam("values", null); + + final Table upv = t.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter))); + final long startTime = System.nanoTime(); + for (int iters = 0; iters < numIterations; ++iters) { + final long trueValues = upv.columnIterator("AWM").stream().filter(x -> (Boolean) x).count(); + assertEquals(values.stream().filter(x -> x).count(), trueValues); + } + final long endTime = System.nanoTime(); + final double duration = endTime - startTime; + System.out.println("Density: " + new DecimalFormat("0.0000").format(density) + ", Nanos: " + (long) duration + + ", per cell=" + new DecimalFormat("0.00").format(duration / (size * numIterations))); } @Test @@ -1424,6 +1456,8 @@ public void testFilterExpressionArray() { final Filter filter = WhereFilterFactory.getExpression("A=A_[i-1]"); final Filter filterArrayOnly = WhereFilterFactory.getExpression("A=A_.size() = 1"); final Filter filterKonly = WhereFilterFactory.getExpression("A=k+1"); + final QueryTable setTable = TstUtils.testRefreshingTable(intCol("B")); + final Filter whereIn = new DynamicWhereFilter(setTable, true, MatchPairFactory.getExpression("A=B")); final QueryTable table = TstUtils.testRefreshingTable(intCol("A", 1, 1, 2, 3, 5, 8, 9, 9)); final UncheckedTableException wme = Assert.assertThrows(UncheckedTableException.class, @@ -1446,7 +1480,7 @@ public void testFilterExpressionArray() { () -> table.updateView(List.of(Selectable.of(ColumnName.of("AWM"), filter)))); Assert.assertEquals( "Cannot use a filter with column Vectors (_ syntax) in select, view, update, or updateView: A=A_[i-1]", - upe.getMessage()); + uve.getMessage()); final UncheckedTableException se = Assert.assertThrows(UncheckedTableException.class, () -> table.select(List.of(Selectable.of(ColumnName.of("AWM"), filterKonly)))); @@ -1460,6 +1494,12 @@ public void testFilterExpressionArray() { "Cannot use a filter with virtual row variables (i, ii, or k) in select, view, update, or updateView: A=k+1", ve.getMessage()); + final UncheckedTableException dw = Assert.assertThrows(UncheckedTableException.class, + () -> table.view(List.of(Selectable.of(ColumnName.of("AWM"), whereIn)))); + Assert.assertEquals( + "Cannot use a refreshing filter in select, view, update, or updateView: DynamicWhereFilter([A=B])", + dw.getMessage()); + } @Test From 3ecc9b6f986a22983796c93efaf96bcf9f7cf3c7 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Thu, 21 Nov 2024 14:58:34 -0500 Subject: [PATCH 16/18] Use Ryan's formulation of fillChunk. --- .../table/impl/select/FilterSelectColumn.java | 32 ++++++------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 58898307b88..0b9b358265a 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -281,34 +281,20 @@ private void doFill(@NotNull final RowSequence rowSequence, final WritableChunk< int offset = 0; - try (final RowSet.Iterator inputRows = inputRowSet.iterator(); + try (final RowSequence.Iterator inputRows = inputRowSet.getRowSequenceIterator(); final RowSet.Iterator trueRows = filtered.iterator()) { - long nextTrue = trueRows.hasNext() ? trueRows.nextLong() : -1; - while (nextTrue >= 0) { - // the input iterator is a superset of the true iterator, so we can always find out what - // the next value is without needing to check hasNext - final long nextInput = inputRows.nextLong(); - final boolean found = nextInput == nextTrue; - booleanDestination.set(offset++, found); - if (found) { - nextTrue = trueRows.hasNext() ? trueRows.nextLong() : -1; + while (trueRows.hasNext()) { + final long nextTrue = trueRows.nextLong(); + // Find all the false rows between the last consumed input row and the next true row + final int falsesSkipped = (int) inputRows.advanceAndGetPositionDistance(nextTrue + 1) - 1; + if (falsesSkipped > 0) { + booleanDestination.fillWithValue(offset, falsesSkipped, false); + offset += falsesSkipped; } + booleanDestination.set(offset++, true); } } - /* - * This alternative formulation from Ryan is fairly close in terms of performance. It might be very - * slightly worse on the dense cases, and slightly better on the sparse cases. - */ - /* - * try (final RowSequence.Iterator inputRows = inputRowSet.getRowSequenceIterator(); final - * RowSet.Iterator trueRows = filtered.iterator()) { while (trueRows.hasNext()) { final long nextTrue = - * trueRows.nextLong(); // Find all the false rows between the last consumed input row and the next true - * row final int falsesSkipped = (int) inputRows.advanceAndGetPositionDistance(nextTrue + 1) - 1; if - * (falsesSkipped > 0) { booleanDestination.fillWithValue(offset, falsesSkipped, false); offset += - * falsesSkipped; } booleanDestination.set(offset++, true); } } - */ - final int remainingFalses = booleanDestination.size() - offset; // Fill everything else up with false, because we've exhausted the trues if (remainingFalses > 0) { From fa87da883fbda4257bc2fb0cc480fee488f85929 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Fri, 22 Nov 2024 16:08:35 -0500 Subject: [PATCH 17/18] Code review. --- .../table/impl/select/FilterSelectColumn.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 0b9b358265a..74926639988 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -114,14 +114,14 @@ private List checkForInvalidFilters() { + filter); } if (filter.isRefreshing()) { - // TODO: DH-18052: updateView and view should support refreshing Filter Expressions - // - // This would enable us to use a whereIn or whereNotIn for things like conditional formatting; which could - // be attractive. However, a join or actualy wouldMatch gets you there without the additional complexity. - // - // Supporting this requires SelectColumn dependencies, which have not previously existed. Additionally, - // if we were to support these for select and update (as opposed to view and updateView), then the filter - // could require recomputing the entire result table whenever anything changes. + /* TODO: DH-18052: updateView and view should support refreshing Filter Expressions + * + * This would enable us to use a whereIn or whereNotIn for things like conditional formatting; which could + * be attractive. However, a join or wouldMatch gets you there without the additional complexity. + * + * Supporting this requires SelectColumn dependencies, which have not previously existed. Additionally, + * if we were to support these for select and update (as opposed to view and updateView), then the filter + * could require recomputing the entire result table whenever anything changes. */ throw new UncheckedTableException( "Cannot use a refreshing filter in select, view, update, or updateView: " + filter); } @@ -215,15 +215,17 @@ public FilterFormula() { @Override public Boolean getBoolean(final long rowKey) { - try (final WritableRowSet filteredRowSet = - filter.filter(RowSetFactory.fromKeys(rowKey), tableToFilter.getRowSet(), tableToFilter, false)) { + try (final WritableRowSet selection = RowSetFactory.fromKeys(rowKey); + final WritableRowSet filteredRowSet = + filter.filter(selection, tableToFilter.getRowSet(), tableToFilter, false)) { return filteredRowSet.isNonempty(); } } @Override public Boolean getPrevBoolean(final long rowKey) { - try (final WritableRowSet filteredRowSet = filter.filter(RowSetFactory.fromKeys(rowKey), + try (final WritableRowSet selection = RowSetFactory.fromKeys(rowKey); + final WritableRowSet filteredRowSet = filter.filter(selection, tableToFilter.getRowSet().prev(), tableToFilter, true)) { return filteredRowSet.isNonempty(); } From a445088cbbc22fb502133524c808b406256e2084 Mon Sep 17 00:00:00 2001 From: "Charles P. Wright" Date: Fri, 22 Nov 2024 16:08:58 -0500 Subject: [PATCH 18/18] spotless. --- .../table/impl/select/FilterSelectColumn.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java index 74926639988..2b096a69002 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/FilterSelectColumn.java @@ -114,14 +114,16 @@ private List checkForInvalidFilters() { + filter); } if (filter.isRefreshing()) { - /* TODO: DH-18052: updateView and view should support refreshing Filter Expressions - * - * This would enable us to use a whereIn or whereNotIn for things like conditional formatting; which could - * be attractive. However, a join or wouldMatch gets you there without the additional complexity. - * - * Supporting this requires SelectColumn dependencies, which have not previously existed. Additionally, - * if we were to support these for select and update (as opposed to view and updateView), then the filter - * could require recomputing the entire result table whenever anything changes. */ + /* + * TODO: DH-18052: updateView and view should support refreshing Filter Expressions + * + * This would enable us to use a whereIn or whereNotIn for things like conditional formatting; which could + * be attractive. However, a join or wouldMatch gets you there without the additional complexity. + * + * Supporting this requires SelectColumn dependencies, which have not previously existed. Additionally, if + * we were to support these for select and update (as opposed to view and updateView), then the filter could + * require recomputing the entire result table whenever anything changes. + */ throw new UncheckedTableException( "Cannot use a refreshing filter in select, view, update, or updateView: " + filter); } @@ -217,7 +219,7 @@ public FilterFormula() { public Boolean getBoolean(final long rowKey) { try (final WritableRowSet selection = RowSetFactory.fromKeys(rowKey); final WritableRowSet filteredRowSet = - filter.filter(selection, tableToFilter.getRowSet(), tableToFilter, false)) { + filter.filter(selection, tableToFilter.getRowSet(), tableToFilter, false)) { return filteredRowSet.isNonempty(); } } @@ -225,8 +227,8 @@ public Boolean getBoolean(final long rowKey) { @Override public Boolean getPrevBoolean(final long rowKey) { try (final WritableRowSet selection = RowSetFactory.fromKeys(rowKey); - final WritableRowSet filteredRowSet = filter.filter(selection, - tableToFilter.getRowSet().prev(), tableToFilter, true)) { + final WritableRowSet filteredRowSet = filter.filter(selection, + tableToFilter.getRowSet().prev(), tableToFilter, true)) { return filteredRowSet.isNonempty(); } }