Address Blink table usage of i, ii, k, and column arrays; addre…

…ss improper memoization sharing between blink and non-blink table copies (#5216)

* Conditions and formulas applied to blink tables should be allowed to use i, ii, k, and column arrays.

* 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/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;

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;

engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java

@@ -241,9 +241,9 @@ public void validateSafeForRefresh(BaseTable<?> sourceTable) {
  }
  if (sourceTable.isRefreshing() && !AbstractFormulaColumn.ALLOW_UNSAFE_REFRESHING_FORMULAS) {
  // note that constant offset array accesss does not use i/ii or end up in usedColumnArrays
- boolean isUnsafe = !sourceTable.isAppendOnly() && (usesI || usesII);
- isUnsafe |= !sourceTable.isAddOnly() && usesK;
- isUnsafe |= !usedColumnArrays.isEmpty();
+ boolean isUnsafe = (usesI || usesII) && !sourceTable.isAppendOnly() && !sourceTable.isBlink();
+ isUnsafe |= usesK && !sourceTable.isAddOnly() && !sourceTable.isBlink();
+ isUnsafe |= !usedColumnArrays.isEmpty() && !sourceTable.isBlink();
  if (isUnsafe) {
  throw new IllegalArgumentException("Formula '" + formula + "' uses i, ii, k, or column array " +
  "variables, and is not safe to refresh. Note that some usages, such as on an append-only " +

engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractFormulaColumn.java

@@ -90,9 +90,9 @@ public void validateSafeForRefresh(BaseTable<?> sourceTable) {
  }
  if (sourceTable.isRefreshing() && !ALLOW_UNSAFE_REFRESHING_FORMULAS) {
  // note that constant offset array accesss does not use i/ii or end up in usedColumnArrays
- boolean isUnsafe = !sourceTable.isAppendOnly() && (usesI || usesII);
- isUnsafe |= !sourceTable.isAddOnly() && usesK;
- isUnsafe |= !usedColumnArrays.isEmpty();
+ boolean isUnsafe = (usesI || usesII) && !sourceTable.isAppendOnly() && !sourceTable.isBlink();
+ isUnsafe |= usesK && !sourceTable.isAddOnly() && !sourceTable.isBlink();
+ isUnsafe |= !usedColumnArrays.isEmpty() && !sourceTable.isBlink();
  if (isUnsafe) {
  throw new IllegalArgumentException("Formula '" + formulaString + "' uses i, ii, k, or column array " +
  "variables, and is not safe to refresh. Note that some usages, such as on an append-only " +