Ryan's feedback

deephaven · Feb 22, 2023 · a3efc5d · a3efc5d
1 parent 950f695
commit a3efc5d
Show file tree

Hide file tree

Showing 53 changed files with 244 additions and 294 deletions.
diff --git a/Util/src/main/java/io/deephaven/util/codec/ZonedDateTimeCodec.java b/Util/src/main/java/io/deephaven/util/codec/ZonedDateTimeCodec.java
@@ -6,6 +6,7 @@
 import org.jetbrains.annotations.Nullable;
 
 import java.nio.ByteBuffer;
+import java.nio.charset.StandardCharsets;
 import java.time.Instant;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
@@ -29,7 +30,7 @@ public byte[] encode(@Nullable ZonedDateTime input) {
 
         final String zone = input.getZone().getId();
         bb.putInt(zone.length());
-        bb.put(zone.getBytes());
+        bb.put(zone.getBytes(StandardCharsets.UTF_8));
 
         return buf;
     }
@@ -47,7 +48,7 @@ public ZonedDateTime decode(@NotNull byte[] input, int offset, int length) {
 
         final byte[] zidBytes = new byte[zidLen];
         buf.get(zidBytes, 0, zidLen);
-        final String zid = new String(zidBytes);
+        final String zid = new String(zidBytes, StandardCharsets.UTF_8);
 
         return ZonedDateTime.ofInstant(Instant.ofEpochSecond(0, nanos), ZoneId.of(zid));
     }
@@ -76,7 +77,7 @@ private static int computeSize(@NotNull ZonedDateTime val) {
         return Long.BYTES + Integer.BYTES + val.getZone().getId().length();
     }
 
-    // Sadly, this is copied from DBTimeUtils since that lives in the DB package and this cannot.
+    // Sadly, this is copied from DateTimeUtils, since we cannot depend on the engine-time package.
     private static long toEpochNano(@Nullable final ZonedDateTime value) {
         if (value == null) {
             return QueryConstants.NULL_LONG;
@@ -91,6 +92,6 @@ private static long safeComputeNanos(long epochSecond, long nanoOfSecond) {
                     "Numeric overflow detected during conversion of " + epochSecond + " to nanoseconds");
         }
 
-        return epochSecond * 1_000_000_000L + nanoOfSecond;
+        return Math.addExact(epochSecond * 1_000_000_000L, nanoOfSecond);
     }
 }
diff --git a/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java b/engine/api/src/main/java/io/deephaven/engine/table/ColumnSource.java
@@ -12,8 +12,6 @@
 import org.jetbrains.annotations.NotNull;
 
 import javax.annotation.OverridingMethodsMustInvokeSuper;
-import java.util.Collections;
-import java.util.List;
 import java.util.Map;
 
 /**

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/ColumnSourceGetDefaults.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/ColumnSourceGetDefaults.java
@@ -409,8 +409,8 @@ default DateTime get(final long rowKey) {
     public interface ForLongAsInstant extends LongBacked<Instant> {
         @Nullable
         @Override
-        default Instant get(long index) {
-            return DateTimeUtils.makeInstant(getLong(index));
+        default Instant get(long rowKey) {
+            return DateTimeUtils.makeInstant(getLong(rowKey));
         }
     }
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinShiftState.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinShiftState.java
@@ -61,20 +61,20 @@ public boolean leftOuterJoin() {
         return leftOuterJoin;
     }
 
-    public long getShifted(long index) {
-        return index >> getNumShiftBits();
+    public long getShifted(long rowKey) {
+        return rowKey >> getNumShiftBits();
     }
 
-    public long getPrevShifted(long index) {
-        return index >> getPrevNumShiftBits();
+    public long getPrevShifted(long rowKey) {
+        return rowKey >> getPrevNumShiftBits();
     }
 
-    public long getMasked(long index) {
-        return index & getMask();
+    public long getMasked(long rowKey) {
+        return rowKey & getMask();
     }
 
-    public long getPrevMasked(long index) {
-        return index & getPrevMask();
+    public long getPrevMasked(long rowKey) {
+        return rowKey & getPrevMask();
     }
 
     private long getMask() {

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinStateManager.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinStateManager.java
@@ -10,10 +10,10 @@ public interface CrossJoinStateManager {
 
     TrackingRowSet getRightRowSetFromPrevLeftRow(long leftIndex);
 
-    long getShifted(long index);
-    long getPrevShifted(long index);
-    long getMasked(long index);
-    long getPrevMasked(long index);
+    long getShifted(long rowKey);
+    long getPrevShifted(long rowKey);
+    long getMasked(long rowKey);
+    long getPrevMasked(long rowKey);
 
     /**
      * If our result is a leftOuterJoin, which means that for each unmatched left row we produce one row of RHS output,

diff --git a/...ne/table/src/main/java/io/deephaven/engine/table/impl/MutableColumnSourceGetDefaults.java b/...ne/table/src/main/java/io/deephaven/engine/table/impl/MutableColumnSourceGetDefaults.java
@@ -411,8 +411,8 @@ default DateTime getPrev(final long rowKey) {
     public interface ForLongAsInstant extends ColumnSourceGetDefaults.ForLongAsInstant, LongBacked<Instant> {
         @Nullable
         @Override
-        default Instant getPrev(long index) {
-            return DateTimeUtils.makeInstant(getPrevLong(index));
+        default Instant getPrev(long rowKey) {
+            return DateTimeUtils.makeInstant(getPrevLong(rowKey));
         }
     }
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationProcessor.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationProcessor.java
@@ -95,6 +95,7 @@
 import io.deephaven.engine.table.impl.by.ssmcountdistinct.unique.ShortRollupUniqueOperator;
 import io.deephaven.engine.table.impl.by.ssmminmax.SsmChunkedMinMaxOperator;
 import io.deephaven.engine.table.impl.by.ssmpercentile.SsmChunkedPercentileOperator;
+import io.deephaven.engine.table.impl.sources.ConvertableTimeSource;
 import io.deephaven.engine.table.impl.sources.ReinterpretUtils;
 import io.deephaven.engine.table.impl.ssms.SegmentedSortedMultiSet;
 import io.deephaven.engine.table.impl.util.freezeby.FreezeByCountOperator;
@@ -108,6 +109,8 @@
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -235,7 +238,7 @@ public static AggregationContextFactory forTreeSourceRowLookup() {
 
     /**
      * Create a trivial {@link AggregationContextFactory} to implement {@link Table#selectDistinct select distinct}.
-     * 
+     *
      * @return The {@link AggregationContextFactory}
      */
     public static AggregationContextFactory forSelectDistinct() {
@@ -507,6 +510,9 @@ final void addMinOrMaxOperator(final boolean isMin, @NotNull final String inputN
                     return;
                 }
             }
+            if (rawInputSource instanceof ConvertableTimeSource.Zoned) {
+                ZoneId id = ((ConvertableTimeSource.Zoned) rawInputSource).getZone();
+            }
             addOperator(makeMinOrMaxOperator(type, resultName, isMin, isAddOnly || isStream), inputSource, inputName);
         }
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/ArraySourceHelper.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/ArraySourceHelper.java
@@ -195,12 +195,12 @@ final void startTrackingPrev(int numBlocks) {
      * @return true if the inheritor should return a value from its "prev" data structure; false if it should return a
      *         value from its "current" data structure.
      */
-    final boolean shouldUsePrevious(final long index) {
+    final boolean shouldUsePrevious(final long rowKey) {
         if (prevFlusher == null) {
             return false;
         }
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final int indexWithinInUse = indexWithinBlock >> LOG_INUSE_BITSET_SIZE;
         final long maskWithinInUse = 1L << (indexWithinBlock & IN_USE_MASK);
         final long[] inUse = prevInUse[blockIndex];

diff --git a/...ne/table/src/main/java/io/deephaven/engine/table/impl/sources/BitMaskingColumnSource.java b/...ne/table/src/main/java/io/deephaven/engine/table/impl/sources/BitMaskingColumnSource.java
@@ -398,9 +398,9 @@ private void ensureMaskedKeysInitialized(@NotNull final CrossJoinShiftState shif
                 }
 
                 maskedKeys.setSize(0);
-                rowSequence.forAllRowKeys((final long indexKey) -> {
+                rowSequence.forAllRowKeys((final long rowKey) -> {
                     final long innerIndexKey =
-                            usePrev ? shiftState.getPrevMasked(indexKey) : shiftState.getMasked(indexKey);
+                            usePrev ? shiftState.getPrevMasked(rowKey) : shiftState.getMasked(rowKey);
                     maskedKeys.add(innerIndexKey);
                 });
 

diff --git a/...e/table/src/main/java/io/deephaven/engine/table/impl/sources/BitShiftingColumnSource.java b/...e/table/src/main/java/io/deephaven/engine/table/impl/sources/BitShiftingColumnSource.java
@@ -404,10 +404,10 @@ private void ensureKeysAndLengthsInitialized(@NotNull final CrossJoinShiftState
                 final MutableInt currentRunPosition = new MutableInt(0);
                 final MutableLong currentRunInnerIndexKey = new MutableLong(RowSequence.NULL_ROW_KEY);
 
-                rowSequence.forAllRowKeys((final long indexKey) -> {
+                rowSequence.forAllRowKeys((final long rowKey) -> {
                     final long lastInnerIndexKey = currentRunInnerIndexKey.longValue();
                     final long innerIndexKey =
-                            usePrev ? shiftState.getPrevShifted(indexKey) : shiftState.getShifted(indexKey);
+                            usePrev ? shiftState.getPrevShifted(rowKey) : shiftState.getShifted(rowKey);
                     if (innerIndexKey != lastInnerIndexKey) {
                         if (lastInnerIndexKey != RowSequence.NULL_ROW_KEY) {
                             uniqueIndices.set(currentRunPosition.intValue(), lastInnerIndexKey);

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/BooleanArraySource.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/BooleanArraySource.java
@@ -140,23 +140,23 @@ public Boolean get(long rowKey) {
         return BooleanUtils.byteAsBoolean(getByte(rowKey));
     }
 
-    public Boolean getUnsafe(long index) {
-        return BooleanUtils.byteAsBoolean(getByteUnsafe(index));
+    public Boolean getUnsafe(long rowKey) {
+        return BooleanUtils.byteAsBoolean(getByteUnsafe(rowKey));
     }
 
-    public final Boolean getAndSetUnsafe(long index, Boolean newValue) {
-        return BooleanUtils.byteAsBoolean(getAndSetUnsafe(index, BooleanUtils.booleanAsByte(newValue)));
+    public final Boolean getAndSetUnsafe(long rowKey, Boolean newValue) {
+        return BooleanUtils.byteAsBoolean(getAndSetUnsafe(rowKey, BooleanUtils.booleanAsByte(newValue)));
     }
 
-    public final byte getAndSetUnsafe(long index, byte newValue) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final byte getAndSetUnsafe(long rowKey, byte newValue) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final byte oldValue = blocks[blockIndex][indexWithinBlock];
         // not a perfect comparison, but very cheap
         if (oldValue == newValue) {
             return oldValue;
         }
-        if (shouldRecordPrevious(index, prevBlocks, recycler)) {
+        if (shouldRecordPrevious(rowKey, prevBlocks, recycler)) {
             prevBlocks[blockIndex][indexWithinBlock] = oldValue;
         }
         blocks[blockIndex][indexWithinBlock] = newValue;
@@ -171,9 +171,9 @@ public byte getByte(long rowKey) {
         return getByteUnsafe(rowKey);
     }
 
-    private byte getByteUnsafe(long index) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    private byte getByteUnsafe(long rowKey) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         return blocks[blockIndex][indexWithinBlock];
     }
 

diff --git a/.../table/src/main/java/io/deephaven/engine/table/impl/sources/BooleanSparseArraySource.java b/.../table/src/main/java/io/deephaven/engine/table/impl/sources/BooleanSparseArraySource.java
@@ -469,7 +469,7 @@ public void prepareForParallelPopulation(final RowSet changedRows) {
      * @return true if the inheritor should return a value from its "prev" data structure; false if it should return a
      * value from its "current" data structure.
      */
-    private boolean shouldUsePrevious(final long index) {
+    private boolean shouldUsePrevious(final long rowKey) {
         if (prevFlusher == null) {
             return false;
         }
@@ -478,12 +478,12 @@ private boolean shouldUsePrevious(final long index) {
             return false;
         }
 
-        final long [] inUse = prevInUse.getInnermostBlockByKeyOrNull(index);
+        final long [] inUse = prevInUse.getInnermostBlockByKeyOrNull(rowKey);
         if (inUse == null) {
             return false;
         }
 
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final int indexWithinInUse = indexWithinBlock >> LOG_INUSE_BITSET_SIZE;
         final long maskWithinInUse = 1L << (indexWithinBlock & IN_USE_MASK);
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/ByteArraySource.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/ByteArraySource.java
@@ -154,18 +154,18 @@ public final byte getByte(long rowKey) {
         return getUnsafe(rowKey);
     }
 
-    public final byte getUnsafe(long index) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final byte getUnsafe(long rowKey) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         return blocks[blockIndex][indexWithinBlock];
     }
 
-    public final byte getAndSetUnsafe(long index, byte newValue) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final byte getAndSetUnsafe(long rowKey, byte newValue) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final byte oldValue = blocks[blockIndex][indexWithinBlock];
         if (!ByteComparisons.eq(oldValue, newValue)) {
-            if (shouldRecordPrevious(index, prevBlocks, recycler)) {
+            if (shouldRecordPrevious(rowKey, prevBlocks, recycler)) {
                 prevBlocks[blockIndex][indexWithinBlock] = oldValue;
             }
             blocks[blockIndex][indexWithinBlock] = newValue;

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/ByteSparseArraySource.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/ByteSparseArraySource.java
@@ -463,7 +463,7 @@ public void prepareForParallelPopulation(final RowSet changedRows) {
      * @return true if the inheritor should return a value from its "prev" data structure; false if it should return a
      * value from its "current" data structure.
      */
-    private boolean shouldUsePrevious(final long index) {
+    private boolean shouldUsePrevious(final long rowKey) {
         if (prevFlusher == null) {
             return false;
         }
@@ -472,12 +472,12 @@ private boolean shouldUsePrevious(final long index) {
             return false;
         }
 
-        final long [] inUse = prevInUse.getInnermostBlockByKeyOrNull(index);
+        final long [] inUse = prevInUse.getInnermostBlockByKeyOrNull(rowKey);
         if (inUse == null) {
             return false;
         }
 
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final int indexWithinInUse = indexWithinBlock >> LOG_INUSE_BITSET_SIZE;
         final long maskWithinInUse = 1L << (indexWithinBlock & IN_USE_MASK);
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/CharacterArraySource.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/CharacterArraySource.java
@@ -149,18 +149,18 @@ public final char getChar(long rowKey) {
         return getUnsafe(rowKey);
     }
 
-    public final char getUnsafe(long index) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final char getUnsafe(long rowKey) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         return blocks[blockIndex][indexWithinBlock];
     }
 
-    public final char getAndSetUnsafe(long index, char newValue) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final char getAndSetUnsafe(long rowKey, char newValue) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final char oldValue = blocks[blockIndex][indexWithinBlock];
         if (!CharComparisons.eq(oldValue, newValue)) {
-            if (shouldRecordPrevious(index, prevBlocks, recycler)) {
+            if (shouldRecordPrevious(rowKey, prevBlocks, recycler)) {
                 prevBlocks[blockIndex][indexWithinBlock] = oldValue;
             }
             blocks[blockIndex][indexWithinBlock] = newValue;

diff --git a/...able/src/main/java/io/deephaven/engine/table/impl/sources/CharacterSparseArraySource.java b/...able/src/main/java/io/deephaven/engine/table/impl/sources/CharacterSparseArraySource.java
@@ -458,7 +458,7 @@ public void prepareForParallelPopulation(final RowSet changedRows) {
      * @return true if the inheritor should return a value from its "prev" data structure; false if it should return a
      * value from its "current" data structure.
      */
-    private boolean shouldUsePrevious(final long index) {
+    private boolean shouldUsePrevious(final long rowKey) {
         if (prevFlusher == null) {
             return false;
         }
@@ -467,12 +467,12 @@ private boolean shouldUsePrevious(final long index) {
             return false;
         }
 
-        final long [] inUse = prevInUse.getInnermostBlockByKeyOrNull(index);
+        final long [] inUse = prevInUse.getInnermostBlockByKeyOrNull(rowKey);
         if (inUse == null) {
             return false;
         }
 
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final int indexWithinInUse = indexWithinBlock >> LOG_INUSE_BITSET_SIZE;
         final long maskWithinInUse = 1L << (indexWithinBlock & IN_USE_MASK);
 

diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/DoubleArraySource.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/sources/DoubleArraySource.java
@@ -154,18 +154,18 @@ public final double getDouble(long rowKey) {
         return getUnsafe(rowKey);
     }
 
-    public final double getUnsafe(long index) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final double getUnsafe(long rowKey) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         return blocks[blockIndex][indexWithinBlock];
     }
 
-    public final double getAndSetUnsafe(long index, double newValue) {
-        final int blockIndex = (int) (index >> LOG_BLOCK_SIZE);
-        final int indexWithinBlock = (int) (index & INDEX_MASK);
+    public final double getAndSetUnsafe(long rowKey, double newValue) {
+        final int blockIndex = (int) (rowKey >> LOG_BLOCK_SIZE);
+        final int indexWithinBlock = (int) (rowKey & INDEX_MASK);
         final double oldValue = blocks[blockIndex][indexWithinBlock];
         if (!DoubleComparisons.eq(oldValue, newValue)) {
-            if (shouldRecordPrevious(index, prevBlocks, recycler)) {
+            if (shouldRecordPrevious(rowKey, prevBlocks, recycler)) {
                 prevBlocks[blockIndex][indexWithinBlock] = oldValue;
             }
             blocks[blockIndex][indexWithinBlock] = newValue;