kgyrtkirk commented on code in PR #16741: URL: https://github.com/apache/druid/pull/16741#discussion_r1682185223
########## processing/src/main/java/org/apache/druid/query/operator/window/WindowFrame.java: ########## @@ -21,159 +21,226 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; -import org.apache.druid.query.operator.ColumnWithDirection; +import org.apache.druid.annotations.SubclassesMustOverrideEqualsAndHashCode; + +import javax.annotation.Nullable; -import java.util.Collections; import java.util.List; import java.util.Objects; -import java.util.stream.Collectors; -public class WindowFrame +@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type") +@JsonSubTypes(value = { + @JsonSubTypes.Type(name = "unbounded", value = WindowFrame.Unbounded.class), + @JsonSubTypes.Type(name = "rows", value = WindowFrame.Rows.class), + @JsonSubTypes.Type(name = "groups", value = WindowFrame.Groups.class), +}) +@SubclassesMustOverrideEqualsAndHashCode +public interface WindowFrame { - public static WindowFrame unbounded() + static WindowFrame unbounded() { - return new WindowFrame(PeerType.ROWS, true, 0, true, 0, null); + return new WindowFrame.Unbounded(); } - @SuppressWarnings("unused") - public enum PeerType + static Rows rows(Integer lowerOffset, Integer upperOffset) { - ROWS, - RANGE + return new WindowFrame.Rows(lowerOffset, upperOffset); } - // Will likely need to add the order by columns to also be able to deal with RANGE peer type. - private final PeerType peerType; - private final boolean lowerUnbounded; - private final int lowerOffset; - private final boolean upperUnbounded; - private final int upperOffset; - private final List<ColumnWithDirection> orderBy; - - @JsonCreator - public WindowFrame( - @JsonProperty("peerType") PeerType peerType, - @JsonProperty("lowUnbounded") boolean lowerUnbounded, - @JsonProperty("lowOffset") int lowerOffset, - @JsonProperty("uppUnbounded") boolean upperUnbounded, - @JsonProperty("uppOffset") int upperOffset, Review Comment: that's kinda the main reason it's not yet rolled out - and its not on the wire; so we could still change it... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org