JS API must support SKIP as an aggregation type (#4780)

Fixes #4182
deephaven · Nov 16, 2023 · a52c2fe · a52c2fe
1 parent b7fdbcc
commit a52c2fe
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 7 deletions.
diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/JsTotalsTableConfig.java b/web/client-api/src/main/java/io/deephaven/web/client/api/JsTotalsTableConfig.java
@@ -83,7 +83,8 @@ public class JsTotalsTableConfig {
             JsAggregationOperation.LAST,
             JsAggregationOperation.COUNT_DISTINCT,
             JsAggregationOperation.DISTINCT,
-            JsAggregationOperation.UNIQUE);
+            JsAggregationOperation.UNIQUE,
+            JsAggregationOperation.SKIP);
 
     /**
      * Specifies if a Totals Table should be expanded by default in the UI. Defaults to false.
@@ -111,7 +112,6 @@ public class JsTotalsTableConfig {
      */
     public JsArray<String> groupBy = new JsArray<>();
 
-    private AggregateRequest grpcRequest;
     private JsArray<String> customColumns;
     private JsArray<String> dropColumns;
 
@@ -406,6 +406,10 @@ public AggregateRequest buildRequest(JsArray<Column> allColumns) {
                 // case JsAggregationOperation.WSUM: {
                 // // TODO #3302 support this
                 // }
+                case JsAggregationOperation.SKIP: {
+                    // cancel entirely, start the loop again
+                    return;
+                }
                 default:
                     JsLog.warn("Aggregation " + aggregationType + " not supported, ignoring");
             }

diff --git a/web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsRollupConfig.java b/web/client-api/src/main/java/io/deephaven/web/client/api/tree/JsRollupConfig.java
@@ -261,6 +261,10 @@ public RollupRequest buildRequest(JsArray<Column> tableColumns) {
                 // case JsAggregationOperation.WSUM: {
                 // // TODO #3302 support this
                 // }
+                case JsAggregationOperation.SKIP: {
+                    // cancel entirely, start the loop again
+                    return;
+                }
                 default:
                     JsLog.warn("Aggregation " + aggregationType + " not supported, ignoring");
             }

diff --git a/...ient-api/src/main/java/io/deephaven/web/client/api/tree/enums/JsAggregationOperation.java b/...ient-api/src/main/java/io/deephaven/web/client/api/tree/enums/JsAggregationOperation.java
@@ -74,17 +74,18 @@ public class JsAggregationOperation {
              */
             LAST = "Last",
             UNIQUE = "Unique";
-    /**
-     * Indicates that this column should not be aggregated. String value is "Skip".
-     */
+
     // Array operation isn't legal in all contexts, just omit it for now
     // ARRAY = "Array",
     // These need some other parameter to function, not supported yet
     // TODO #3302 support these
     // SORTED_FIRST="SortedFirst",
     // SORTED_LAST="SortedLast",
     // WSUM = "WeightedSum";
-    @Deprecated
+
+    /**
+     * Indicates that this column should not be aggregated. String value is "Skip".
+     */
     public static final String SKIP = "Skip";
 
     @JsIgnore
@@ -95,7 +96,8 @@ public static boolean canAggregateType(String aggregationType, String columnType
             case DISTINCT:
             case FIRST:
             case LAST:
-            case UNIQUE: {
+            case UNIQUE:
+            case SKIP: {
                 // These operations are always safe
                 return true;
             }