From ade825dbab9a2014a2541c28b96907641a2135eb Mon Sep 17 00:00:00 2001
From: Ryan Caudy <rcaudy@gmail.com>
Date: Mon, 4 Mar 2024 13:05:40 -0500
Subject: [PATCH] Ensure that aggregations and rollups on copied tables (e.g.
 those resulting from attribute changes for adding/removing "blink") never use
 the parent rather than the copy if "isBlink" is different.

---
 .../engine/table/impl/BlinkTableTools.java      | 17 ++++++++++++++---
 .../engine/table/impl/MemoizedOperationKey.java | 14 ++++++++++++--
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/BlinkTableTools.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/BlinkTableTools.java
index 12dc55fb52a..38e73b19b97 100644
--- a/engine/table/src/main/java/io/deephaven/engine/table/impl/BlinkTableTools.java
+++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/BlinkTableTools.java
@@ -104,19 +104,30 @@ public static Table blinkToAppendOnly(
     }
 
     /**
-     * Returns true if table is a blink table.
+     * Returns true if {@code table} is a blink table.
      *
      * @param table The table to check for blink behavior
-     * @return Whether this table is a blink table
+     * @return Whether {@code table} is a blink table
      * @see Table#BLINK_TABLE_ATTRIBUTE
      */
-    public static boolean isBlink(Table table) {
+    public static boolean isBlink(@NotNull final Table table) {
         if (!table.isRefreshing()) {
             return false;
         }
         return Boolean.TRUE.equals(table.getAttribute(Table.BLINK_TABLE_ATTRIBUTE));
     }
 
+    /**
+     * Returns true if {@code attributes} indicate a blink table.
+     *
+     * @param attributes The map to check for blink table attributes
+     * @return Whether {@code attributes} indicate a blink table
+     * @see Table#BLINK_TABLE_ATTRIBUTE
+     */
+    public static boolean hasBlink(@NotNull final Map<String, Object> attributes) {
+        return Boolean.TRUE.equals(attributes.get(Table.BLINK_TABLE_ATTRIBUTE));
+    }
+
     private static class BlinkToAppendOnlyOperation implements QueryTable.MemoizableOperation<QueryTable> {
         private final QueryTable parent;
         private final long sizeLimit;
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/MemoizedOperationKey.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/MemoizedOperationKey.java
index 1aa88d6a160..6d7541b073f 100644
--- a/engine/table/src/main/java/io/deephaven/engine/table/impl/MemoizedOperationKey.java
+++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/MemoizedOperationKey.java
@@ -60,6 +60,16 @@ boolean attributesCompatible(Map<String, Object> oldAttributes, Map<String, Obje
         abstract BaseTable.CopyAttributeOperation copyType();
     }
 
+    abstract static class BlinkIncompatibleMemoizedOperationKey extends MemoizedOperationKey {
+        @Override
+        boolean attributesCompatible(Map<String, Object> oldAttributes, Map<String, Object> newAttributes) {
+            return BlinkTableTools.hasBlink(oldAttributes) == BlinkTableTools.hasBlink(newAttributes);
+        }
+
+        @Override
+        abstract BaseTable.CopyAttributeOperation copyType();
+    }
+
     public interface Provider {
         MemoizedOperationKey getMemoKey();
     }
@@ -360,7 +370,7 @@ BaseTable.CopyAttributeOperation getParentCopyType() {
         }
     }
 
-    private static class AggBy extends AttributeAgnosticMemoizedOperationKey {
+    private static class AggBy extends BlinkIncompatibleMemoizedOperationKey {
 
         private final List<? extends Aggregation> aggregations;
         private final boolean preserveEmpty;
@@ -442,7 +452,7 @@ public int hashCode() {
         }
     }
 
-    private static class Rollup extends AttributeAgnosticMemoizedOperationKey {
+    private static class Rollup extends BlinkIncompatibleMemoizedOperationKey {
 
         private final AggBy aggBy;
         private final boolean includeConstituents;