Propagate Blink Attribute Downstream of View/UpdateView/Select/Update…

…View (#5125)
deephaven · Feb 8, 2024 · 59bbf86 · 59bbf86
1 parent ac364d1
commit 59bbf86
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 4 deletions.
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java
@@ -299,13 +299,14 @@ public enum CopyAttributeOperation {
 
         tempMap.put(SNAPSHOT_VIEWPORT_TYPE, EnumSet.allOf(CopyAttributeOperation.class));
 
+        // Note: The logic applying this attribute for select/update/view/updateView is in SelectAndViewAnalyzerWrapper.
         tempMap.put(ADD_ONLY_TABLE_ATTRIBUTE, EnumSet.of(
                 CopyAttributeOperation.DropColumns,
                 CopyAttributeOperation.RenameColumns,
                 CopyAttributeOperation.PartitionBy,
                 CopyAttributeOperation.Coalesce));
 
-
+        // Note: The logic applying this attribute for select/update/view/updateView is in SelectAndViewAnalyzerWrapper.
         tempMap.put(APPEND_ONLY_TABLE_ATTRIBUTE, EnumSet.of(
                 CopyAttributeOperation.DropColumns,
                 CopyAttributeOperation.RenameColumns,
@@ -339,6 +340,7 @@ public enum CopyAttributeOperation {
                 CopyAttributeOperation.Filter,
                 CopyAttributeOperation.PartitionBy));
 
+        // Note: The logic applying this attribute for select/update/view/updateView is in SelectAndViewAnalyzerWrapper.
         tempMap.put(BLINK_TABLE_ATTRIBUTE, EnumSet.of(
                 CopyAttributeOperation.Coalesce,
                 CopyAttributeOperation.Filter,
@@ -347,8 +349,6 @@ public enum CopyAttributeOperation {
                 CopyAttributeOperation.Flatten,
                 CopyAttributeOperation.PartitionBy,
                 CopyAttributeOperation.Preview,
-                CopyAttributeOperation.View, // and Select, if added
-                CopyAttributeOperation.UpdateView, // and Update, if added
                 CopyAttributeOperation.DropColumns,
                 CopyAttributeOperation.RenameColumns,
                 CopyAttributeOperation.Join,

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/QueryTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/QueryTable.java
@@ -1732,7 +1732,7 @@ public Table lazyUpdate(final Collection<? extends Selectable> newColumns) {
                         final SelectAndViewAnalyzerWrapper analyzerWrapper = SelectAndViewAnalyzer.create(
                                 this, SelectAndViewAnalyzer.Mode.VIEW_LAZY, columns, rowSet,
                                 getModifiedColumnSetForUpdates(),
-                                true, false, selectColumns);
+                                true, true, selectColumns);
                         final SelectColumn[] processedColumns = analyzerWrapper.getProcessedColumns()
                                 .toArray(SelectColumn[]::new);
                         final QueryTable result = new QueryTable(

diff --git a/...in/java/io/deephaven/engine/table/impl/select/analyzers/SelectAndViewAnalyzerWrapper.java b/...in/java/io/deephaven/engine/table/impl/select/analyzers/SelectAndViewAnalyzerWrapper.java
@@ -80,6 +80,10 @@ public QueryTable applyShiftsAndRemainingColumns(
                 // be convenient for test authors by propagating the test source table attribute
                 queryTable.setAttribute(Table.TEST_SOURCE_TABLE_ATTRIBUTE, true);
             }
+            if (sourceTable.isBlink()) {
+                // blink tables, although possibly not useful, can have shift columns
+                queryTable.setAttribute(Table.BLINK_TABLE_ATTRIBUTE, true);
+            }
         }
 
         boolean isMultiStateSelect = shiftColumn != null || remainingCols != null;

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
@@ -4,6 +4,7 @@
 package io.deephaven.engine.table.impl;
 
 import io.deephaven.api.JoinMatch;
+import io.deephaven.api.TableOperations;
 import io.deephaven.base.testing.BaseArrayTestCase;
 import io.deephaven.configuration.Configuration;
 import io.deephaven.engine.context.ExecutionContext;
@@ -1237,4 +1238,80 @@ public void testAliasColumnSelectRefreshing() {
 
         Assert.assertEquals(numCalls.intValue(), 2 * size);
     }
+
+    @FunctionalInterface
+    private interface TableOpInvoker {
+        Table invoke(Table source, String... args);
+    }
+
+    @Test
+    public void testPropagationOfAttributes() {
+        final TableOpInvoker[] tableOps = new TableOpInvoker[] {
+                TableOperations::select,
+                TableOperations::update,
+                TableOperations::view,
+                TableOperations::updateView,
+                TableOperations::lazyUpdate
+        };
+
+        // Add-only with no shift column; propagate
+        final BaseTable<?> addonly = testRefreshingTable(RowSetFactory.empty().toTracking());
+        addonly.setAttribute(Table.ADD_ONLY_TABLE_ATTRIBUTE, Boolean.TRUE);
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(addonly, "I = ii");
+            Assert.assertTrue(result.isAddOnly());
+        }
+
+        // Add-only with positive shift column; don't propagate (generates modifies if adds between existing rows)
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(addonly, "I = ii", "J = I_[ii + 1]");
+            Assert.assertFalse(result.isAddOnly());
+        }
+
+        // Add-only with negative shift column; don't propagate (generates modifies if adds between existing rows)
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(addonly, "I = ii", "J = I_[ii - 1]");
+            Assert.assertFalse(result.isAddOnly());
+        }
+
+        // Append-only with no shift column; propagate
+        final BaseTable<?> appendOnly = testRefreshingTable(RowSetFactory.empty().toTracking());
+        appendOnly.setAttribute(Table.APPEND_ONLY_TABLE_ATTRIBUTE, Boolean.TRUE);
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(appendOnly, "I = ii");
+            Assert.assertTrue(result.isAppendOnly());
+        }
+
+        // Append-only with positive shift column; don't propagate (shift depends on future rows thus generates mods)
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(appendOnly, "I = ii", "J = I_[ii + 1]");
+            Assert.assertFalse(result.isAppendOnly());
+        }
+
+        // Append-only with negative shift column; propagate (shift depends on rows that will never change)
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(appendOnly, "I = ii", "J = I_[ii - 1]");
+            Assert.assertTrue(result.isAppendOnly());
+        }
+
+        // Blink with no shift column; propagate
+        final BaseTable<?> blink = testRefreshingTable(RowSetFactory.empty().toTracking());
+        blink.setAttribute(Table.BLINK_TABLE_ATTRIBUTE, Boolean.TRUE);
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(blink, "I = ii");
+            Assert.assertTrue(result.isBlink());
+        }
+
+        // Blink with positive shift column; propagate (no rows are saved across cycles)
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(blink, "I = ii", "J = I_[ii + 1]");
+            Assert.assertTrue(result.isBlink());
+        }
+
+        // Blink with negative shift column; propagate (no rows are saved across cycles)
+        for (TableOpInvoker op : tableOps) {
+            final BaseTable<?> result = (BaseTable<?>) op.invoke(blink, "I = ii", "J = I_[ii - 1]");
+            Assert.assertTrue(result.isBlink());
+        }
+    }
 }