Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: DH-18471 update_by min/max functions to return original-typed output columns #6629

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,8 @@ private UpdateByOperator makeCumMinMaxOperator(MatchPair pair, TableDefinition t

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

return new RollingGroupOperator(pairs, affectingColumns,
rg.revWindowScale().timestampCol(),
prevWindowScaleUnits, fwdWindowScaleUnits, tableDef);
prevWindowScaleUnits, fwdWindowScaleUnits);
}

private UpdateByOperator makeRollingAvgOperator(@NotNull final MatchPair pair,
Expand Down Expand Up @@ -1132,7 +1134,7 @@ private UpdateByOperator makeRollingMinMaxOperator(@NotNull MatchPair pair,
} else if (csType == long.class || csType == Long.class || isTimeType(csType)) {
return new LongRollingMinMaxOperator(pair, affectingColumns,
rmm.revWindowScale().timestampCol(),
prevWindowScaleUnits, fwdWindowScaleUnits, rmm.isMax());
prevWindowScaleUnits, fwdWindowScaleUnits, rmm.isMax(), csType);
} else if (csType == float.class || csType == Float.class) {
return new FloatRollingMinMaxOperator(pair, affectingColumns,
rmm.revWindowScale().timestampCol(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
//
// @formatter:off
package io.deephaven.engine.table.impl.updateby.minmax;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
//
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
package io.deephaven.engine.table.impl.updateby.minmax;

import io.deephaven.base.verify.Assert;
import io.deephaven.chunk.Chunk;
import io.deephaven.chunk.CharChunk;
import io.deephaven.chunk.attributes.Values;
import io.deephaven.engine.table.impl.MatchPair;
import io.deephaven.engine.table.impl.updateby.UpdateByOperator;
import io.deephaven.engine.table.impl.updateby.internal.BaseCharUpdateByOperator;
import org.jetbrains.annotations.NotNull;

import static io.deephaven.util.QueryConstants.*;

public class CharCumMinMaxOperator extends BaseCharUpdateByOperator {
private final boolean isMax;

// region extra-fields
// endregion extra-fields

protected class Context extends BaseCharUpdateByOperator.Context {
public CharChunk<? extends Values> charValueChunk;

protected Context(final int chunkSize) {
super(chunkSize);
}

@Override
public void setValueChunks(@NotNull final Chunk<? extends Values>[] valueChunks) {
charValueChunk = valueChunks[0].asCharChunk();
}

@Override
public void push(int pos, int count) {
Assert.eq(count, "push count", 1);

final char val = charValueChunk.get(pos);

if (curVal == NULL_CHAR) {
curVal = val;
} else if (val != NULL_CHAR) {
if ((isMax && val > curVal) ||
(!isMax && val < curVal)) {
curVal = val;
}
}
}
}

public CharCumMinMaxOperator(
@NotNull final MatchPair pair,
final boolean isMax
// region extra-constructor-args
// endregion extra-constructor-args
) {
super(pair, new String[] {pair.rightColumn});
this.isMax = isMax;
// region constructor
// endregion constructor
}

@Override
public UpdateByOperator copy() {
return new CharCumMinMaxOperator(
pair,
isMax
// region extra-copy-args
// endregion extra-copy-args
);
}

@NotNull
@Override
public UpdateByOperator.Context makeUpdateContext(final int affectedChunkSize, final int influencerChunkSize) {
return new Context(affectedChunkSize);
}

// region extra-methods
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
//
// @formatter:off
package io.deephaven.engine.table.impl.updateby.minmax;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
//
// @formatter:off
package io.deephaven.engine.table.impl.updateby.minmax;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
//
// @formatter:off
package io.deephaven.engine.table.impl.updateby.minmax;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
// ****** Edit ShortCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
//
// @formatter:off
package io.deephaven.engine.table.impl.updateby.minmax;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
//
// Copyright (c) 2016-2025 Deephaven Data Labs and Patent Pending
//
// ****** AUTO-GENERATED CLASS - DO NOT EDIT MANUALLY
// ****** Edit CharCumMinMaxOperator and run "./gradlew replicateUpdateBy" to regenerate
//
// @formatter:off
package io.deephaven.engine.table.impl.updateby.minmax;

import io.deephaven.base.verify.Assert;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,7 @@ public RollingGroupOperator(
@NotNull final String[] affectingColumns,
@Nullable final String timestampColumnName,
final long reverseWindowScaleUnits,
final long forwardWindowScaleUnits,
@NotNull final TableDefinition tableDef) {
final long forwardWindowScaleUnits) {
super(pairs[0], affectingColumns, timestampColumnName, reverseWindowScaleUnits, forwardWindowScaleUnits, true);

this.pairs = pairs;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,7 @@ public UpdateByOperator copy() {
// endregion extra-copy-args
);
}

// region extra-methods
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,7 @@ public UpdateByOperator copy() {
// endregion extra-copy-args
);
}

// region extra-methods
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,7 @@ public UpdateByOperator copy() {
// endregion extra-copy-args
);
}

// region extra-methods
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,7 @@ public UpdateByOperator copy() {
// endregion extra-copy-args
);
}

// region extra-methods
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,7 @@ public UpdateByOperator copy() {
// endregion extra-copy-args
);
}

// region extra-methods
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,13 @@
// @formatter:off
package io.deephaven.engine.table.impl.updateby.rollingminmax;

import java.time.Instant;
import java.util.Map;
import java.util.Collections;

import io.deephaven.engine.table.ColumnSource;
import io.deephaven.engine.table.impl.sources.ReinterpretUtils;

import io.deephaven.base.ringbuffer.AggregatingLongRingBuffer;
import io.deephaven.base.verify.Assert;
import io.deephaven.chunk.LongChunk;
Expand All @@ -24,6 +31,7 @@ public class LongRollingMinMaxOperator extends BaseLongUpdateByOperator {
private final boolean isMax;
private static final int BUFFER_INITIAL_CAPACITY = 128;
// region extra-fields
private final Class<?> type;
// endregion extra-fields

protected class Context extends BaseLongUpdateByOperator.Context {
Expand Down Expand Up @@ -147,11 +155,13 @@ public LongRollingMinMaxOperator(
final long forwardWindowScaleUnits,
final boolean isMax
// region extra-constructor-args
,@NotNull final Class<?> type
// endregion extra-constructor-args
) {
super(pair, affectingColumns, timestampColumnName, reverseWindowScaleUnits, forwardWindowScaleUnits, true);
this.isMax = isMax;
// region constructor
this.type = type;
// endregion constructor
}

Expand All @@ -165,7 +175,22 @@ public UpdateByOperator copy() {
forwardWindowScaleUnits,
isMax
// region extra-copy-args
, type
// endregion extra-copy-args
);
}

// region extra-methods
@NotNull
@Override
public Map<String, ColumnSource<?>> getOutputColumns() {
final ColumnSource<?> actualOutput;
if(type == Instant.class) {
actualOutput = ReinterpretUtils.longToInstantSource(outputSource);
} else {
actualOutput = outputSource;
}
return Collections.singletonMap(pair.leftColumn, actualOutput);
}
// endregion extra-methods
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,7 @@ public UpdateByOperator copy() {
// endregion extra-copy-args
);
}

// region extra-methods
// endregion extra-methods
}
Loading