Addressing PR comments, improving code coverage.

lbooker42 · lbooker42 · commit d8abd522b1ae · 2026-04-28T09:25:44.000-07:00
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilterDelegating.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/WhereFilterDelegating.java
@@ -18,7 +18,7 @@ public interface WhereFilterDelegating {
 
     /**
      * If the provided filter is an instance of {@code WhereFilterDelegating}, returns the effective wrapped filter of
-     * that filter. Otherwise, returns the filter itself wrapped in an Optional.
+     * that filter. Otherwise, returns the filter itself.
      */
     static WhereFilter maybeUnwrapFilter(WhereFilter filter) {
         if (filter instanceof WhereFilterDelegating) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/ByteRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/ByteRegionBinarySearchKernel.java
@@ -124,7 +124,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -156,9 +156,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -193,11 +193,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/CharRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/CharRegionBinarySearchKernel.java
@@ -120,7 +120,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -152,9 +152,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -189,11 +189,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/DoubleRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/DoubleRegionBinarySearchKernel.java
@@ -124,7 +124,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -156,9 +156,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -193,11 +193,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/FloatRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/FloatRegionBinarySearchKernel.java
@@ -124,7 +124,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -156,9 +156,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -193,11 +193,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/IntRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/IntRegionBinarySearchKernel.java
@@ -124,7 +124,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -156,9 +156,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -193,11 +193,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/LongRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/LongRegionBinarySearchKernel.java
@@ -124,7 +124,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -156,9 +156,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -193,11 +193,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/ObjectRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/ObjectRegionBinarySearchKernel.java
@@ -123,7 +123,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -155,9 +155,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -192,11 +192,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/ShortRegionBinarySearchKernel.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/regioned/kernel/ShortRegionBinarySearchKernel.java
@@ -124,7 +124,7 @@ public static RowSet binarySearchMinMax(
         }
 
         // Validate that a logical range was found and the bounds didn't cross
-        if (start != -1 && end != -1) {
+        if (start != -1 && end != -1 && start <= end) {
             return RowSetFactory.fromRange(start, end);
         }
 
@@ -156,9 +156,9 @@ public static RowSet binarySearchMin(
 
         if (sortColumn.isAscending()) {
             start = findStartIndexAscending(region, firstKey, lastKey, min, minInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         } else {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexDescending(region, firstKey, lastKey, min, minInc);
         }
 
@@ -193,11 +193,11 @@ public static RowSet binarySearchMax(
         final int end;
 
         if (sortColumn.isAscending()) {
-            start = (int) firstKey;
+            start = Math.toIntExact(firstKey);
             end = findEndIndexAscending(region, firstKey, lastKey, max, maxInc);
         } else {
             start = findStartIndexDescending(region, firstKey, lastKey, max, maxInc);
-            end = (int) lastKey;
+            end = Math.toIntExact(lastKey);
         }
 
         if (start != -1 && end != -1 && start <= end) {
diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableWhereTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableWhereTest.java
@@ -2726,7 +2726,7 @@ public void testMergedTableSources() {
                 filter1.withRespectedBarriers("1").withDeclaredBarriers("2"),
                 postFilter.withRespectedBarriers("2")));
         assertEquals(200_000, preFilter.numRowsProcessed());
-        assertEquals(100_000, filter0.numRowsProcessed()); // 100_000 from source1, 0 from source2
+        assertEquals(100_000, filter1.numRowsProcessed()); // 100_000 from source1, 0 from source2
         assertEquals(99_999, postFilter.numRowsProcessed()); // 99_000 from source1, 0 from source2
 
         assertEquals(99_999, res1.size());
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionByte.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionByte.java
@@ -49,7 +49,7 @@ public final class ParquetColumnRegionByte<ATTR extends Any> extends ParquetColu
     private static final RegionedPushdownAction.Region SORTED_REGION_ACTION =
             new RegionedPushdownAction.Region(
                     () -> QueryTable.DISABLE_WHERE_PUSHDOWN_SORTED_COLUMN_LOCATION,
-                    PushdownResult.REGION_DICTIONARY_DATA_COST,
+                    PushdownResult.REGION_SORTED_DATA_COST,
                     (ctx) -> ctx.isMatchFilter() || ctx.isRangeFilter(),
                     (tl, cr) -> true);
     private static final List<RegionedPushdownAction> SUPPORTED_ACTIONS = List.of(SORTED_REGION_ACTION);
@@ -99,10 +99,10 @@ public long estimatePushdownAction(
         if (action.equals(SORTED_REGION_ACTION)) {
             final RegionedPushdownFilterContext ctx = (RegionedPushdownFilterContext) filterContext;
             final TableLocation tableLocation = getColumnLocation().map(ColumnLocation::getTableLocation).orElse(null);
-            if (tableLocation == null || (ctx.isMatchFilter() && !ctx.isRangeFilter())) {
+            if (tableLocation == null || (!ctx.isMatchFilter() && !ctx.isRangeFilter())) {
                 return PushdownResult.UNSUPPORTED_ACTION_COST;
             }
-            // Only range and match filers can benefit from sorted column data.
+            // Only range and match filters can benefit from sorted column data.
             final SortColumn firstSortedColumn = tableLocation.getSortedColumns().isEmpty()
                     ? null
                     : tableLocation.getSortedColumns().get(0);
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionChar.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionChar.java
@@ -40,7 +40,7 @@ public final class ParquetColumnRegionChar<ATTR extends Any> extends ParquetColu
     private static final RegionedPushdownAction.Region SORTED_REGION_ACTION =
             new RegionedPushdownAction.Region(
                     () -> QueryTable.DISABLE_WHERE_PUSHDOWN_SORTED_COLUMN_LOCATION,
-                    PushdownResult.REGION_DICTIONARY_DATA_COST,
+                    PushdownResult.REGION_SORTED_DATA_COST,
                     (ctx) -> ctx.isMatchFilter() || ctx.isRangeFilter(),
                     (tl, cr) -> true);
     private static final List<RegionedPushdownAction> SUPPORTED_ACTIONS = List.of(SORTED_REGION_ACTION);
@@ -78,10 +78,10 @@ public long estimatePushdownAction(
         if (action.equals(SORTED_REGION_ACTION)) {
             final RegionedPushdownFilterContext ctx = (RegionedPushdownFilterContext) filterContext;
             final TableLocation tableLocation = getColumnLocation().map(ColumnLocation::getTableLocation).orElse(null);
-            if (tableLocation == null || (ctx.isMatchFilter() && !ctx.isRangeFilter())) {
+            if (tableLocation == null || (!ctx.isMatchFilter() && !ctx.isRangeFilter())) {
                 return PushdownResult.UNSUPPORTED_ACTION_COST;
             }
-            // Only range and match filers can benefit from sorted column data.
+            // Only range and match filters can benefit from sorted column data.
             final SortColumn firstSortedColumn = tableLocation.getSortedColumns().isEmpty()
                     ? null
                     : tableLocation.getSortedColumns().get(0);
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionDouble.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionDouble.java
@@ -44,7 +44,7 @@ public final class ParquetColumnRegionDouble<ATTR extends Any> extends ParquetCo
     private static final RegionedPushdownAction.Region SORTED_REGION_ACTION =
             new RegionedPushdownAction.Region(
                     () -> QueryTable.DISABLE_WHERE_PUSHDOWN_SORTED_COLUMN_LOCATION,
-                    PushdownResult.REGION_DICTIONARY_DATA_COST,
+                    PushdownResult.REGION_SORTED_DATA_COST,
                     (ctx) -> ctx.isMatchFilter() || ctx.isRangeFilter(),
                     (tl, cr) -> true);
     private static final List<RegionedPushdownAction> SUPPORTED_ACTIONS = List.of(SORTED_REGION_ACTION);
@@ -82,10 +82,10 @@ public long estimatePushdownAction(
         if (action.equals(SORTED_REGION_ACTION)) {
             final RegionedPushdownFilterContext ctx = (RegionedPushdownFilterContext) filterContext;
             final TableLocation tableLocation = getColumnLocation().map(ColumnLocation::getTableLocation).orElse(null);
-            if (tableLocation == null || (ctx.isMatchFilter() && !ctx.isRangeFilter())) {
+            if (tableLocation == null || (!ctx.isMatchFilter() && !ctx.isRangeFilter())) {
                 return PushdownResult.UNSUPPORTED_ACTION_COST;
             }
-            // Only range and match filers can benefit from sorted column data.
+            // Only range and match filters can benefit from sorted column data.
             final SortColumn firstSortedColumn = tableLocation.getSortedColumns().isEmpty()
                     ? null
                     : tableLocation.getSortedColumns().get(0);
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionFloat.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionFloat.java
@@ -44,7 +44,7 @@ public final class ParquetColumnRegionFloat<ATTR extends Any> extends ParquetCol
     private static final RegionedPushdownAction.Region SORTED_REGION_ACTION =
             new RegionedPushdownAction.Region(
                     () -> QueryTable.DISABLE_WHERE_PUSHDOWN_SORTED_COLUMN_LOCATION,
-                    PushdownResult.REGION_DICTIONARY_DATA_COST,
+                    PushdownResult.REGION_SORTED_DATA_COST,
                     (ctx) -> ctx.isMatchFilter() || ctx.isRangeFilter(),
                     (tl, cr) -> true);
     private static final List<RegionedPushdownAction> SUPPORTED_ACTIONS = List.of(SORTED_REGION_ACTION);
@@ -82,10 +82,10 @@ public long estimatePushdownAction(
         if (action.equals(SORTED_REGION_ACTION)) {
             final RegionedPushdownFilterContext ctx = (RegionedPushdownFilterContext) filterContext;
             final TableLocation tableLocation = getColumnLocation().map(ColumnLocation::getTableLocation).orElse(null);
-            if (tableLocation == null || (ctx.isMatchFilter() && !ctx.isRangeFilter())) {
+            if (tableLocation == null || (!ctx.isMatchFilter() && !ctx.isRangeFilter())) {
                 return PushdownResult.UNSUPPORTED_ACTION_COST;
             }
-            // Only range and match filers can benefit from sorted column data.
+            // Only range and match filters can benefit from sorted column data.
             final SortColumn firstSortedColumn = tableLocation.getSortedColumns().isEmpty()
                     ? null
                     : tableLocation.getSortedColumns().get(0);
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionInt.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionInt.java
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionLong.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionLong.java
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionObject.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionObject.java
diff --git a/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionShort.java b/extensions/parquet/table/src/main/java/io/deephaven/parquet/table/region/ParquetColumnRegionShort.java
diff --git a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/ParquetTableReadWriteTest.java b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/ParquetTableReadWriteTest.java
