Skip to content

Commit 1feacf4

Browse files
lbooker42lbooker42
authored
fix!: DH-18471 update_by min/max functions to return original-typed output columns (#6629)
Add'l work performed: - Added `char` support to `CumMin`/`CumMax` and expanded tests - Added PartionedTable proxy tests
1 parent 3f9f781 commit 1feacf4

20 files changed

+410
-38
lines changed

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/UpdateByOperatorFactory.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,8 @@ private UpdateByOperator makeCumMinMaxOperator(MatchPair pair, TableDefinition t
856856

857857
if (csType == byte.class || csType == Byte.class) {
858858
return new ByteCumMinMaxOperator(pair, isMax, NULL_BYTE);
859+
} else if (csType == char.class || csType == Character.class) {
860+
return new CharCumMinMaxOperator(pair, isMax);
859861
} else if (csType == short.class || csType == Short.class) {
860862
return new ShortCumMinMaxOperator(pair, isMax);
861863
} else if (csType == int.class || csType == Integer.class) {
@@ -1033,7 +1035,7 @@ private UpdateByOperator makeRollingGroupOperator(@NotNull final MatchPair[] pai
10331035

10341036
return new RollingGroupOperator(pairs, affectingColumns,
10351037
rg.revWindowScale().timestampCol(),
1036-
prevWindowScaleUnits, fwdWindowScaleUnits, tableDef);
1038+
prevWindowScaleUnits, fwdWindowScaleUnits);
10371039
}
10381040

10391041
private UpdateByOperator makeRollingAvgOperator(@NotNull final MatchPair pair,
@@ -1132,7 +1134,7 @@ private UpdateByOperator makeRollingMinMaxOperator(@NotNull MatchPair pair,
11321134
} else if (csType == long.class || csType == Long.class || isTimeType(csType)) {
11331135
return new LongRollingMinMaxOperator(pair, affectingColumns,
11341136
rmm.revWindowScale().timestampCol(),
1135-
prevWindowScaleUnits, fwdWindowScaleUnits, rmm.isMax());
1137+
prevWindowScaleUnits, fwdWindowScaleUnits, rmm.isMax(), csType);
11361138
} else if (csType == float.class || csType == Float.class) {
11371139
return new FloatRollingMinMaxOperator(pair, affectingColumns,
11381140
rmm.revWindowScale().timestampCol(),

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/ByteCumMinMaxOperator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
33
//
44
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
5-
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
5+
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
66
//
77
// @formatter:off
88
package io.deephaven.engine.table.impl.updateby.minmax;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/CharCumMinMaxOperator.java

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
//
2+
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
3+
//
4+
package io.deephaven.engine.table.impl.updateby.minmax;
5+
6+
import io.deephaven.base.verify.Assert;
7+
import io.deephaven.chunk.Chunk;
8+
import io.deephaven.chunk.CharChunk;
9+
import io.deephaven.chunk.attributes.Values;
10+
import io.deephaven.engine.table.impl.MatchPair;
11+
import io.deephaven.engine.table.impl.updateby.UpdateByOperator;
12+
import io.deephaven.engine.table.impl.updateby.internal.BaseCharUpdateByOperator;
13+
import org.jetbrains.annotations.NotNull;
14+
15+
import static io.deephaven.util.QueryConstants.*;
16+
17+
public class CharCumMinMaxOperator extends BaseCharUpdateByOperator {
18+
private final boolean isMax;
19+
20+
// region extra-fields
21+
// endregion extra-fields
22+
23+
protected class Context extends BaseCharUpdateByOperator.Context {
24+
public CharChunk<? extends Values> charValueChunk;
25+
26+
protected Context(final int chunkSize) {
27+
super(chunkSize);
28+
}
29+
30+
@Override
31+
public void setValueChunks(@NotNull final Chunk<? extends Values>[] valueChunks) {
32+
charValueChunk = valueChunks[0].asCharChunk();
33+
}
34+
35+
@Override
36+
public void push(int pos, int count) {
37+
Assert.eq(count, "push count", 1);
38+
39+
final char val = charValueChunk.get(pos);
40+
41+
if (curVal == NULL_CHAR) {
42+
curVal = val;
43+
} else if (val != NULL_CHAR) {
44+
if ((isMax && val > curVal) ||
45+
(!isMax && val < curVal)) {
46+
curVal = val;
47+
}
48+
}
49+
}
50+
}
51+
52+
public CharCumMinMaxOperator(
53+
@NotNull final MatchPair pair,
54+
final boolean isMax
55+
// region extra-constructor-args
56+
// endregion extra-constructor-args
57+
) {
58+
super(pair, new String[] {pair.rightColumn});
59+
this.isMax = isMax;
60+
// region constructor
61+
// endregion constructor
62+
}
63+
64+
@Override
65+
public UpdateByOperator copy() {
66+
return new CharCumMinMaxOperator(
67+
pair,
68+
isMax
69+
// region extra-copy-args
70+
// endregion extra-copy-args
71+
);
72+
}
73+
74+
@NotNull
75+
@Override
76+
public UpdateByOperator.Context makeUpdateContext(final int affectedChunkSize, final int influencerChunkSize) {
77+
return new Context(affectedChunkSize);
78+
}
79+
80+
// region extra-methods
81+
// endregion extra-methods
82+
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/DoubleCumMinMaxOperator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
33
//
44
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
5-
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
5+
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
66
//
77
// @formatter:off
88
package io.deephaven.engine.table.impl.updateby.minmax;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/FloatCumMinMaxOperator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
33
//
44
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
5-
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
5+
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
66
//
77
// @formatter:off
88
package io.deephaven.engine.table.impl.updateby.minmax;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/IntCumMinMaxOperator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
33
//
44
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
5-
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
5+
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
66
//
77
// @formatter:off
88
package io.deephaven.engine.table.impl.updateby.minmax;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/LongCumMinMaxOperator.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
33
//
44
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
5-
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
5+
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
66
//
77
// @formatter:off
88
package io.deephaven.engine.table.impl.updateby.minmax;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/minmax/ShortCumMinMaxOperator.java

+4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
//
22
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
33
//
4+
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
5+
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
6+
//
7+
// @formatter:off
48
package io.deephaven.engine.table.impl.updateby.minmax;
59

610
import io.deephaven.base.verify.Assert;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollinggroup/RollingGroupOperator.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -234,8 +234,7 @@ public RollingGroupOperator(
234234
@NotNull final String[] affectingColumns,
235235
@Nullable final String timestampColumnName,
236236
final long reverseWindowScaleUnits,
237-
final long forwardWindowScaleUnits,
238-
@NotNull final TableDefinition tableDef) {
237+
final long forwardWindowScaleUnits) {
239238
super(pairs[0], affectingColumns, timestampColumnName, reverseWindowScaleUnits, forwardWindowScaleUnits, true);
240239

241240
this.pairs = pairs;

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/ByteRollingMinMaxOperator.java

+3
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,7 @@ public UpdateByOperator copy() {
168168
// endregion extra-copy-args
169169
);
170170
}
171+
172+
// region extra-methods
173+
// endregion extra-methods
171174
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/CharRollingMinMaxOperator.java

+3
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,7 @@ public UpdateByOperator copy() {
164164
// endregion extra-copy-args
165165
);
166166
}
167+
168+
// region extra-methods
169+
// endregion extra-methods
167170
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/DoubleRollingMinMaxOperator.java

+3
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,7 @@ public UpdateByOperator copy() {
168168
// endregion extra-copy-args
169169
);
170170
}
171+
172+
// region extra-methods
173+
// endregion extra-methods
171174
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/FloatRollingMinMaxOperator.java

+3
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,7 @@ public UpdateByOperator copy() {
168168
// endregion extra-copy-args
169169
);
170170
}
171+
172+
// region extra-methods
173+
// endregion extra-methods
171174
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/IntRollingMinMaxOperator.java

+3
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,7 @@ public UpdateByOperator copy() {
168168
// endregion extra-copy-args
169169
);
170170
}
171+
172+
// region extra-methods
173+
// endregion extra-methods
171174
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/LongRollingMinMaxOperator.java

+25
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@
77
// @formatter:off
88
package io.deephaven.engine.table.impl.updateby.rollingminmax;
99

10+
import java.time.Instant;
11+
import java.util.Map;
12+
import java.util.Collections;
13+
14+
import io.deephaven.engine.table.ColumnSource;
15+
import io.deephaven.engine.table.impl.sources.ReinterpretUtils;
16+
1017
import io.deephaven.base.ringbuffer.AggregatingLongRingBuffer;
1118
import io.deephaven.base.verify.Assert;
1219
import io.deephaven.chunk.LongChunk;
@@ -24,6 +31,7 @@ public class LongRollingMinMaxOperator extends BaseLongUpdateByOperator {
2431
private final boolean isMax;
2532
private static final int BUFFER_INITIAL_CAPACITY = 128;
2633
// region extra-fields
34+
private final Class<?> type;
2735
// endregion extra-fields
2836

2937
protected class Context extends BaseLongUpdateByOperator.Context {
@@ -147,11 +155,13 @@ public LongRollingMinMaxOperator(
147155
final long forwardWindowScaleUnits,
148156
final boolean isMax
149157
// region extra-constructor-args
158+
,@NotNull final Class<?> type
150159
// endregion extra-constructor-args
151160
) {
152161
super(pair, affectingColumns, timestampColumnName, reverseWindowScaleUnits, forwardWindowScaleUnits, true);
153162
this.isMax = isMax;
154163
// region constructor
164+
this.type = type;
155165
// endregion constructor
156166
}
157167

@@ -165,7 +175,22 @@ public UpdateByOperator copy() {
165175
forwardWindowScaleUnits,
166176
isMax
167177
// region extra-copy-args
178+
, type
168179
// endregion extra-copy-args
169180
);
170181
}
182+
183+
// region extra-methods
184+
@NotNull
185+
@Override
186+
public Map<String, ColumnSource<?>> getOutputColumns() {
187+
final ColumnSource<?> actualOutput;
188+
if(type == Instant.class) {
189+
actualOutput = ReinterpretUtils.longToInstantSource(outputSource);
190+
} else {
191+
actualOutput = outputSource;
192+
}
193+
return Collections.singletonMap(pair.leftColumn, actualOutput);
194+
}
195+
// endregion extra-methods
171196
}

‎engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/rollingminmax/ShortRollingMinMaxOperator.java

+3
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,7 @@ public UpdateByOperator copy() {
168168
// endregion extra-copy-args
169169
);
170170
}
171+
172+
// region extra-methods
173+
// endregion extra-methods
171174
}

0 commit comments

Comments
 (0)