From 897c775f25350f2f114687885a6237706eb88d9c Mon Sep 17 00:00:00 2001 From: Max Zinal Date: Thu, 10 Jul 2025 12:26:49 +0300 Subject: [PATCH 01/17] support statistics reporting for topic writes --- .../java/tech/ydb/topic/write/WriteAck.java | 84 ++++++++++++++++--- .../tech/ydb/topic/write/impl/WriterImpl.java | 15 ++-- 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java index b7b082f15..eacd640be 100644 --- a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java +++ b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java @@ -1,5 +1,9 @@ package tech.ydb.topic.write; +import java.time.Duration; + +import tech.ydb.proto.topic.YdbTopic; + /** * @author Nikolay Perfilov */ @@ -7,11 +11,13 @@ public class WriteAck { private final long seqNo; private final State state; private final Details details; + private final Statistics statistics; - public WriteAck(long seqNo, State state, Details details) { + public WriteAck(long seqNo, State state, Details details, Statistics statistics) { this.seqNo = seqNo; this.state = state; this.details = details; + this.statistics = statistics; } public enum State { @@ -20,18 +26,6 @@ public enum State { WRITTEN_IN_TX } - public static class Details { - private final long offset; - - public Details(long offset) { - this.offset = offset; - } - - public long getOffset() { - return offset; - } - } - public long getSeqNo() { return seqNo; } @@ -47,4 +41,68 @@ public State getState() { public Details getDetails() { return details; } + + /** + * Obtain message write statistics + * @return {@link Statistics} with timings if statistics are available or null otherwise + */ + public Statistics getStatistics() { + return statistics; + } + + private static Duration convert(com.google.protobuf.Duration d) { + if (d == null) { + return Duration.ZERO; + } + return Duration.ofSeconds(d.getSeconds(), d.getNanos()); + } + + public static class Details { + private final long offset; + + public Details(long offset) { + this.offset = offset; + } + + public long getOffset() { + return offset; + } + } + + public static class Statistics { + private final Duration persistingTime; + private final Duration partitionQuotaWaitTime; + private final Duration topicQuotaWaitTime; + private final Duration maxQueueWaitTime; + private final Duration minQueueWaitTime; + + public Statistics(YdbTopic.StreamWriteMessage.WriteResponse.WriteStatistics src) { + this.persistingTime = convert(src.getPersistingTime()); + this.partitionQuotaWaitTime = convert(src.getPartitionQuotaWaitTime()); + this.topicQuotaWaitTime = convert(src.getTopicQuotaWaitTime()); + this.maxQueueWaitTime = convert(src.getMaxQueueWaitTime()); + this.minQueueWaitTime = convert(src.getMinQueueWaitTime()); + } + + public Duration getPersistingTime() { + return persistingTime; + } + + public Duration getPartitionQuotaWaitTime() { + return partitionQuotaWaitTime; + } + + public Duration getTopicQuotaWaitTime() { + return topicQuotaWaitTime; + } + + public Duration getMaxQueueWaitTime() { + return maxQueueWaitTime; + } + + public Duration getMinQueueWaitTime() { + return minQueueWaitTime; + } + } + } diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index 58733d676..13617ab76 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -421,6 +421,10 @@ private void onInitResponse(YdbTopic.StreamWriteMessage.InitResponse response) { private void onWriteResponse(YdbTopic.StreamWriteMessage.WriteResponse response) { List acks = response.getAcksList(); logger.debug("[{}] Received WriteResponse with {} WriteAcks", streamId, acks.size()); + WriteAck.Statistics statistics = null; + if (response.getWriteStatistics() != null) { + statistics = new WriteAck.Statistics(response.getWriteStatistics()); + } int inFlightFreed = 0; long bytesFreed = 0; for (YdbTopic.StreamWriteMessage.WriteResponse.WriteAck ack : acks) { @@ -433,7 +437,7 @@ private void onWriteResponse(YdbTopic.StreamWriteMessage.WriteResponse response) inFlightFreed++; bytesFreed += sentMessage.getSize(); sentMessages.remove(); - processWriteAck(sentMessage, ack); + processWriteAck(sentMessage, statistics, ack); break; } if (sentMessage.getSeqNo() < ack.getSeqNo()) { @@ -474,7 +478,7 @@ private void processMessage(YdbTopic.StreamWriteMessage.FromServer message) { } } - private void processWriteAck(EnqueuedMessage message, + private void processWriteAck(EnqueuedMessage message, WriteAck.Statistics statistics, YdbTopic.StreamWriteMessage.WriteResponse.WriteAck ack) { logger.debug("[{}] Received WriteAck with seqNo {} and status {}", streamId, ack.getSeqNo(), ack.getMessageWriteStatusCase()); @@ -482,12 +486,12 @@ private void processWriteAck(EnqueuedMessage message, switch (ack.getMessageWriteStatusCase()) { case WRITTEN: WriteAck.Details details = new WriteAck.Details(ack.getWritten().getOffset()); - resultAck = new WriteAck(ack.getSeqNo(), WriteAck.State.WRITTEN, details); + resultAck = new WriteAck(ack.getSeqNo(), WriteAck.State.WRITTEN, details, statistics); break; case SKIPPED: switch (ack.getSkipped().getReason()) { case REASON_ALREADY_WRITTEN: - resultAck = new WriteAck(ack.getSeqNo(), WriteAck.State.ALREADY_WRITTEN, null); + resultAck = new WriteAck(ack.getSeqNo(), WriteAck.State.ALREADY_WRITTEN, null, statistics); break; case REASON_UNSPECIFIED: default: @@ -497,7 +501,7 @@ private void processWriteAck(EnqueuedMessage message, } break; case WRITTEN_IN_TX: - resultAck = new WriteAck(ack.getSeqNo(), WriteAck.State.WRITTEN_IN_TX, null); + resultAck = new WriteAck(ack.getSeqNo(), WriteAck.State.WRITTEN_IN_TX, null, statistics); break; default: message.getFuture().completeExceptionally( @@ -519,5 +523,6 @@ private void closeDueToError(Status status, Throwable th) { protected void onStop() { logger.debug("[{}] Session {} onStop called", streamId, sessionId); } + } } From 4e117f79a8d1db4f1eb32dd2fd0aec6ce7e5a302 Mon Sep 17 00:00:00 2001 From: Max Zinal Date: Thu, 10 Jul 2025 12:42:39 +0300 Subject: [PATCH 02/17] removed private protobuf dependency from public class WriteAck --- .../java/tech/ydb/topic/write/WriteAck.java | 23 +++++++------------ .../tech/ydb/topic/write/impl/WriterImpl.java | 17 +++++++++++++- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java index eacd640be..2699849e9 100644 --- a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java +++ b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java @@ -2,8 +2,6 @@ import java.time.Duration; -import tech.ydb.proto.topic.YdbTopic; - /** * @author Nikolay Perfilov */ @@ -50,13 +48,6 @@ public Statistics getStatistics() { return statistics; } - private static Duration convert(com.google.protobuf.Duration d) { - if (d == null) { - return Duration.ZERO; - } - return Duration.ofSeconds(d.getSeconds(), d.getNanos()); - } - public static class Details { private final long offset; @@ -76,12 +67,14 @@ public static class Statistics { private final Duration maxQueueWaitTime; private final Duration minQueueWaitTime; - public Statistics(YdbTopic.StreamWriteMessage.WriteResponse.WriteStatistics src) { - this.persistingTime = convert(src.getPersistingTime()); - this.partitionQuotaWaitTime = convert(src.getPartitionQuotaWaitTime()); - this.topicQuotaWaitTime = convert(src.getTopicQuotaWaitTime()); - this.maxQueueWaitTime = convert(src.getMaxQueueWaitTime()); - this.minQueueWaitTime = convert(src.getMinQueueWaitTime()); + public Statistics(Duration persistingTime, + Duration partitionQuotaWaitTime, Duration topicQuotaWaitTime, + Duration maxQueueWaitTime, Duration minQueueWaitTime) { + this.persistingTime = persistingTime; + this.partitionQuotaWaitTime = partitionQuotaWaitTime; + this.topicQuotaWaitTime = topicQuotaWaitTime; + this.maxQueueWaitTime = maxQueueWaitTime; + this.minQueueWaitTime = minQueueWaitTime; } public Duration getPersistingTime() { diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index 13617ab76..137a91adb 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -1,6 +1,7 @@ package tech.ydb.topic.write.impl; import java.io.IOException; +import java.time.Duration; import java.util.Deque; import java.util.LinkedList; import java.util.List; @@ -320,6 +321,13 @@ protected void onShutdown(String reason) { } } + private static Duration convertDuration(com.google.protobuf.Duration d) { + if (d == null) { + return Duration.ZERO; + } + return Duration.ofSeconds(d.getSeconds(), d.getNanos()); + } + private class WriteSessionImpl extends WriteSession { protected String sessionId; private final MessageSender messageSender; @@ -423,7 +431,14 @@ private void onWriteResponse(YdbTopic.StreamWriteMessage.WriteResponse response) logger.debug("[{}] Received WriteResponse with {} WriteAcks", streamId, acks.size()); WriteAck.Statistics statistics = null; if (response.getWriteStatistics() != null) { - statistics = new WriteAck.Statistics(response.getWriteStatistics()); + YdbTopic.StreamWriteMessage.WriteResponse.WriteStatistics src = response.getWriteStatistics(); + statistics = new WriteAck.Statistics( + convertDuration(src.getPersistingTime()), + convertDuration(src.getPartitionQuotaWaitTime()), + convertDuration(src.getTopicQuotaWaitTime()), + convertDuration(src.getMaxQueueWaitTime()), + convertDuration(src.getMinQueueWaitTime()) + ); } int inFlightFreed = 0; long bytesFreed = 0; From 23cea4a12a21b2b0d4a1ff717cabf9ad74181c4a Mon Sep 17 00:00:00 2001 From: Max Zinal Date: Thu, 10 Jul 2025 12:55:32 +0300 Subject: [PATCH 03/17] re-use ProtobufUtils.protoToDuration() for stats conversion --- .../tech/ydb/topic/write/impl/WriterImpl.java | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index 137a91adb..90ccdbcff 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -21,6 +21,7 @@ import tech.ydb.core.Issue; import tech.ydb.core.Status; import tech.ydb.core.StatusCode; +import tech.ydb.core.utils.ProtobufUtils; import tech.ydb.proto.StatusCodesProtos; import tech.ydb.proto.topic.YdbTopic; import tech.ydb.topic.TopicRpc; @@ -321,13 +322,6 @@ protected void onShutdown(String reason) { } } - private static Duration convertDuration(com.google.protobuf.Duration d) { - if (d == null) { - return Duration.ZERO; - } - return Duration.ofSeconds(d.getSeconds(), d.getNanos()); - } - private class WriteSessionImpl extends WriteSession { protected String sessionId; private final MessageSender messageSender; @@ -433,11 +427,11 @@ private void onWriteResponse(YdbTopic.StreamWriteMessage.WriteResponse response) if (response.getWriteStatistics() != null) { YdbTopic.StreamWriteMessage.WriteResponse.WriteStatistics src = response.getWriteStatistics(); statistics = new WriteAck.Statistics( - convertDuration(src.getPersistingTime()), - convertDuration(src.getPartitionQuotaWaitTime()), - convertDuration(src.getTopicQuotaWaitTime()), - convertDuration(src.getMaxQueueWaitTime()), - convertDuration(src.getMinQueueWaitTime()) + ProtobufUtils.protoToDuration(src.getPersistingTime()), + ProtobufUtils.protoToDuration(src.getPartitionQuotaWaitTime()), + ProtobufUtils.protoToDuration(src.getTopicQuotaWaitTime()), + ProtobufUtils.protoToDuration(src.getMaxQueueWaitTime()), + ProtobufUtils.protoToDuration(src.getMinQueueWaitTime()) ); } int inFlightFreed = 0; From 312bb07d911ec8494942de47513e606f10805a17 Mon Sep 17 00:00:00 2001 From: Max Zinal Date: Thu, 10 Jul 2025 12:59:15 +0300 Subject: [PATCH 04/17] removed unused import --- topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index 90ccdbcff..7617fe9f9 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -1,7 +1,6 @@ package tech.ydb.topic.write.impl; import java.io.IOException; -import java.time.Duration; import java.util.Deque; import java.util.LinkedList; import java.util.List; From 7a490e184c6810ca396affe7e2d3f9f109f4e8ee Mon Sep 17 00:00:00 2001 From: Max Zinal Date: Thu, 10 Jul 2025 15:06:47 +0300 Subject: [PATCH 05/17] added comments on statistics in WriteAck --- .../java/tech/ydb/topic/write/WriteAck.java | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java index 2699849e9..6a401494d 100644 --- a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java +++ b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java @@ -41,7 +41,10 @@ public Details getDetails() { } /** - * Obtain message write statistics + * Returns write statistics associated with this write confirmation. + * Note: The statistics may cover multiple messages confirmed together by the server. + * Although this WriteAck corresponds to a single written message, the server may confirm several messages in a single response. + * Therefore, the returned statistics may represent the combined data for all messages included in the same write confirmation from the server. * @return {@link Statistics} with timings if statistics are available or null otherwise */ public Statistics getStatistics() { @@ -60,6 +63,11 @@ public long getOffset() { } } + /** + * Messages batch statistics. + * All messages within the batch are persisted together so write + * statistics is for the whole messages batch. + */ public static class Statistics { private final Duration persistingTime; private final Duration partitionQuotaWaitTime; @@ -67,6 +75,15 @@ public static class Statistics { private final Duration maxQueueWaitTime; private final Duration minQueueWaitTime; + /** + * Create the messages batch statistics object, for a single messages batch. + * + * @param persistingTime + * @param partitionQuotaWaitTime + * @param topicQuotaWaitTime + * @param maxQueueWaitTime + * @param minQueueWaitTime + */ public Statistics(Duration persistingTime, Duration partitionQuotaWaitTime, Duration topicQuotaWaitTime, Duration maxQueueWaitTime, Duration minQueueWaitTime) { @@ -77,22 +94,37 @@ public Statistics(Duration persistingTime, this.minQueueWaitTime = minQueueWaitTime; } + /** + * @return Time spent in persisting of data. + */ public Duration getPersistingTime() { return persistingTime; } + /** + * @return Time spent awaiting for partition write quota. + */ public Duration getPartitionQuotaWaitTime() { return partitionQuotaWaitTime; } + /** + * @return Time spent awaiting for topic write quota. + */ public Duration getTopicQuotaWaitTime() { return topicQuotaWaitTime; } + /** + * @return Time spent in queue before persisting, maximal of all messages in response. + */ public Duration getMaxQueueWaitTime() { return maxQueueWaitTime; } + /** + * @return Time spent in queue before persisting, minimal of all messages in response. + */ public Duration getMinQueueWaitTime() { return minQueueWaitTime; } From 12b2b03a1a95bbd49dedcb2d5ee1292620691449 Mon Sep 17 00:00:00 2001 From: Max Zinal Date: Thu, 10 Jul 2025 15:59:42 +0300 Subject: [PATCH 06/17] make linter happy --- topic/src/main/java/tech/ydb/topic/write/WriteAck.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java index 6a401494d..7d8b21075 100644 --- a/topic/src/main/java/tech/ydb/topic/write/WriteAck.java +++ b/topic/src/main/java/tech/ydb/topic/write/WriteAck.java @@ -77,12 +77,12 @@ public static class Statistics { /** * Create the messages batch statistics object, for a single messages batch. - * + * * @param persistingTime * @param partitionQuotaWaitTime * @param topicQuotaWaitTime * @param maxQueueWaitTime - * @param minQueueWaitTime + * @param minQueueWaitTime */ public Statistics(Duration persistingTime, Duration partitionQuotaWaitTime, Duration topicQuotaWaitTime, From 82702a915478b886f330cf51b6a133a40fbe7343 Mon Sep 17 00:00:00 2001 From: Ivan Sopov Date: Tue, 15 Jul 2025 10:47:14 +0300 Subject: [PATCH 07/17] add maven wrapper --- .mvn/wrapper/maven-wrapper.properties | 19 ++ BUILD.md | 4 +- mvnw | 259 ++++++++++++++++++++++++++ mvnw.cmd | 149 +++++++++++++++ 4 files changed, 429 insertions(+), 2 deletions(-) create mode 100644 .mvn/wrapper/maven-wrapper.properties create mode 100755 mvnw create mode 100644 mvnw.cmd diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties new file mode 100644 index 000000000..2f94e6169 --- /dev/null +++ b/.mvn/wrapper/maven-wrapper.properties @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +wrapperVersion=3.3.2 +distributionType=only-script +distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.10/apache-maven-3.9.10-bin.zip diff --git a/BUILD.md b/BUILD.md index 91c6e4e6b..cd251e75b 100644 --- a/BUILD.md +++ b/BUILD.md @@ -10,11 +10,11 @@ You can install the SDK artifacts in your local maven cache by running the following command in project folder. During the build process, the working directory will be cleared, tests will be run, artifacts will be built and copied to the local repository. ``` -mvn clean install +./mvnw clean install ``` If you don't need the test executions, just disable them ``` -mvn clean install -DskipTests=true +./mvnw clean install -DskipTests=true ``` diff --git a/mvnw b/mvnw new file mode 100755 index 000000000..19529ddf8 --- /dev/null +++ b/mvnw @@ -0,0 +1,259 @@ +#!/bin/sh +# ---------------------------------------------------------------------------- +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# ---------------------------------------------------------------------------- + +# ---------------------------------------------------------------------------- +# Apache Maven Wrapper startup batch script, version 3.3.2 +# +# Optional ENV vars +# ----------------- +# JAVA_HOME - location of a JDK home dir, required when download maven via java source +# MVNW_REPOURL - repo url base for downloading maven distribution +# MVNW_USERNAME/MVNW_PASSWORD - user and password for downloading maven +# MVNW_VERBOSE - true: enable verbose log; debug: trace the mvnw script; others: silence the output +# ---------------------------------------------------------------------------- + +set -euf +[ "${MVNW_VERBOSE-}" != debug ] || set -x + +# OS specific support. +native_path() { printf %s\\n "$1"; } +case "$(uname)" in +CYGWIN* | MINGW*) + [ -z "${JAVA_HOME-}" ] || JAVA_HOME="$(cygpath --unix "$JAVA_HOME")" + native_path() { cygpath --path --windows "$1"; } + ;; +esac + +# set JAVACMD and JAVACCMD +set_java_home() { + # For Cygwin and MinGW, ensure paths are in Unix format before anything is touched + if [ -n "${JAVA_HOME-}" ]; then + if [ -x "$JAVA_HOME/jre/sh/java" ]; then + # IBM's JDK on AIX uses strange locations for the executables + JAVACMD="$JAVA_HOME/jre/sh/java" + JAVACCMD="$JAVA_HOME/jre/sh/javac" + else + JAVACMD="$JAVA_HOME/bin/java" + JAVACCMD="$JAVA_HOME/bin/javac" + + if [ ! -x "$JAVACMD" ] || [ ! -x "$JAVACCMD" ]; then + echo "The JAVA_HOME environment variable is not defined correctly, so mvnw cannot run." >&2 + echo "JAVA_HOME is set to \"$JAVA_HOME\", but \"\$JAVA_HOME/bin/java\" or \"\$JAVA_HOME/bin/javac\" does not exist." >&2 + return 1 + fi + fi + else + JAVACMD="$( + 'set' +e + 'unset' -f command 2>/dev/null + 'command' -v java + )" || : + JAVACCMD="$( + 'set' +e + 'unset' -f command 2>/dev/null + 'command' -v javac + )" || : + + if [ ! -x "${JAVACMD-}" ] || [ ! -x "${JAVACCMD-}" ]; then + echo "The java/javac command does not exist in PATH nor is JAVA_HOME set, so mvnw cannot run." >&2 + return 1 + fi + fi +} + +# hash string like Java String::hashCode +hash_string() { + str="${1:-}" h=0 + while [ -n "$str" ]; do + char="${str%"${str#?}"}" + h=$(((h * 31 + $(LC_CTYPE=C printf %d "'$char")) % 4294967296)) + str="${str#?}" + done + printf %x\\n $h +} + +verbose() { :; } +[ "${MVNW_VERBOSE-}" != true ] || verbose() { printf %s\\n "${1-}"; } + +die() { + printf %s\\n "$1" >&2 + exit 1 +} + +trim() { + # MWRAPPER-139: + # Trims trailing and leading whitespace, carriage returns, tabs, and linefeeds. + # Needed for removing poorly interpreted newline sequences when running in more + # exotic environments such as mingw bash on Windows. + printf "%s" "${1}" | tr -d '[:space:]' +} + +# parse distributionUrl and optional distributionSha256Sum, requires .mvn/wrapper/maven-wrapper.properties +while IFS="=" read -r key value; do + case "${key-}" in + distributionUrl) distributionUrl=$(trim "${value-}") ;; + distributionSha256Sum) distributionSha256Sum=$(trim "${value-}") ;; + esac +done <"${0%/*}/.mvn/wrapper/maven-wrapper.properties" +[ -n "${distributionUrl-}" ] || die "cannot read distributionUrl property in ${0%/*}/.mvn/wrapper/maven-wrapper.properties" + +case "${distributionUrl##*/}" in +maven-mvnd-*bin.*) + MVN_CMD=mvnd.sh _MVNW_REPO_PATTERN=/maven/mvnd/ + case "${PROCESSOR_ARCHITECTURE-}${PROCESSOR_ARCHITEW6432-}:$(uname -a)" in + *AMD64:CYGWIN* | *AMD64:MINGW*) distributionPlatform=windows-amd64 ;; + :Darwin*x86_64) distributionPlatform=darwin-amd64 ;; + :Darwin*arm64) distributionPlatform=darwin-aarch64 ;; + :Linux*x86_64*) distributionPlatform=linux-amd64 ;; + *) + echo "Cannot detect native platform for mvnd on $(uname)-$(uname -m), use pure java version" >&2 + distributionPlatform=linux-amd64 + ;; + esac + distributionUrl="${distributionUrl%-bin.*}-$distributionPlatform.zip" + ;; +maven-mvnd-*) MVN_CMD=mvnd.sh _MVNW_REPO_PATTERN=/maven/mvnd/ ;; +*) MVN_CMD="mvn${0##*/mvnw}" _MVNW_REPO_PATTERN=/org/apache/maven/ ;; +esac + +# apply MVNW_REPOURL and calculate MAVEN_HOME +# maven home pattern: ~/.m2/wrapper/dists/{apache-maven-,maven-mvnd--}/ +[ -z "${MVNW_REPOURL-}" ] || distributionUrl="$MVNW_REPOURL$_MVNW_REPO_PATTERN${distributionUrl#*"$_MVNW_REPO_PATTERN"}" +distributionUrlName="${distributionUrl##*/}" +distributionUrlNameMain="${distributionUrlName%.*}" +distributionUrlNameMain="${distributionUrlNameMain%-bin}" +MAVEN_USER_HOME="${MAVEN_USER_HOME:-${HOME}/.m2}" +MAVEN_HOME="${MAVEN_USER_HOME}/wrapper/dists/${distributionUrlNameMain-}/$(hash_string "$distributionUrl")" + +exec_maven() { + unset MVNW_VERBOSE MVNW_USERNAME MVNW_PASSWORD MVNW_REPOURL || : + exec "$MAVEN_HOME/bin/$MVN_CMD" "$@" || die "cannot exec $MAVEN_HOME/bin/$MVN_CMD" +} + +if [ -d "$MAVEN_HOME" ]; then + verbose "found existing MAVEN_HOME at $MAVEN_HOME" + exec_maven "$@" +fi + +case "${distributionUrl-}" in +*?-bin.zip | *?maven-mvnd-?*-?*.zip) ;; +*) die "distributionUrl is not valid, must match *-bin.zip or maven-mvnd-*.zip, but found '${distributionUrl-}'" ;; +esac + +# prepare tmp dir +if TMP_DOWNLOAD_DIR="$(mktemp -d)" && [ -d "$TMP_DOWNLOAD_DIR" ]; then + clean() { rm -rf -- "$TMP_DOWNLOAD_DIR"; } + trap clean HUP INT TERM EXIT +else + die "cannot create temp dir" +fi + +mkdir -p -- "${MAVEN_HOME%/*}" + +# Download and Install Apache Maven +verbose "Couldn't find MAVEN_HOME, downloading and installing it ..." +verbose "Downloading from: $distributionUrl" +verbose "Downloading to: $TMP_DOWNLOAD_DIR/$distributionUrlName" + +# select .zip or .tar.gz +if ! command -v unzip >/dev/null; then + distributionUrl="${distributionUrl%.zip}.tar.gz" + distributionUrlName="${distributionUrl##*/}" +fi + +# verbose opt +__MVNW_QUIET_WGET=--quiet __MVNW_QUIET_CURL=--silent __MVNW_QUIET_UNZIP=-q __MVNW_QUIET_TAR='' +[ "${MVNW_VERBOSE-}" != true ] || __MVNW_QUIET_WGET='' __MVNW_QUIET_CURL='' __MVNW_QUIET_UNZIP='' __MVNW_QUIET_TAR=v + +# normalize http auth +case "${MVNW_PASSWORD:+has-password}" in +'') MVNW_USERNAME='' MVNW_PASSWORD='' ;; +has-password) [ -n "${MVNW_USERNAME-}" ] || MVNW_USERNAME='' MVNW_PASSWORD='' ;; +esac + +if [ -z "${MVNW_USERNAME-}" ] && command -v wget >/dev/null; then + verbose "Found wget ... using wget" + wget ${__MVNW_QUIET_WGET:+"$__MVNW_QUIET_WGET"} "$distributionUrl" -O "$TMP_DOWNLOAD_DIR/$distributionUrlName" || die "wget: Failed to fetch $distributionUrl" +elif [ -z "${MVNW_USERNAME-}" ] && command -v curl >/dev/null; then + verbose "Found curl ... using curl" + curl ${__MVNW_QUIET_CURL:+"$__MVNW_QUIET_CURL"} -f -L -o "$TMP_DOWNLOAD_DIR/$distributionUrlName" "$distributionUrl" || die "curl: Failed to fetch $distributionUrl" +elif set_java_home; then + verbose "Falling back to use Java to download" + javaSource="$TMP_DOWNLOAD_DIR/Downloader.java" + targetZip="$TMP_DOWNLOAD_DIR/$distributionUrlName" + cat >"$javaSource" <<-END + public class Downloader extends java.net.Authenticator + { + protected java.net.PasswordAuthentication getPasswordAuthentication() + { + return new java.net.PasswordAuthentication( System.getenv( "MVNW_USERNAME" ), System.getenv( "MVNW_PASSWORD" ).toCharArray() ); + } + public static void main( String[] args ) throws Exception + { + setDefault( new Downloader() ); + java.nio.file.Files.copy( java.net.URI.create( args[0] ).toURL().openStream(), java.nio.file.Paths.get( args[1] ).toAbsolutePath().normalize() ); + } + } + END + # For Cygwin/MinGW, switch paths to Windows format before running javac and java + verbose " - Compiling Downloader.java ..." + "$(native_path "$JAVACCMD")" "$(native_path "$javaSource")" || die "Failed to compile Downloader.java" + verbose " - Running Downloader.java ..." + "$(native_path "$JAVACMD")" -cp "$(native_path "$TMP_DOWNLOAD_DIR")" Downloader "$distributionUrl" "$(native_path "$targetZip")" +fi + +# If specified, validate the SHA-256 sum of the Maven distribution zip file +if [ -n "${distributionSha256Sum-}" ]; then + distributionSha256Result=false + if [ "$MVN_CMD" = mvnd.sh ]; then + echo "Checksum validation is not supported for maven-mvnd." >&2 + echo "Please disable validation by removing 'distributionSha256Sum' from your maven-wrapper.properties." >&2 + exit 1 + elif command -v sha256sum >/dev/null; then + if echo "$distributionSha256Sum $TMP_DOWNLOAD_DIR/$distributionUrlName" | sha256sum -c >/dev/null 2>&1; then + distributionSha256Result=true + fi + elif command -v shasum >/dev/null; then + if echo "$distributionSha256Sum $TMP_DOWNLOAD_DIR/$distributionUrlName" | shasum -a 256 -c >/dev/null 2>&1; then + distributionSha256Result=true + fi + else + echo "Checksum validation was requested but neither 'sha256sum' or 'shasum' are available." >&2 + echo "Please install either command, or disable validation by removing 'distributionSha256Sum' from your maven-wrapper.properties." >&2 + exit 1 + fi + if [ $distributionSha256Result = false ]; then + echo "Error: Failed to validate Maven distribution SHA-256, your Maven distribution might be compromised." >&2 + echo "If you updated your Maven version, you need to update the specified distributionSha256Sum property." >&2 + exit 1 + fi +fi + +# unzip and move +if command -v unzip >/dev/null; then + unzip ${__MVNW_QUIET_UNZIP:+"$__MVNW_QUIET_UNZIP"} "$TMP_DOWNLOAD_DIR/$distributionUrlName" -d "$TMP_DOWNLOAD_DIR" || die "failed to unzip" +else + tar xzf${__MVNW_QUIET_TAR:+"$__MVNW_QUIET_TAR"} "$TMP_DOWNLOAD_DIR/$distributionUrlName" -C "$TMP_DOWNLOAD_DIR" || die "failed to untar" +fi +printf %s\\n "$distributionUrl" >"$TMP_DOWNLOAD_DIR/$distributionUrlNameMain/mvnw.url" +mv -- "$TMP_DOWNLOAD_DIR/$distributionUrlNameMain" "$MAVEN_HOME" || [ -d "$MAVEN_HOME" ] || die "fail to move MAVEN_HOME" + +clean || : +exec_maven "$@" diff --git a/mvnw.cmd b/mvnw.cmd new file mode 100644 index 000000000..249bdf382 --- /dev/null +++ b/mvnw.cmd @@ -0,0 +1,149 @@ +<# : batch portion +@REM ---------------------------------------------------------------------------- +@REM Licensed to the Apache Software Foundation (ASF) under one +@REM or more contributor license agreements. See the NOTICE file +@REM distributed with this work for additional information +@REM regarding copyright ownership. The ASF licenses this file +@REM to you under the Apache License, Version 2.0 (the +@REM "License"); you may not use this file except in compliance +@REM with the License. You may obtain a copy of the License at +@REM +@REM http://www.apache.org/licenses/LICENSE-2.0 +@REM +@REM Unless required by applicable law or agreed to in writing, +@REM software distributed under the License is distributed on an +@REM "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +@REM KIND, either express or implied. See the License for the +@REM specific language governing permissions and limitations +@REM under the License. +@REM ---------------------------------------------------------------------------- + +@REM ---------------------------------------------------------------------------- +@REM Apache Maven Wrapper startup batch script, version 3.3.2 +@REM +@REM Optional ENV vars +@REM MVNW_REPOURL - repo url base for downloading maven distribution +@REM MVNW_USERNAME/MVNW_PASSWORD - user and password for downloading maven +@REM MVNW_VERBOSE - true: enable verbose log; others: silence the output +@REM ---------------------------------------------------------------------------- + +@IF "%__MVNW_ARG0_NAME__%"=="" (SET __MVNW_ARG0_NAME__=%~nx0) +@SET __MVNW_CMD__= +@SET __MVNW_ERROR__= +@SET __MVNW_PSMODULEP_SAVE=%PSModulePath% +@SET PSModulePath= +@FOR /F "usebackq tokens=1* delims==" %%A IN (`powershell -noprofile "& {$scriptDir='%~dp0'; $script='%__MVNW_ARG0_NAME__%'; icm -ScriptBlock ([Scriptblock]::Create((Get-Content -Raw '%~f0'))) -NoNewScope}"`) DO @( + IF "%%A"=="MVN_CMD" (set __MVNW_CMD__=%%B) ELSE IF "%%B"=="" (echo %%A) ELSE (echo %%A=%%B) +) +@SET PSModulePath=%__MVNW_PSMODULEP_SAVE% +@SET __MVNW_PSMODULEP_SAVE= +@SET __MVNW_ARG0_NAME__= +@SET MVNW_USERNAME= +@SET MVNW_PASSWORD= +@IF NOT "%__MVNW_CMD__%"=="" (%__MVNW_CMD__% %*) +@echo Cannot start maven from wrapper >&2 && exit /b 1 +@GOTO :EOF +: end batch / begin powershell #> + +$ErrorActionPreference = "Stop" +if ($env:MVNW_VERBOSE -eq "true") { + $VerbosePreference = "Continue" +} + +# calculate distributionUrl, requires .mvn/wrapper/maven-wrapper.properties +$distributionUrl = (Get-Content -Raw "$scriptDir/.mvn/wrapper/maven-wrapper.properties" | ConvertFrom-StringData).distributionUrl +if (!$distributionUrl) { + Write-Error "cannot read distributionUrl property in $scriptDir/.mvn/wrapper/maven-wrapper.properties" +} + +switch -wildcard -casesensitive ( $($distributionUrl -replace '^.*/','') ) { + "maven-mvnd-*" { + $USE_MVND = $true + $distributionUrl = $distributionUrl -replace '-bin\.[^.]*$',"-windows-amd64.zip" + $MVN_CMD = "mvnd.cmd" + break + } + default { + $USE_MVND = $false + $MVN_CMD = $script -replace '^mvnw','mvn' + break + } +} + +# apply MVNW_REPOURL and calculate MAVEN_HOME +# maven home pattern: ~/.m2/wrapper/dists/{apache-maven-,maven-mvnd--}/ +if ($env:MVNW_REPOURL) { + $MVNW_REPO_PATTERN = if ($USE_MVND) { "/org/apache/maven/" } else { "/maven/mvnd/" } + $distributionUrl = "$env:MVNW_REPOURL$MVNW_REPO_PATTERN$($distributionUrl -replace '^.*'+$MVNW_REPO_PATTERN,'')" +} +$distributionUrlName = $distributionUrl -replace '^.*/','' +$distributionUrlNameMain = $distributionUrlName -replace '\.[^.]*$','' -replace '-bin$','' +$MAVEN_HOME_PARENT = "$HOME/.m2/wrapper/dists/$distributionUrlNameMain" +if ($env:MAVEN_USER_HOME) { + $MAVEN_HOME_PARENT = "$env:MAVEN_USER_HOME/wrapper/dists/$distributionUrlNameMain" +} +$MAVEN_HOME_NAME = ([System.Security.Cryptography.MD5]::Create().ComputeHash([byte[]][char[]]$distributionUrl) | ForEach-Object {$_.ToString("x2")}) -join '' +$MAVEN_HOME = "$MAVEN_HOME_PARENT/$MAVEN_HOME_NAME" + +if (Test-Path -Path "$MAVEN_HOME" -PathType Container) { + Write-Verbose "found existing MAVEN_HOME at $MAVEN_HOME" + Write-Output "MVN_CMD=$MAVEN_HOME/bin/$MVN_CMD" + exit $? +} + +if (! $distributionUrlNameMain -or ($distributionUrlName -eq $distributionUrlNameMain)) { + Write-Error "distributionUrl is not valid, must end with *-bin.zip, but found $distributionUrl" +} + +# prepare tmp dir +$TMP_DOWNLOAD_DIR_HOLDER = New-TemporaryFile +$TMP_DOWNLOAD_DIR = New-Item -Itemtype Directory -Path "$TMP_DOWNLOAD_DIR_HOLDER.dir" +$TMP_DOWNLOAD_DIR_HOLDER.Delete() | Out-Null +trap { + if ($TMP_DOWNLOAD_DIR.Exists) { + try { Remove-Item $TMP_DOWNLOAD_DIR -Recurse -Force | Out-Null } + catch { Write-Warning "Cannot remove $TMP_DOWNLOAD_DIR" } + } +} + +New-Item -Itemtype Directory -Path "$MAVEN_HOME_PARENT" -Force | Out-Null + +# Download and Install Apache Maven +Write-Verbose "Couldn't find MAVEN_HOME, downloading and installing it ..." +Write-Verbose "Downloading from: $distributionUrl" +Write-Verbose "Downloading to: $TMP_DOWNLOAD_DIR/$distributionUrlName" + +$webclient = New-Object System.Net.WebClient +if ($env:MVNW_USERNAME -and $env:MVNW_PASSWORD) { + $webclient.Credentials = New-Object System.Net.NetworkCredential($env:MVNW_USERNAME, $env:MVNW_PASSWORD) +} +[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 +$webclient.DownloadFile($distributionUrl, "$TMP_DOWNLOAD_DIR/$distributionUrlName") | Out-Null + +# If specified, validate the SHA-256 sum of the Maven distribution zip file +$distributionSha256Sum = (Get-Content -Raw "$scriptDir/.mvn/wrapper/maven-wrapper.properties" | ConvertFrom-StringData).distributionSha256Sum +if ($distributionSha256Sum) { + if ($USE_MVND) { + Write-Error "Checksum validation is not supported for maven-mvnd. `nPlease disable validation by removing 'distributionSha256Sum' from your maven-wrapper.properties." + } + Import-Module $PSHOME\Modules\Microsoft.PowerShell.Utility -Function Get-FileHash + if ((Get-FileHash "$TMP_DOWNLOAD_DIR/$distributionUrlName" -Algorithm SHA256).Hash.ToLower() -ne $distributionSha256Sum) { + Write-Error "Error: Failed to validate Maven distribution SHA-256, your Maven distribution might be compromised. If you updated your Maven version, you need to update the specified distributionSha256Sum property." + } +} + +# unzip and move +Expand-Archive "$TMP_DOWNLOAD_DIR/$distributionUrlName" -DestinationPath "$TMP_DOWNLOAD_DIR" | Out-Null +Rename-Item -Path "$TMP_DOWNLOAD_DIR/$distributionUrlNameMain" -NewName $MAVEN_HOME_NAME | Out-Null +try { + Move-Item -Path "$TMP_DOWNLOAD_DIR/$MAVEN_HOME_NAME" -Destination $MAVEN_HOME_PARENT | Out-Null +} catch { + if (! (Test-Path -Path "$MAVEN_HOME" -PathType Container)) { + Write-Error "fail to move MAVEN_HOME" + } +} finally { + try { Remove-Item $TMP_DOWNLOAD_DIR -Recurse -Force | Out-Null } + catch { Write-Warning "Cannot remove $TMP_DOWNLOAD_DIR" } +} + +Write-Output "MVN_CMD=$MAVEN_HOME/bin/$MVN_CMD" From 4d5003a9f1222626aa2197dd794e8c5ee572ba1a Mon Sep 17 00:00:00 2001 From: Maksim Zinal Date: Wed, 30 Jul 2025 23:04:46 +0300 Subject: [PATCH 08/17] basic comparison logic --- .../tech/ydb/table/values/DecimalValue.java | 59 ++++ .../java/tech/ydb/table/values/DictValue.java | 83 +++++ .../java/tech/ydb/table/values/ListValue.java | 52 +++ .../java/tech/ydb/table/values/NullValue.java | 14 + .../tech/ydb/table/values/OptionalValue.java | 58 ++++ .../tech/ydb/table/values/PrimitiveValue.java | 80 +++++ .../tech/ydb/table/values/StructValue.java | 52 +++ .../tech/ydb/table/values/TupleValue.java | 52 +++ .../java/tech/ydb/table/values/Value.java | 14 +- .../tech/ydb/table/values/VariantValue.java | 45 +++ .../java/tech/ydb/table/values/VoidValue.java | 14 + .../ydb/table/values/ValueComparableTest.java | 295 ++++++++++++++++++ 12 files changed, 817 insertions(+), 1 deletion(-) create mode 100644 table/src/test/java/tech/ydb/table/values/ValueComparableTest.java diff --git a/table/src/main/java/tech/ydb/table/values/DecimalValue.java b/table/src/main/java/tech/ydb/table/values/DecimalValue.java index bc68308f5..ea48ca858 100644 --- a/table/src/main/java/tech/ydb/table/values/DecimalValue.java +++ b/table/src/main/java/tech/ydb/table/values/DecimalValue.java @@ -269,6 +269,65 @@ public ValueProtos.Value toPb() { return ProtoValue.fromDecimal(high, low); } + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof DecimalValue)) { + throw new IllegalArgumentException("Cannot compare DecimalValue with " + other.getClass().getSimpleName()); + } + + DecimalValue otherDecimal = (DecimalValue) other; + + // Handle special values first + if (isNan() || otherDecimal.isNan()) { + // NaN is not comparable, but we need to provide a consistent ordering + if (isNan() && otherDecimal.isNan()) { + return 0; + } + if (isNan()) { + return 1; // NaN is considered greater than any other value + } + return -1; + } + + if (isInf() && otherDecimal.isInf()) { + return 0; + } + if (isInf()) { + return 1; // Positive infinity is greater than any finite value + } + if (otherDecimal.isInf()) { + return -1; + } + + if (isNegativeInf() && otherDecimal.isNegativeInf()) { + return 0; + } + if (isNegativeInf()) { + return -1; // Negative infinity is less than any finite value + } + if (otherDecimal.isNegativeInf()) { + return 1; + } + + // Compare finite values + if (isNegative() != otherDecimal.isNegative()) { + return isNegative() ? -1 : 1; + } + + // Both have the same sign, compare magnitudes + int highComparison = Long.compareUnsigned(high, otherDecimal.high); + if (highComparison != 0) { + return isNegative() ? -highComparison : highComparison; + } + + int lowComparison = Long.compareUnsigned(low, otherDecimal.low); + return isNegative() ? -lowComparison : lowComparison; + } + /** * Write long to a big-endian buffer. */ diff --git a/table/src/main/java/tech/ydb/table/values/DictValue.java b/table/src/main/java/tech/ydb/table/values/DictValue.java index 59c9d28b1..6ba9db076 100644 --- a/table/src/main/java/tech/ydb/table/values/DictValue.java +++ b/table/src/main/java/tech/ydb/table/values/DictValue.java @@ -1,7 +1,9 @@ package tech.ydb.table.values; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; @@ -116,4 +118,85 @@ public ValueProtos.Value toPb() { } return builder.build(); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof DictValue)) { + throw new IllegalArgumentException("Cannot compare DictValue with " + other.getClass().getSimpleName()); + } + + DictValue otherDict = (DictValue) other; + + // Compare sizes first + int sizeComparison = Integer.compare(items.size(), otherDict.items.size()); + if (sizeComparison != 0) { + return sizeComparison; + } + + // Convert to sorted lists for lexicographical comparison + List, Value>> thisEntries = new ArrayList<>(items.entrySet()); + List, Value>> otherEntries = new ArrayList<>(otherDict.items.entrySet()); + + // Sort entries by key first, then by value + thisEntries.sort((e1, e2) -> { + int keyComparison = compareValues(e1.getKey(), e2.getKey()); + if (keyComparison != 0) { + return keyComparison; + } + return compareValues(e1.getValue(), e2.getValue()); + }); + + otherEntries.sort((e1, e2) -> { + int keyComparison = compareValues(e1.getKey(), e2.getKey()); + if (keyComparison != 0) { + return keyComparison; + } + return compareValues(e1.getValue(), e2.getValue()); + }); + + // Compare sorted entries + for (int i = 0; i < thisEntries.size(); i++) { + Map.Entry, Value> thisEntry = thisEntries.get(i); + Map.Entry, Value> otherEntry = otherEntries.get(i); + + int keyComparison = compareValues(thisEntry.getKey(), otherEntry.getKey()); + if (keyComparison != 0) { + return keyComparison; + } + + int valueComparison = compareValues(thisEntry.getValue(), otherEntry.getValue()); + if (valueComparison != 0) { + return valueComparison; + } + } + + return 0; + } + + private static int compareValues(Value a, Value b) { + // Handle null values + if (a == null && b == null) return 0; + if (a == null) return -1; + if (b == null) return 1; + + // Check that the types are the same + if (!a.getType().equals(b.getType())) { + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getType() + " vs " + b.getType()); + } + + // Use the actual compareTo method of the values + if (a instanceof Comparable && b instanceof Comparable) { + try { + return ((Comparable>) a).compareTo((Value) b); + } catch (ClassCastException e) {} + } + + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + } } diff --git a/table/src/main/java/tech/ydb/table/values/ListValue.java b/table/src/main/java/tech/ydb/table/values/ListValue.java index 94699b605..38a11703e 100644 --- a/table/src/main/java/tech/ydb/table/values/ListValue.java +++ b/table/src/main/java/tech/ydb/table/values/ListValue.java @@ -113,4 +113,56 @@ public ValueProtos.Value toPb() { } return builder.build(); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof ListValue)) { + throw new IllegalArgumentException("Cannot compare ListValue with " + other.getClass().getSimpleName()); + } + + ListValue otherList = (ListValue) other; + + // Compare elements lexicographically + int minLength = Math.min(items.length, otherList.items.length); + for (int i = 0; i < minLength; i++) { + Value thisItem = items[i]; + Value otherItem = otherList.items[i]; + + int itemComparison = compareValues(thisItem, otherItem); + if (itemComparison != 0) { + return itemComparison; + } + } + + // If we reach here, one list is a prefix of the other + // The shorter list comes first + return Integer.compare(items.length, otherList.items.length); + } + + private static int compareValues(Value a, Value b) { + // Handle null values + if (a == null && b == null) return 0; + if (a == null) return -1; + if (b == null) return 1; + + // Check that the types are the same + if (!a.getType().equals(b.getType())) { + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getType() + " vs " + b.getType()); + } + + // Use the actual compareTo method of the values + if (a instanceof Comparable && b instanceof Comparable) { + try { + return ((Comparable>) a).compareTo((Value) b); + } catch (ClassCastException e) {} + } + + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + } } diff --git a/table/src/main/java/tech/ydb/table/values/NullValue.java b/table/src/main/java/tech/ydb/table/values/NullValue.java index 8d3637967..352ef1baa 100644 --- a/table/src/main/java/tech/ydb/table/values/NullValue.java +++ b/table/src/main/java/tech/ydb/table/values/NullValue.java @@ -32,4 +32,18 @@ public NullType getType() { public ValueProtos.Value toPb() { return ProtoValue.nullValue(); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof NullValue)) { + throw new IllegalArgumentException("Cannot compare NullValue with " + other.getClass().getSimpleName()); + } + + // All NullValue instances are equal + return 0; + } } diff --git a/table/src/main/java/tech/ydb/table/values/OptionalValue.java b/table/src/main/java/tech/ydb/table/values/OptionalValue.java index 71c8d3f19..8d31dc69c 100644 --- a/table/src/main/java/tech/ydb/table/values/OptionalValue.java +++ b/table/src/main/java/tech/ydb/table/values/OptionalValue.java @@ -96,4 +96,62 @@ public ValueProtos.Value toPb() { return ProtoValue.optional(); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + // Handle comparison with another OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item types are the same + if (!type.getItemType().equals(otherOptional.type.getItemType())) { + throw new IllegalArgumentException("Cannot compare OptionalValue with different item types: " + + type.getItemType() + " vs " + otherOptional.type.getItemType()); + } + + // Handle empty values: empty values are considered less than non-empty values + if (value == null && otherOptional.value == null) { + return 0; + } + if (value == null) { + return -1; + } + if (otherOptional.value == null) { + return 1; + } + + // Both values are non-null and have the same type, compare them using their compareTo method + return compareValues(value, otherOptional.value); + } + + // Handle comparison with non-optional values of the same underlying type + if (type.getItemType().equals(other.getType())) { + // This OptionalValue is empty, so it's less than any non-optional value + if (value == null) { + return -1; + } + + // This OptionalValue has a value, compare it with the non-optional value + return compareValues(value, other); + } + + // Types are incompatible + throw new IllegalArgumentException("Cannot compare OptionalValue with incompatible type: " + + type.getItemType() + " vs " + other.getType()); + } + + private static int compareValues(Value a, Value b) { + // Since we've already verified the types are the same, we can safely cast + // and use the compareTo method of the actual value type + if (a instanceof Comparable && b instanceof Comparable) { + try { + return ((Comparable>) a).compareTo((Value) b); + } catch (ClassCastException e) {} + } + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + } } diff --git a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java index 103387154..1c61c84f9 100644 --- a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java +++ b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java @@ -423,6 +423,86 @@ private static void checkType(PrimitiveType expected, PrimitiveType actual) { } } + private static int compareByteArrays(byte[] a, byte[] b) { + int minLength = Math.min(a.length, b.length); + for (int i = 0; i < minLength; i++) { + int comparison = Byte.compare(a[i], b[i]); + if (comparison != 0) { + return comparison; + } + } + return Integer.compare(a.length, b.length); + } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof PrimitiveValue)) { + throw new IllegalArgumentException("Cannot compare PrimitiveValue with " + other.getClass().getSimpleName()); + } + + PrimitiveValue otherPrimitive = (PrimitiveValue) other; + if (getType() != otherPrimitive.getType()) { + throw new IllegalArgumentException("Cannot compare values of different types: " + getType() + " vs " + otherPrimitive.getType()); + } + + // Compare based on the actual primitive type + switch (getType()) { + case Bool: + return Boolean.compare(getBool(), otherPrimitive.getBool()); + case Int8: + return Byte.compare(getInt8(), otherPrimitive.getInt8()); + case Uint8: + return Integer.compare(getUint8(), otherPrimitive.getUint8()); + case Int16: + return Short.compare(getInt16(), otherPrimitive.getInt16()); + case Uint16: + return Integer.compare(getUint16(), otherPrimitive.getUint16()); + case Int32: + return Integer.compare(getInt32(), otherPrimitive.getInt32()); + case Uint32: + return Long.compare(getUint32(), otherPrimitive.getUint32()); + case Int64: + return Long.compare(getInt64(), otherPrimitive.getInt64()); + case Uint64: + return Long.compare(getUint64(), otherPrimitive.getUint64()); + case Float: + return Float.compare(getFloat(), otherPrimitive.getFloat()); + case Double: + return Double.compare(getDouble(), otherPrimitive.getDouble()); + case Bytes: + case Yson: + return compareByteArrays(getBytesUnsafe(), otherPrimitive.getBytesUnsafe()); + case Text: + case Json: + case JsonDocument: + return getText().compareTo(otherPrimitive.getText()); + case Uuid: + return getUuidJdk().compareTo(otherPrimitive.getUuidJdk()); + case Date: + case Date32: + return getDate().compareTo(otherPrimitive.getDate()); + case Datetime: + case Datetime64: + return getDatetime().compareTo(otherPrimitive.getDatetime()); + case Timestamp: + case Timestamp64: + return getTimestamp().compareTo(otherPrimitive.getTimestamp()); + case Interval: + case Interval64: + return getInterval().compareTo(otherPrimitive.getInterval()); + case TzDate: + case TzDatetime: + case TzTimestamp: + return getTzDate().compareTo(otherPrimitive.getTzDate()); + default: + throw new UnsupportedOperationException("Comparison not supported for type: " + getType()); + } + } + // -- implementations -- private static final class Bool extends PrimitiveValue { diff --git a/table/src/main/java/tech/ydb/table/values/StructValue.java b/table/src/main/java/tech/ydb/table/values/StructValue.java index 7c34d37a4..5b3f44b28 100644 --- a/table/src/main/java/tech/ydb/table/values/StructValue.java +++ b/table/src/main/java/tech/ydb/table/values/StructValue.java @@ -134,6 +134,58 @@ public ValueProtos.Value toPb() { return builder.build(); } + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof StructValue)) { + throw new IllegalArgumentException("Cannot compare StructValue with " + other.getClass().getSimpleName()); + } + + StructValue otherStruct = (StructValue) other; + + // Compare members lexicographically + int minLength = Math.min(members.length, otherStruct.members.length); + for (int i = 0; i < minLength; i++) { + Value thisMember = members[i]; + Value otherMember = otherStruct.members[i]; + + int memberComparison = compareValues(thisMember, otherMember); + if (memberComparison != 0) { + return memberComparison; + } + } + + // If we reach here, one struct is a prefix of the other + // The shorter struct comes first + return Integer.compare(members.length, otherStruct.members.length); + } + + private static int compareValues(Value a, Value b) { + // Handle null values + if (a == null && b == null) return 0; + if (a == null) return -1; + if (b == null) return 1; + + // Check that the types are the same + if (!a.getType().equals(b.getType())) { + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getType() + " vs " + b.getType()); + } + + // Use the actual compareTo method of the values + if (a instanceof Comparable && b instanceof Comparable) { + try { + return ((Comparable>) a).compareTo((Value) b); + } catch (ClassCastException e) {} + } + + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + } + private static StructValue newStruct(String[] names, Value[] values) { Arrays2.sortBothByFirst(names, values); final Type[] types = new Type[values.length]; diff --git a/table/src/main/java/tech/ydb/table/values/TupleValue.java b/table/src/main/java/tech/ydb/table/values/TupleValue.java index 9b80a1b24..f2a6f1943 100644 --- a/table/src/main/java/tech/ydb/table/values/TupleValue.java +++ b/table/src/main/java/tech/ydb/table/values/TupleValue.java @@ -145,4 +145,56 @@ private static TupleValue fromArray(Value... items) { } return new TupleValue(TupleType.ofOwn(types), items); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof TupleValue)) { + throw new IllegalArgumentException("Cannot compare TupleValue with " + other.getClass().getSimpleName()); + } + + TupleValue otherTuple = (TupleValue) other; + + // Compare elements lexicographically + int minLength = Math.min(items.length, otherTuple.items.length); + for (int i = 0; i < minLength; i++) { + Value thisItem = items[i]; + Value otherItem = otherTuple.items[i]; + + int itemComparison = compareValues(thisItem, otherItem); + if (itemComparison != 0) { + return itemComparison; + } + } + + // If we reach here, one tuple is a prefix of the other + // The shorter tuple comes first + return Integer.compare(items.length, otherTuple.items.length); + } + + private static int compareValues(Value a, Value b) { + // Handle null values + if (a == null && b == null) return 0; + if (a == null) return -1; + if (b == null) return 1; + + // Check that the types are the same + if (!a.getType().equals(b.getType())) { + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getType() + " vs " + b.getType()); + } + + // Use the actual compareTo method of the values + if (a instanceof Comparable && b instanceof Comparable) { + try { + return ((Comparable>) a).compareTo((Value) b); + } catch (ClassCastException e) {} + } + + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + } } diff --git a/table/src/main/java/tech/ydb/table/values/Value.java b/table/src/main/java/tech/ydb/table/values/Value.java index 91ed8df7f..a20a41df7 100644 --- a/table/src/main/java/tech/ydb/table/values/Value.java +++ b/table/src/main/java/tech/ydb/table/values/Value.java @@ -8,7 +8,7 @@ * @author Sergey Polovko * @param type of value */ -public interface Value extends Serializable { +public interface Value extends Serializable, Comparable> { Value[] EMPTY_ARRAY = {}; @@ -16,6 +16,18 @@ public interface Value extends Serializable { ValueProtos.Value toPb(); + /** + * Compares this value with another value. + * The comparison is based on the actual data type of the value stored. + * For complex types like ListValue and StructValue, the comparison follows lexicographical rules. + * For OptionalValue, comparison with non-optional values of the same underlying type is supported. + * + * @param other the value to compare with + * @return a negative integer, zero, or a positive integer as this value is less than, equal to, or greater than the other value + * @throws IllegalArgumentException if the other value is null or has an incompatible type + */ + int compareTo(Value other); + default PrimitiveValue asData() { return (PrimitiveValue) this; } diff --git a/table/src/main/java/tech/ydb/table/values/VariantValue.java b/table/src/main/java/tech/ydb/table/values/VariantValue.java index a8fe231bb..d2f700b8d 100644 --- a/table/src/main/java/tech/ydb/table/values/VariantValue.java +++ b/table/src/main/java/tech/ydb/table/values/VariantValue.java @@ -70,4 +70,49 @@ public ValueProtos.Value toPb() { builder.setVariantIndex(typeIndex); return builder.build(); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof VariantValue)) { + throw new IllegalArgumentException("Cannot compare VariantValue with " + other.getClass().getSimpleName()); + } + + VariantValue otherVariant = (VariantValue) other; + + // Compare type indices first + int indexComparison = Integer.compare(typeIndex, otherVariant.typeIndex); + if (indexComparison != 0) { + return indexComparison; + } + + // If type indices are the same, compare the items + return compareValues(item, otherVariant.item); + } + + private static int compareValues(Value a, Value b) { + // Handle null values + if (a == null && b == null) return 0; + if (a == null) return -1; + if (b == null) return 1; + + // Check that the types are the same + if (!a.getType().equals(b.getType())) { + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getType() + " vs " + b.getType()); + } + + // Use the actual compareTo method of the values + if (a instanceof Comparable && b instanceof Comparable) { + try { + return ((Comparable>) a).compareTo((Value) b); + } catch (ClassCastException e) {} + } + + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + } } diff --git a/table/src/main/java/tech/ydb/table/values/VoidValue.java b/table/src/main/java/tech/ydb/table/values/VoidValue.java index ba96c879c..0e2239533 100644 --- a/table/src/main/java/tech/ydb/table/values/VoidValue.java +++ b/table/src/main/java/tech/ydb/table/values/VoidValue.java @@ -42,4 +42,18 @@ public VoidType getType() { public ValueProtos.Value toPb() { return ProtoValue.voidValue(); } + + @Override + public int compareTo(Value other) { + if (other == null) { + throw new IllegalArgumentException("Cannot compare with null value"); + } + + if (!(other instanceof VoidValue)) { + throw new IllegalArgumentException("Cannot compare VoidValue with " + other.getClass().getSimpleName()); + } + + // All VoidValue instances are equal + return 0; + } } diff --git a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java new file mode 100644 index 000000000..9ff82e045 --- /dev/null +++ b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java @@ -0,0 +1,295 @@ +package tech.ydb.table.values; + +import org.junit.Test; +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.List; + +/** + * Test for Comparable implementation of Value classes + */ +public class ValueComparableTest { + + @Test + public void testPrimitiveValueComparison() { + // Test numeric comparisons + PrimitiveValue int1 = PrimitiveValue.newInt32(1); + PrimitiveValue int2 = PrimitiveValue.newInt32(2); + PrimitiveValue int3 = PrimitiveValue.newInt32(1); + + assertTrue(int1.compareTo(int2) < 0); + assertTrue(int2.compareTo(int1) > 0); + assertEquals(0, int1.compareTo(int3)); + + // Test string comparisons + PrimitiveValue text1 = PrimitiveValue.newText("abc"); + PrimitiveValue text2 = PrimitiveValue.newText("def"); + PrimitiveValue text3 = PrimitiveValue.newText("abc"); + + assertTrue(text1.compareTo(text2) < 0); + assertTrue(text2.compareTo(text1) > 0); + assertEquals(0, text1.compareTo(text3)); + + // Test boolean comparisons + PrimitiveValue bool1 = PrimitiveValue.newBool(false); + PrimitiveValue bool2 = PrimitiveValue.newBool(true); + + assertTrue(bool1.compareTo(bool2) < 0); + assertTrue(bool2.compareTo(bool1) > 0); + } + + @Test + public void testListValueComparison() { + ListValue list1 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); + ListValue list2 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); + ListValue list3 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); + ListValue list4 = ListValue.of(PrimitiveValue.newInt32(1)); + + assertTrue(list1.compareTo(list2) < 0); + assertTrue(list2.compareTo(list1) > 0); + assertEquals(0, list1.compareTo(list3)); + assertTrue(list4.compareTo(list1) < 0); // shorter list comes first + } + + @Test + public void testListValueLexicographical() { + // Test proper lexicographical ordering + ListValue list1 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); + ListValue list2 = ListValue.of(PrimitiveValue.newText("Z")); + + // ('Z') should be "bigger" than ('A','Z') in lexicographical order + assertTrue(list1.compareTo(list2) < 0); // ('A','Z') < ('Z') + assertTrue(list2.compareTo(list1) > 0); // ('Z') > ('A','Z') + + // Test prefix ordering + ListValue list3 = ListValue.of(PrimitiveValue.newText("A")); + ListValue list4 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("B")); + + assertTrue(list3.compareTo(list4) < 0); // ('A') < ('A','B') + assertTrue(list4.compareTo(list3) > 0); // ('A','B') > ('A') + } + + @Test(expected = IllegalArgumentException.class) + public void testListValueDifferentTypes() { + ListValue list1 = ListValue.of(PrimitiveValue.newInt32(1)); + ListValue list2 = ListValue.of(PrimitiveValue.newText("abc")); + list1.compareTo(list2); // Should throw exception for different element types + } + + @Test + public void testStructValueComparison() { + StructValue struct1 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); + StructValue struct2 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(3)); + StructValue struct3 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); + + assertTrue(struct1.compareTo(struct2) < 0); + assertTrue(struct2.compareTo(struct1) > 0); + assertEquals(0, struct1.compareTo(struct3)); + } + + @Test + public void testStructValueLexicographical() { + // Test proper lexicographical ordering + StructValue struct1 = StructValue.of("a", PrimitiveValue.newText("A"), "b", PrimitiveValue.newText("Z")); + StructValue struct2 = StructValue.of("a", PrimitiveValue.newText("Z")); + + // ('Z') should be "bigger" than ('A','Z') in lexicographical order + assertTrue(struct1.compareTo(struct2) < 0); // ('A','Z') < ('Z') + assertTrue(struct2.compareTo(struct1) > 0); // ('Z') > ('A','Z') + } + + @Test(expected = IllegalArgumentException.class) + public void testStructValueDifferentTypes() { + StructValue struct1 = StructValue.of("a", PrimitiveValue.newInt32(1)); + StructValue struct2 = StructValue.of("a", PrimitiveValue.newText("abc")); + struct1.compareTo(struct2); // Should throw exception for different member types + } + + @Test + public void testDictValueComparison() { + DictValue dict1 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + DictValue dict2 = DictValue.of(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); + DictValue dict3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + + assertTrue(dict1.compareTo(dict2) < 0); + assertTrue(dict2.compareTo(dict1) > 0); + assertEquals(0, dict1.compareTo(dict3)); + } + + @Test(expected = IllegalArgumentException.class) + public void testDictValueDifferentTypes() { + DictValue dict1 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + DictValue dict2 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newText("abc")); + dict1.compareTo(dict2); // Should throw exception for different value types + } + + @Test + public void testOptionalValueComparison() { + OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); + OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newInt32(2)); + OptionalValue opt3 = OptionalValue.of(PrimitiveValue.newInt32(1)); + OptionalValue opt4 = OptionalValue.of(null); + + assertTrue(opt1.compareTo(opt2) < 0); + assertTrue(opt2.compareTo(opt1) > 0); + assertEquals(0, opt1.compareTo(opt3)); + assertTrue(opt4.compareTo(opt1) < 0); // empty values come first + } + + @Test(expected = IllegalArgumentException.class) + public void testOptionalValueDifferentTypes() { + OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); + OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newText("abc")); + opt1.compareTo(opt2); // Should throw exception for different item types + } + + @Test + public void testOptionalValueWithNonOptional() { + OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); + OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newInt32(2)); + OptionalValue opt3 = PrimitiveType.Int32.makeOptional().emptyValue(); + PrimitiveValue prim1 = PrimitiveValue.newInt32(1); + PrimitiveValue prim2 = PrimitiveValue.newInt32(2); + + // Optional with non-optional of same type + assertEquals(0, opt1.compareTo(prim1)); // Same value + assertTrue(opt1.compareTo(prim2) < 0); // Optional value less than non-optional + assertTrue(opt2.compareTo(prim1) > 0); // Optional value greater than non-optional + + // Empty optional with non-optional + assertTrue(opt3.compareTo(prim1) < 0); // Empty < non-empty + + // Non-optional with optional + assertEquals(0, prim1.compareTo(opt1)); // Same value + assertTrue(prim1.compareTo(opt2) < 0); // Non-optional less than optional + assertTrue(prim2.compareTo(opt1) > 0); // Non-optional greater than optional + assertTrue(prim1.compareTo(opt3) > 0); // Non-empty > empty + } + + @Test(expected = IllegalArgumentException.class) + public void testOptionalValueWithIncompatibleType() { + OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); + PrimitiveValue prim1 = PrimitiveValue.newText("abc"); + opt1.compareTo(prim1); // Should throw exception for incompatible types + } + + @Test + public void testTupleValueComparison() { + TupleValue tuple1 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); + TupleValue tuple2 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); + TupleValue tuple3 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); + TupleValue tuple4 = TupleValue.of(PrimitiveValue.newInt32(1)); + + assertTrue(tuple1.compareTo(tuple2) < 0); + assertTrue(tuple2.compareTo(tuple1) > 0); + assertEquals(0, tuple1.compareTo(tuple3)); + assertTrue(tuple4.compareTo(tuple1) < 0); // shorter tuple comes first + } + + @Test + public void testTupleValueLexicographical() { + // Test proper lexicographical ordering + TupleValue tuple1 = TupleValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); + TupleValue tuple2 = TupleValue.of(PrimitiveValue.newText("Z")); + + // ('Z') should be "bigger" than ('A','Z') in lexicographical order + assertTrue(tuple1.compareTo(tuple2) < 0); // ('A','Z') < ('Z') + assertTrue(tuple2.compareTo(tuple1) > 0); // ('Z') > ('A','Z') + } + + @Test(expected = IllegalArgumentException.class) + public void testTupleValueDifferentTypes() { + TupleValue tuple1 = TupleValue.of(PrimitiveValue.newInt32(1)); + TupleValue tuple2 = TupleValue.of(PrimitiveValue.newText("abc")); + tuple1.compareTo(tuple2); // Should throw exception for different element types + } + + @Test + public void testVariantValueComparison() { + VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + PrimitiveValue.newInt32(1), 0); + VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + PrimitiveValue.newText("abc"), 1); + VariantValue variant3 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + PrimitiveValue.newInt32(2), 0); + + assertTrue(variant1.compareTo(variant2) < 0); // type index 0 < 1 + assertTrue(variant2.compareTo(variant1) > 0); + assertTrue(variant1.compareTo(variant3) < 0); // same type index, compare values + } + + @Test(expected = IllegalArgumentException.class) + public void testVariantValueDifferentTypes() { + VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + PrimitiveValue.newInt32(1), 0); + VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + PrimitiveValue.newText("abc"), 0); + variant1.compareTo(variant2); // Should throw exception for different item types + } + + @Test + public void testVoidValueComparison() { + VoidValue void1 = VoidValue.of(); + VoidValue void2 = VoidValue.of(); + + assertEquals(0, void1.compareTo(void2)); + } + + @Test + public void testNullValueComparison() { + NullValue null1 = NullValue.of(); + NullValue null2 = NullValue.of(); + + assertEquals(0, null1.compareTo(null2)); + } + + @Test + public void testDecimalValueComparison() { + DecimalValue decimal1 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); + DecimalValue decimal2 = DecimalValue.fromLong(DecimalType.of(10, 2), 200); + DecimalValue decimal3 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); + + assertTrue(decimal1.compareTo(decimal2) < 0); + assertTrue(decimal2.compareTo(decimal1) > 0); + assertEquals(0, decimal1.compareTo(decimal3)); + } + + @Test(expected = IllegalArgumentException.class) + public void testCompareWithNull() { + PrimitiveValue value = PrimitiveValue.newInt32(1); + value.compareTo(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testCompareDifferentTypes() { + PrimitiveValue intValue = PrimitiveValue.newInt32(1); + PrimitiveValue textValue = PrimitiveValue.newText("abc"); + intValue.compareTo(textValue); + } + + @Test + public void testSorting() { + List> values = Arrays.asList( + PrimitiveValue.newInt32(3), + PrimitiveValue.newInt32(1), + PrimitiveValue.newInt32(2), + PrimitiveValue.newText("abc"), + PrimitiveValue.newText("aaa") + ); + + values.sort((a, b) -> { + // Compare by type kind first, then by value + int typeComparison = a.getType().getKind().compareTo(b.getType().getKind()); + if (typeComparison != 0) { + return typeComparison; + } + return a.toString().compareTo(b.toString()); + }); + + // Verify sorting worked + assertTrue(values.get(0).toString().contains("1")); + assertTrue(values.get(1).toString().contains("2")); + assertTrue(values.get(2).toString().contains("3")); + } +} \ No newline at end of file From c5b7231986236c94e28a48f0f66673db92577ba4 Mon Sep 17 00:00:00 2001 From: Maksim Zinal Date: Wed, 30 Jul 2025 23:27:52 +0300 Subject: [PATCH 09/17] proper tests --- .../tech/ydb/table/values/DecimalValue.java | 36 +++++++--- .../java/tech/ydb/table/values/DictValue.java | 68 +++++++++++++------ .../java/tech/ydb/table/values/ListValue.java | 58 ++++++++++++---- .../java/tech/ydb/table/values/NullValue.java | 24 ++++++- .../tech/ydb/table/values/OptionalValue.java | 27 ++++---- .../tech/ydb/table/values/PrimitiveValue.java | 32 +++++++-- .../tech/ydb/table/values/StructValue.java | 58 ++++++++++++---- .../tech/ydb/table/values/TupleValue.java | 58 ++++++++++++---- .../java/tech/ydb/table/values/Value.java | 2 +- .../tech/ydb/table/values/VariantValue.java | 56 +++++++++++---- .../java/tech/ydb/table/values/VoidValue.java | 24 ++++++- .../ydb/table/values/ValueComparableTest.java | 30 +------- 12 files changed, 335 insertions(+), 138 deletions(-) diff --git a/table/src/main/java/tech/ydb/table/values/DecimalValue.java b/table/src/main/java/tech/ydb/table/values/DecimalValue.java index ea48ca858..41464acf4 100644 --- a/table/src/main/java/tech/ydb/table/values/DecimalValue.java +++ b/table/src/main/java/tech/ydb/table/values/DecimalValue.java @@ -274,13 +274,33 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this decimal type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare DecimalValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof DecimalValue)) { throw new IllegalArgumentException("Cannot compare DecimalValue with " + other.getClass().getSimpleName()); } - + DecimalValue otherDecimal = (DecimalValue) other; - + // Handle special values first if (isNan() || otherDecimal.isNan()) { // NaN is not comparable, but we need to provide a consistent ordering @@ -292,7 +312,7 @@ public int compareTo(Value other) { } return -1; } - + if (isInf() && otherDecimal.isInf()) { return 0; } @@ -302,7 +322,7 @@ public int compareTo(Value other) { if (otherDecimal.isInf()) { return -1; } - + if (isNegativeInf() && otherDecimal.isNegativeInf()) { return 0; } @@ -312,18 +332,18 @@ public int compareTo(Value other) { if (otherDecimal.isNegativeInf()) { return 1; } - + // Compare finite values if (isNegative() != otherDecimal.isNegative()) { return isNegative() ? -1 : 1; } - + // Both have the same sign, compare magnitudes int highComparison = Long.compareUnsigned(high, otherDecimal.high); if (highComparison != 0) { return isNegative() ? -highComparison : highComparison; } - + int lowComparison = Long.compareUnsigned(low, otherDecimal.low); return isNegative() ? -lowComparison : lowComparison; } diff --git a/table/src/main/java/tech/ydb/table/values/DictValue.java b/table/src/main/java/tech/ydb/table/values/DictValue.java index 6ba9db076..61fc20372 100644 --- a/table/src/main/java/tech/ydb/table/values/DictValue.java +++ b/table/src/main/java/tech/ydb/table/values/DictValue.java @@ -124,23 +124,43 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this dict type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare DictValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof DictValue)) { throw new IllegalArgumentException("Cannot compare DictValue with " + other.getClass().getSimpleName()); } - + DictValue otherDict = (DictValue) other; - + // Compare sizes first int sizeComparison = Integer.compare(items.size(), otherDict.items.size()); if (sizeComparison != 0) { return sizeComparison; } - + // Convert to sorted lists for lexicographical comparison List, Value>> thisEntries = new ArrayList<>(items.entrySet()); List, Value>> otherEntries = new ArrayList<>(otherDict.items.entrySet()); - + // Sort entries by key first, then by value thisEntries.sort((e1, e2) -> { int keyComparison = compareValues(e1.getKey(), e2.getKey()); @@ -149,7 +169,7 @@ public int compareTo(Value other) { } return compareValues(e1.getValue(), e2.getValue()); }); - + otherEntries.sort((e1, e2) -> { int keyComparison = compareValues(e1.getKey(), e2.getKey()); if (keyComparison != 0) { @@ -157,46 +177,54 @@ public int compareTo(Value other) { } return compareValues(e1.getValue(), e2.getValue()); }); - + // Compare sorted entries for (int i = 0; i < thisEntries.size(); i++) { Map.Entry, Value> thisEntry = thisEntries.get(i); Map.Entry, Value> otherEntry = otherEntries.get(i); - + int keyComparison = compareValues(thisEntry.getKey(), otherEntry.getKey()); if (keyComparison != 0) { return keyComparison; } - + int valueComparison = compareValues(thisEntry.getValue(), otherEntry.getValue()); if (valueComparison != 0) { return valueComparison; } } - + return 0; } - + private static int compareValues(Value a, Value b) { // Handle null values - if (a == null && b == null) return 0; - if (a == null) return -1; - if (b == null) return 1; - + if (a == null && b == null) { + return 0; + } + if (a == null) { + return -1; + } + if (b == null) { + return 1; + } + // Check that the types are the same if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getType() + " vs " + b.getType()); } - + // Use the actual compareTo method of the values if (a instanceof Comparable && b instanceof Comparable) { try { return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) {} + } catch (ClassCastException e) { + // Fall back to error + } } - - throw new IllegalArgumentException("Cannot compare values of different types: " + + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); } } diff --git a/table/src/main/java/tech/ydb/table/values/ListValue.java b/table/src/main/java/tech/ydb/table/values/ListValue.java index 38a11703e..f34f9419e 100644 --- a/table/src/main/java/tech/ydb/table/values/ListValue.java +++ b/table/src/main/java/tech/ydb/table/values/ListValue.java @@ -119,50 +119,78 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this list type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare ListValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof ListValue)) { throw new IllegalArgumentException("Cannot compare ListValue with " + other.getClass().getSimpleName()); } - + ListValue otherList = (ListValue) other; - + // Compare elements lexicographically int minLength = Math.min(items.length, otherList.items.length); for (int i = 0; i < minLength; i++) { Value thisItem = items[i]; Value otherItem = otherList.items[i]; - + int itemComparison = compareValues(thisItem, otherItem); if (itemComparison != 0) { return itemComparison; } } - + // If we reach here, one list is a prefix of the other // The shorter list comes first return Integer.compare(items.length, otherList.items.length); } - + private static int compareValues(Value a, Value b) { // Handle null values - if (a == null && b == null) return 0; - if (a == null) return -1; - if (b == null) return 1; - + if (a == null && b == null) { + return 0; + } + if (a == null) { + return -1; + } + if (b == null) { + return 1; + } + // Check that the types are the same if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getType() + " vs " + b.getType()); } - + // Use the actual compareTo method of the values if (a instanceof Comparable && b instanceof Comparable) { try { return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) {} + } catch (ClassCastException e) { + // Fall back to error + } } - - throw new IllegalArgumentException("Cannot compare values of different types: " + + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); } } diff --git a/table/src/main/java/tech/ydb/table/values/NullValue.java b/table/src/main/java/tech/ydb/table/values/NullValue.java index 352ef1baa..b106688b2 100644 --- a/table/src/main/java/tech/ydb/table/values/NullValue.java +++ b/table/src/main/java/tech/ydb/table/values/NullValue.java @@ -38,11 +38,31 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this null type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare NullValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof NullValue)) { throw new IllegalArgumentException("Cannot compare NullValue with " + other.getClass().getSimpleName()); } - + // All NullValue instances are equal return 0; } diff --git a/table/src/main/java/tech/ydb/table/values/OptionalValue.java b/table/src/main/java/tech/ydb/table/values/OptionalValue.java index 8d31dc69c..dde545c60 100644 --- a/table/src/main/java/tech/ydb/table/values/OptionalValue.java +++ b/table/src/main/java/tech/ydb/table/values/OptionalValue.java @@ -102,17 +102,17 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + // Handle comparison with another OptionalValue if (other instanceof OptionalValue) { OptionalValue otherOptional = (OptionalValue) other; - + // Check that the item types are the same if (!type.getItemType().equals(otherOptional.type.getItemType())) { - throw new IllegalArgumentException("Cannot compare OptionalValue with different item types: " + + throw new IllegalArgumentException("Cannot compare OptionalValue with different item types: " + type.getItemType() + " vs " + otherOptional.type.getItemType()); } - + // Handle empty values: empty values are considered less than non-empty values if (value == null && otherOptional.value == null) { return 0; @@ -123,35 +123,38 @@ public int compareTo(Value other) { if (otherOptional.value == null) { return 1; } - + // Both values are non-null and have the same type, compare them using their compareTo method return compareValues(value, otherOptional.value); } - + // Handle comparison with non-optional values of the same underlying type if (type.getItemType().equals(other.getType())) { // This OptionalValue is empty, so it's less than any non-optional value if (value == null) { return -1; } - + // This OptionalValue has a value, compare it with the non-optional value return compareValues(value, other); } - + // Types are incompatible - throw new IllegalArgumentException("Cannot compare OptionalValue with incompatible type: " + + throw new IllegalArgumentException("Cannot compare OptionalValue with incompatible type: " + type.getItemType() + " vs " + other.getType()); } - + private static int compareValues(Value a, Value b) { // Since we've already verified the types are the same, we can safely cast // and use the compareTo method of the actual value type if (a instanceof Comparable && b instanceof Comparable) { try { return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) {} + } catch (ClassCastException e) { + // Fall back to error + } } - throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + throw new IllegalArgumentException("Cannot compare values of different types: " + + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); } } diff --git a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java index 1c61c84f9..9613f1d7d 100644 --- a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java +++ b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java @@ -439,16 +439,38 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this primitive type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare PrimitiveValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof PrimitiveValue)) { - throw new IllegalArgumentException("Cannot compare PrimitiveValue with " + other.getClass().getSimpleName()); + throw new IllegalArgumentException( + "Cannot compare PrimitiveValue with " + other.getClass().getSimpleName()); } - + PrimitiveValue otherPrimitive = (PrimitiveValue) other; if (getType() != otherPrimitive.getType()) { - throw new IllegalArgumentException("Cannot compare values of different types: " + getType() + " vs " + otherPrimitive.getType()); + throw new IllegalArgumentException("Cannot compare values of different types: " + + getType() + " vs " + otherPrimitive.getType()); } - + // Compare based on the actual primitive type switch (getType()) { case Bool: diff --git a/table/src/main/java/tech/ydb/table/values/StructValue.java b/table/src/main/java/tech/ydb/table/values/StructValue.java index 5b3f44b28..196771c2d 100644 --- a/table/src/main/java/tech/ydb/table/values/StructValue.java +++ b/table/src/main/java/tech/ydb/table/values/StructValue.java @@ -139,50 +139,78 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this struct type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare StructValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof StructValue)) { throw new IllegalArgumentException("Cannot compare StructValue with " + other.getClass().getSimpleName()); } - + StructValue otherStruct = (StructValue) other; - + // Compare members lexicographically int minLength = Math.min(members.length, otherStruct.members.length); for (int i = 0; i < minLength; i++) { Value thisMember = members[i]; Value otherMember = otherStruct.members[i]; - + int memberComparison = compareValues(thisMember, otherMember); if (memberComparison != 0) { return memberComparison; } } - + // If we reach here, one struct is a prefix of the other // The shorter struct comes first return Integer.compare(members.length, otherStruct.members.length); } - + private static int compareValues(Value a, Value b) { // Handle null values - if (a == null && b == null) return 0; - if (a == null) return -1; - if (b == null) return 1; - + if (a == null && b == null) { + return 0; + } + if (a == null) { + return -1; + } + if (b == null) { + return 1; + } + // Check that the types are the same if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getType() + " vs " + b.getType()); } - + // Use the actual compareTo method of the values if (a instanceof Comparable && b instanceof Comparable) { try { return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) {} + } catch (ClassCastException e) { + // Fall back to error + } } - - throw new IllegalArgumentException("Cannot compare values of different types: " + + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); } diff --git a/table/src/main/java/tech/ydb/table/values/TupleValue.java b/table/src/main/java/tech/ydb/table/values/TupleValue.java index f2a6f1943..00f16ed45 100644 --- a/table/src/main/java/tech/ydb/table/values/TupleValue.java +++ b/table/src/main/java/tech/ydb/table/values/TupleValue.java @@ -151,50 +151,78 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this tuple type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare TupleValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof TupleValue)) { throw new IllegalArgumentException("Cannot compare TupleValue with " + other.getClass().getSimpleName()); } - + TupleValue otherTuple = (TupleValue) other; - + // Compare elements lexicographically int minLength = Math.min(items.length, otherTuple.items.length); for (int i = 0; i < minLength; i++) { Value thisItem = items[i]; Value otherItem = otherTuple.items[i]; - + int itemComparison = compareValues(thisItem, otherItem); if (itemComparison != 0) { return itemComparison; } } - + // If we reach here, one tuple is a prefix of the other // The shorter tuple comes first return Integer.compare(items.length, otherTuple.items.length); } - + private static int compareValues(Value a, Value b) { // Handle null values - if (a == null && b == null) return 0; - if (a == null) return -1; - if (b == null) return 1; - + if (a == null && b == null) { + return 0; + } + if (a == null) { + return -1; + } + if (b == null) { + return 1; + } + // Check that the types are the same if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getType() + " vs " + b.getType()); } - + // Use the actual compareTo method of the values if (a instanceof Comparable && b instanceof Comparable) { try { return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) {} + } catch (ClassCastException e) { + // Fall back to error + } } - - throw new IllegalArgumentException("Cannot compare values of different types: " + + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); } } diff --git a/table/src/main/java/tech/ydb/table/values/Value.java b/table/src/main/java/tech/ydb/table/values/Value.java index a20a41df7..d21867089 100644 --- a/table/src/main/java/tech/ydb/table/values/Value.java +++ b/table/src/main/java/tech/ydb/table/values/Value.java @@ -21,7 +21,7 @@ public interface Value extends Serializable, Comparable * The comparison is based on the actual data type of the value stored. * For complex types like ListValue and StructValue, the comparison follows lexicographical rules. * For OptionalValue, comparison with non-optional values of the same underlying type is supported. - * + * * @param other the value to compare with * @return a negative integer, zero, or a positive integer as this value is less than, equal to, or greater than the other value * @throws IllegalArgumentException if the other value is null or has an incompatible type diff --git a/table/src/main/java/tech/ydb/table/values/VariantValue.java b/table/src/main/java/tech/ydb/table/values/VariantValue.java index d2f700b8d..dda7842e1 100644 --- a/table/src/main/java/tech/ydb/table/values/VariantValue.java +++ b/table/src/main/java/tech/ydb/table/values/VariantValue.java @@ -76,43 +76,71 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this variant type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare VariantValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof VariantValue)) { throw new IllegalArgumentException("Cannot compare VariantValue with " + other.getClass().getSimpleName()); } - + VariantValue otherVariant = (VariantValue) other; - + // Compare type indices first int indexComparison = Integer.compare(typeIndex, otherVariant.typeIndex); if (indexComparison != 0) { return indexComparison; } - + // If type indices are the same, compare the items return compareValues(item, otherVariant.item); } - + private static int compareValues(Value a, Value b) { // Handle null values - if (a == null && b == null) return 0; - if (a == null) return -1; - if (b == null) return 1; - + if (a == null && b == null) { + return 0; + } + if (a == null) { + return -1; + } + if (b == null) { + return 1; + } + // Check that the types are the same if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getType() + " vs " + b.getType()); } - + // Use the actual compareTo method of the values if (a instanceof Comparable && b instanceof Comparable) { try { return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) {} + } catch (ClassCastException e) { + // Fall back to error + } } - - throw new IllegalArgumentException("Cannot compare values of different types: " + + + throw new IllegalArgumentException("Cannot compare values of different types: " + a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); } } diff --git a/table/src/main/java/tech/ydb/table/values/VoidValue.java b/table/src/main/java/tech/ydb/table/values/VoidValue.java index 0e2239533..7997604e2 100644 --- a/table/src/main/java/tech/ydb/table/values/VoidValue.java +++ b/table/src/main/java/tech/ydb/table/values/VoidValue.java @@ -48,11 +48,31 @@ public int compareTo(Value other) { if (other == null) { throw new IllegalArgumentException("Cannot compare with null value"); } - + + // Handle comparison with OptionalValue + if (other instanceof OptionalValue) { + OptionalValue otherOptional = (OptionalValue) other; + + // Check that the item type matches this void type + if (!getType().equals(otherOptional.getType().getItemType())) { + throw new IllegalArgumentException( + "Cannot compare VoidValue with OptionalValue of different item type: " + + getType() + " vs " + otherOptional.getType().getItemType()); + } + + // Non-empty value is greater than empty optional + if (!otherOptional.isPresent()) { + return 1; + } + + // Compare with the wrapped value + return compareTo(otherOptional.get()); + } + if (!(other instanceof VoidValue)) { throw new IllegalArgumentException("Cannot compare VoidValue with " + other.getClass().getSimpleName()); } - + // All VoidValue instances are equal return 0; } diff --git a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java index 9ff82e045..c5e0ad984 100644 --- a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java +++ b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java @@ -3,9 +3,6 @@ import org.junit.Test; import static org.junit.Assert.*; -import java.util.Arrays; -import java.util.List; - /** * Test for Comparable implementation of Value classes */ @@ -129,7 +126,7 @@ public void testOptionalValueComparison() { OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newInt32(2)); OptionalValue opt3 = OptionalValue.of(PrimitiveValue.newInt32(1)); - OptionalValue opt4 = OptionalValue.of(null); + OptionalValue opt4 = PrimitiveType.Int32.makeOptional().emptyValue(); assertTrue(opt1.compareTo(opt2) < 0); assertTrue(opt2.compareTo(opt1) > 0); @@ -267,29 +264,4 @@ public void testCompareDifferentTypes() { PrimitiveValue textValue = PrimitiveValue.newText("abc"); intValue.compareTo(textValue); } - - @Test - public void testSorting() { - List> values = Arrays.asList( - PrimitiveValue.newInt32(3), - PrimitiveValue.newInt32(1), - PrimitiveValue.newInt32(2), - PrimitiveValue.newText("abc"), - PrimitiveValue.newText("aaa") - ); - - values.sort((a, b) -> { - // Compare by type kind first, then by value - int typeComparison = a.getType().getKind().compareTo(b.getType().getKind()); - if (typeComparison != 0) { - return typeComparison; - } - return a.toString().compareTo(b.toString()); - }); - - // Verify sorting worked - assertTrue(values.get(0).toString().contains("1")); - assertTrue(values.get(1).toString().contains("2")); - assertTrue(values.get(2).toString().contains("3")); - } } \ No newline at end of file From a299385fb5f29f97d3d900656add6435c65e10c6 Mon Sep 17 00:00:00 2001 From: Maksim Zinal Date: Wed, 30 Jul 2025 23:35:12 +0300 Subject: [PATCH 10/17] fixed PrimitiveValues comparisons --- .../tech/ydb/table/values/PrimitiveValue.java | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java index 9613f1d7d..5e9d84b70 100644 --- a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java +++ b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java @@ -499,27 +499,35 @@ public int compareTo(Value other) { case Yson: return compareByteArrays(getBytesUnsafe(), otherPrimitive.getBytesUnsafe()); case Text: + return getText().compareTo(otherPrimitive.getText()); case Json: + return getJson().compareTo(otherPrimitive.getJson()); case JsonDocument: - return getText().compareTo(otherPrimitive.getText()); + return getJsonDocument().compareTo(otherPrimitive.getJsonDocument()); case Uuid: return getUuidJdk().compareTo(otherPrimitive.getUuidJdk()); case Date: - case Date32: return getDate().compareTo(otherPrimitive.getDate()); + case Date32: + return getDate32().compareTo(otherPrimitive.getDate32()); case Datetime: - case Datetime64: return getDatetime().compareTo(otherPrimitive.getDatetime()); + case Datetime64: + return getDatetime64().compareTo(otherPrimitive.getDatetime64()); case Timestamp: - case Timestamp64: return getTimestamp().compareTo(otherPrimitive.getTimestamp()); + case Timestamp64: + return getTimestamp64().compareTo(otherPrimitive.getTimestamp64()); case Interval: - case Interval64: return getInterval().compareTo(otherPrimitive.getInterval()); + case Interval64: + return getInterval64().compareTo(otherPrimitive.getInterval64()); case TzDate: + return getTzDate().compareTo(otherPrimitive.getTzDate()); case TzDatetime: + return getTzDatetime().compareTo(otherPrimitive.getTzDatetime()); case TzTimestamp: - return getTzDate().compareTo(otherPrimitive.getTzDate()); + return getTzTimestamp().compareTo(otherPrimitive.getTzTimestamp()); default: throw new UnsupportedOperationException("Comparison not supported for type: " + getType()); } From 85785c5c04324a9b06399965dd42be271a106792 Mon Sep 17 00:00:00 2001 From: Maksim Zinal Date: Wed, 30 Jul 2025 23:46:28 +0300 Subject: [PATCH 11/17] proper comparison for DictValue --- .../java/tech/ydb/table/values/DictValue.java | 15 +-- .../ydb/table/values/ValueComparableTest.java | 109 +++++++++++++----- 2 files changed, 87 insertions(+), 37 deletions(-) diff --git a/table/src/main/java/tech/ydb/table/values/DictValue.java b/table/src/main/java/tech/ydb/table/values/DictValue.java index 61fc20372..e2069d926 100644 --- a/table/src/main/java/tech/ydb/table/values/DictValue.java +++ b/table/src/main/java/tech/ydb/table/values/DictValue.java @@ -151,12 +151,6 @@ public int compareTo(Value other) { DictValue otherDict = (DictValue) other; - // Compare sizes first - int sizeComparison = Integer.compare(items.size(), otherDict.items.size()); - if (sizeComparison != 0) { - return sizeComparison; - } - // Convert to sorted lists for lexicographical comparison List, Value>> thisEntries = new ArrayList<>(items.entrySet()); List, Value>> otherEntries = new ArrayList<>(otherDict.items.entrySet()); @@ -178,8 +172,9 @@ public int compareTo(Value other) { return compareValues(e1.getValue(), e2.getValue()); }); - // Compare sorted entries - for (int i = 0; i < thisEntries.size(); i++) { + // Compare sorted entries lexicographically + int minLength = Math.min(thisEntries.size(), otherEntries.size()); + for (int i = 0; i < minLength; i++) { Map.Entry, Value> thisEntry = thisEntries.get(i); Map.Entry, Value> otherEntry = otherEntries.get(i); @@ -194,7 +189,9 @@ public int compareTo(Value other) { } } - return 0; + // If we reach here, one dict is a prefix of the other + // The shorter dict comes first + return Integer.compare(thisEntries.size(), otherEntries.size()); } private static int compareValues(Value a, Value b) { diff --git a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java index c5e0ad984..8fbc9e57f 100644 --- a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java +++ b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java @@ -1,5 +1,7 @@ package tech.ydb.table.values; +import java.util.HashMap; +import java.util.Map; import org.junit.Test; import static org.junit.Assert.*; @@ -14,24 +16,24 @@ public void testPrimitiveValueComparison() { PrimitiveValue int1 = PrimitiveValue.newInt32(1); PrimitiveValue int2 = PrimitiveValue.newInt32(2); PrimitiveValue int3 = PrimitiveValue.newInt32(1); - + assertTrue(int1.compareTo(int2) < 0); assertTrue(int2.compareTo(int1) > 0); assertEquals(0, int1.compareTo(int3)); - + // Test string comparisons PrimitiveValue text1 = PrimitiveValue.newText("abc"); PrimitiveValue text2 = PrimitiveValue.newText("def"); PrimitiveValue text3 = PrimitiveValue.newText("abc"); - + assertTrue(text1.compareTo(text2) < 0); assertTrue(text2.compareTo(text1) > 0); assertEquals(0, text1.compareTo(text3)); - + // Test boolean comparisons PrimitiveValue bool1 = PrimitiveValue.newBool(false); PrimitiveValue bool2 = PrimitiveValue.newBool(true); - + assertTrue(bool1.compareTo(bool2) < 0); assertTrue(bool2.compareTo(bool1) > 0); } @@ -42,7 +44,7 @@ public void testListValueComparison() { ListValue list2 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); ListValue list3 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); ListValue list4 = ListValue.of(PrimitiveValue.newInt32(1)); - + assertTrue(list1.compareTo(list2) < 0); assertTrue(list2.compareTo(list1) > 0); assertEquals(0, list1.compareTo(list3)); @@ -54,15 +56,15 @@ public void testListValueLexicographical() { // Test proper lexicographical ordering ListValue list1 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); ListValue list2 = ListValue.of(PrimitiveValue.newText("Z")); - + // ('Z') should be "bigger" than ('A','Z') in lexicographical order assertTrue(list1.compareTo(list2) < 0); // ('A','Z') < ('Z') assertTrue(list2.compareTo(list1) > 0); // ('Z') > ('A','Z') - + // Test prefix ordering ListValue list3 = ListValue.of(PrimitiveValue.newText("A")); ListValue list4 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("B")); - + assertTrue(list3.compareTo(list4) < 0); // ('A') < ('A','B') assertTrue(list4.compareTo(list3) > 0); // ('A','B') > ('A') } @@ -79,7 +81,7 @@ public void testStructValueComparison() { StructValue struct1 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); StructValue struct2 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(3)); StructValue struct3 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); - + assertTrue(struct1.compareTo(struct2) < 0); assertTrue(struct2.compareTo(struct1) > 0); assertEquals(0, struct1.compareTo(struct3)); @@ -90,7 +92,7 @@ public void testStructValueLexicographical() { // Test proper lexicographical ordering StructValue struct1 = StructValue.of("a", PrimitiveValue.newText("A"), "b", PrimitiveValue.newText("Z")); StructValue struct2 = StructValue.of("a", PrimitiveValue.newText("Z")); - + // ('Z') should be "bigger" than ('A','Z') in lexicographical order assertTrue(struct1.compareTo(struct2) < 0); // ('A','Z') < ('Z') assertTrue(struct2.compareTo(struct1) > 0); // ('Z') > ('A','Z') @@ -108,7 +110,7 @@ public void testDictValueComparison() { DictValue dict1 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); DictValue dict2 = DictValue.of(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); DictValue dict3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - + assertTrue(dict1.compareTo(dict2) < 0); assertTrue(dict2.compareTo(dict1) > 0); assertEquals(0, dict1.compareTo(dict3)); @@ -121,13 +123,64 @@ public void testDictValueDifferentTypes() { dict1.compareTo(dict2); // Should throw exception for different value types } + @Test + public void testDictValueLexicographical() { + // Test proper lexicographical ordering - content matters more than size + + // Case 1: Shorter dict with "larger" key should be greater than longer dict with "smaller" keys + Map, Value> map1 = new HashMap<>(); + map1.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + map1.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(2)); + DictValue dict1 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map1); + + DictValue dict2 = DictValue.of(PrimitiveValue.newText("z"), PrimitiveValue.newInt32(1)); + + // {"z": 1} should be "bigger" than {"a": 1, "b": 2} in lexicographical order + assertTrue(dict1.compareTo(dict2) < 0); // {"a": 1, "b": 2} < {"z": 1} + assertTrue(dict2.compareTo(dict1) > 0); // {"z": 1} > {"a": 1, "b": 2} + + // Case 2: Same keys, different values - value comparison matters + DictValue dict3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + DictValue dict4 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(2)); + + assertTrue(dict3.compareTo(dict4) < 0); // {"a": 1} < {"a": 2} + assertTrue(dict4.compareTo(dict3) > 0); // {"a": 2} > {"a": 1} + + // Case 3: One dict is a prefix of another - shorter comes first only if it's a prefix + DictValue dict5 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + + Map, Value> map6 = new HashMap<>(); + map6.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + map6.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(2)); + DictValue dict6 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map6); + + assertTrue(dict5.compareTo(dict6) < 0); // {"a": 1} < {"a": 1, "b": 2} (prefix case) + assertTrue(dict6.compareTo(dict5) > 0); // {"a": 1, "b": 2} > {"a": 1} + + // Case 4: Multiple entries with different ordering + Map, Value> map7 = new HashMap<>(); + map7.put(PrimitiveValue.newText("x"), PrimitiveValue.newInt32(1)); + map7.put(PrimitiveValue.newText("y"), PrimitiveValue.newInt32(1)); + DictValue dict7 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map7); + + Map, Value> map8 = new HashMap<>(); + map8.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + map8.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); + map8.put(PrimitiveValue.newText("c"), PrimitiveValue.newInt32(1)); + DictValue dict8 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map8); + + // {"x": 1, "y": 1} should be greater than {"a": 1, "b": 1, "c": 1} despite being shorter + assertTrue(dict8.compareTo(dict7) < 0); // {"a": 1, "b": 1, "c": 1} < {"x": 1, "y": 1} + assertTrue(dict7.compareTo(dict8) > 0); // {"x": 1, "y": 1} > {"a": 1, "b": 1, "c": 1} + } + @Test public void testOptionalValueComparison() { OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newInt32(2)); OptionalValue opt3 = OptionalValue.of(PrimitiveValue.newInt32(1)); OptionalValue opt4 = PrimitiveType.Int32.makeOptional().emptyValue(); - + assertTrue(opt1.compareTo(opt2) < 0); assertTrue(opt2.compareTo(opt1) > 0); assertEquals(0, opt1.compareTo(opt3)); @@ -148,15 +201,15 @@ public void testOptionalValueWithNonOptional() { OptionalValue opt3 = PrimitiveType.Int32.makeOptional().emptyValue(); PrimitiveValue prim1 = PrimitiveValue.newInt32(1); PrimitiveValue prim2 = PrimitiveValue.newInt32(2); - + // Optional with non-optional of same type assertEquals(0, opt1.compareTo(prim1)); // Same value assertTrue(opt1.compareTo(prim2) < 0); // Optional value less than non-optional assertTrue(opt2.compareTo(prim1) > 0); // Optional value greater than non-optional - + // Empty optional with non-optional assertTrue(opt3.compareTo(prim1) < 0); // Empty < non-empty - + // Non-optional with optional assertEquals(0, prim1.compareTo(opt1)); // Same value assertTrue(prim1.compareTo(opt2) < 0); // Non-optional less than optional @@ -177,7 +230,7 @@ public void testTupleValueComparison() { TupleValue tuple2 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); TupleValue tuple3 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); TupleValue tuple4 = TupleValue.of(PrimitiveValue.newInt32(1)); - + assertTrue(tuple1.compareTo(tuple2) < 0); assertTrue(tuple2.compareTo(tuple1) > 0); assertEquals(0, tuple1.compareTo(tuple3)); @@ -189,7 +242,7 @@ public void testTupleValueLexicographical() { // Test proper lexicographical ordering TupleValue tuple1 = TupleValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); TupleValue tuple2 = TupleValue.of(PrimitiveValue.newText("Z")); - + // ('Z') should be "bigger" than ('A','Z') in lexicographical order assertTrue(tuple1.compareTo(tuple2) < 0); // ('A','Z') < ('Z') assertTrue(tuple2.compareTo(tuple1) > 0); // ('Z') > ('A','Z') @@ -204,13 +257,13 @@ public void testTupleValueDifferentTypes() { @Test public void testVariantValueComparison() { - VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), PrimitiveValue.newInt32(1), 0); - VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), PrimitiveValue.newText("abc"), 1); - VariantValue variant3 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + VariantValue variant3 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), PrimitiveValue.newInt32(2), 0); - + assertTrue(variant1.compareTo(variant2) < 0); // type index 0 < 1 assertTrue(variant2.compareTo(variant1) > 0); assertTrue(variant1.compareTo(variant3) < 0); // same type index, compare values @@ -218,9 +271,9 @@ public void testVariantValueComparison() { @Test(expected = IllegalArgumentException.class) public void testVariantValueDifferentTypes() { - VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), PrimitiveValue.newInt32(1), 0); - VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), + VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), PrimitiveValue.newText("abc"), 0); variant1.compareTo(variant2); // Should throw exception for different item types } @@ -229,7 +282,7 @@ public void testVariantValueDifferentTypes() { public void testVoidValueComparison() { VoidValue void1 = VoidValue.of(); VoidValue void2 = VoidValue.of(); - + assertEquals(0, void1.compareTo(void2)); } @@ -237,7 +290,7 @@ public void testVoidValueComparison() { public void testNullValueComparison() { NullValue null1 = NullValue.of(); NullValue null2 = NullValue.of(); - + assertEquals(0, null1.compareTo(null2)); } @@ -246,7 +299,7 @@ public void testDecimalValueComparison() { DecimalValue decimal1 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); DecimalValue decimal2 = DecimalValue.fromLong(DecimalType.of(10, 2), 200); DecimalValue decimal3 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); - + assertTrue(decimal1.compareTo(decimal2) < 0); assertTrue(decimal2.compareTo(decimal1) > 0); assertEquals(0, decimal1.compareTo(decimal3)); @@ -264,4 +317,4 @@ public void testCompareDifferentTypes() { PrimitiveValue textValue = PrimitiveValue.newText("abc"); intValue.compareTo(textValue); } -} \ No newline at end of file +} \ No newline at end of file From e01888918bb4110dab812630ab24de852ef1383c Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 11 Aug 2025 10:50:50 +0100 Subject: [PATCH 12/17] Update impelementation of compareTo for Value --- .../tech/ydb/table/values/DecimalValue.java | 43 +++----- .../java/tech/ydb/table/values/DictValue.java | 103 ++++-------------- .../java/tech/ydb/table/values/ListValue.java | 71 ++---------- .../java/tech/ydb/table/values/NullValue.java | 26 ++--- .../tech/ydb/table/values/OptionalValue.java | 59 ++-------- .../tech/ydb/table/values/PrimitiveValue.java | 87 ++++++--------- .../tech/ydb/table/values/StructValue.java | 73 ++----------- .../tech/ydb/table/values/TupleValue.java | 70 ++---------- .../java/tech/ydb/table/values/Value.java | 12 -- .../tech/ydb/table/values/VariantType.java | 10 +- .../tech/ydb/table/values/VariantValue.java | 32 ++---- .../java/tech/ydb/table/values/VoidType.java | 10 -- .../java/tech/ydb/table/values/VoidValue.java | 36 ++---- .../ydb/table/values/proto/ProtoValue.java | 8 +- 14 files changed, 145 insertions(+), 495 deletions(-) diff --git a/table/src/main/java/tech/ydb/table/values/DecimalValue.java b/table/src/main/java/tech/ydb/table/values/DecimalValue.java index 41464acf4..eb4ad5d9c 100644 --- a/table/src/main/java/tech/ydb/table/values/DecimalValue.java +++ b/table/src/main/java/tech/ydb/table/values/DecimalValue.java @@ -272,39 +272,27 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this decimal type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare DecimalValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } if (!(other instanceof DecimalValue)) { throw new IllegalArgumentException("Cannot compare DecimalValue with " + other.getClass().getSimpleName()); } - DecimalValue otherDecimal = (DecimalValue) other; + DecimalValue decimal = (DecimalValue) other; // Handle special values first - if (isNan() || otherDecimal.isNan()) { + if (isNan() || decimal.isNan()) { // NaN is not comparable, but we need to provide a consistent ordering - if (isNan() && otherDecimal.isNan()) { + if (isNan() && decimal.isNan()) { return 0; } if (isNan()) { @@ -313,38 +301,39 @@ public int compareTo(Value other) { return -1; } - if (isInf() && otherDecimal.isInf()) { + if (isInf() && decimal.isInf()) { return 0; } if (isInf()) { return 1; // Positive infinity is greater than any finite value } - if (otherDecimal.isInf()) { + if (decimal.isInf()) { return -1; } - if (isNegativeInf() && otherDecimal.isNegativeInf()) { + if (isNegativeInf() && decimal.isNegativeInf()) { return 0; } if (isNegativeInf()) { return -1; // Negative infinity is less than any finite value } - if (otherDecimal.isNegativeInf()) { + + if (decimal.isNegativeInf()) { return 1; } // Compare finite values - if (isNegative() != otherDecimal.isNegative()) { + if (isNegative() != decimal.isNegative()) { return isNegative() ? -1 : 1; } // Both have the same sign, compare magnitudes - int highComparison = Long.compareUnsigned(high, otherDecimal.high); + int highComparison = Long.compareUnsigned(high, decimal.high); if (highComparison != 0) { return isNegative() ? -highComparison : highComparison; } - int lowComparison = Long.compareUnsigned(low, otherDecimal.low); + int lowComparison = Long.compareUnsigned(low, decimal.low); return isNegative() ? -lowComparison : lowComparison; } diff --git a/table/src/main/java/tech/ydb/table/values/DictValue.java b/table/src/main/java/tech/ydb/table/values/DictValue.java index e2069d926..921b4a6e5 100644 --- a/table/src/main/java/tech/ydb/table/values/DictValue.java +++ b/table/src/main/java/tech/ydb/table/values/DictValue.java @@ -1,11 +1,10 @@ package tech.ydb.table.values; -import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeSet; import javax.annotation.Nullable; @@ -122,106 +121,42 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this dict type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare DictValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; - } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof DictValue)) { - throw new IllegalArgumentException("Cannot compare DictValue with " + other.getClass().getSimpleName()); + if (!type.equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + type + " with " + other.getType()); } DictValue otherDict = (DictValue) other; - // Convert to sorted lists for lexicographical comparison - List, Value>> thisEntries = new ArrayList<>(items.entrySet()); - List, Value>> otherEntries = new ArrayList<>(otherDict.items.entrySet()); + // Sort entries by keys + Set> keys = new TreeSet<>(); + keys.addAll(items.keySet()); + keys.addAll(otherDict.keySet()); - // Sort entries by key first, then by value - thisEntries.sort((e1, e2) -> { - int keyComparison = compareValues(e1.getKey(), e2.getKey()); - if (keyComparison != 0) { - return keyComparison; - } - return compareValues(e1.getValue(), e2.getValue()); - }); - - otherEntries.sort((e1, e2) -> { - int keyComparison = compareValues(e1.getKey(), e2.getKey()); - if (keyComparison != 0) { - return keyComparison; + for (Value key: keys) { + if (!otherDict.items.containsKey(key)) { + return 1; } - return compareValues(e1.getValue(), e2.getValue()); - }); - - // Compare sorted entries lexicographically - int minLength = Math.min(thisEntries.size(), otherEntries.size()); - for (int i = 0; i < minLength; i++) { - Map.Entry, Value> thisEntry = thisEntries.get(i); - Map.Entry, Value> otherEntry = otherEntries.get(i); - - int keyComparison = compareValues(thisEntry.getKey(), otherEntry.getKey()); - if (keyComparison != 0) { - return keyComparison; + if (!items.containsKey(key)) { + return -1; } - int valueComparison = compareValues(thisEntry.getValue(), otherEntry.getValue()); + int valueComparison = items.get(key).compareTo(otherDict.items.get(key)); if (valueComparison != 0) { return valueComparison; } } - // If we reach here, one dict is a prefix of the other - // The shorter dict comes first - return Integer.compare(thisEntries.size(), otherEntries.size()); - } - - private static int compareValues(Value a, Value b) { - // Handle null values - if (a == null && b == null) { - return 0; - } - if (a == null) { - return -1; - } - if (b == null) { - return 1; - } - - // Check that the types are the same - if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getType() + " vs " + b.getType()); - } - - // Use the actual compareTo method of the values - if (a instanceof Comparable && b instanceof Comparable) { - try { - return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) { - // Fall back to error - } - } - - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + return 0; } } diff --git a/table/src/main/java/tech/ydb/table/values/ListValue.java b/table/src/main/java/tech/ydb/table/values/ListValue.java index f34f9419e..4292a68a9 100644 --- a/table/src/main/java/tech/ydb/table/values/ListValue.java +++ b/table/src/main/java/tech/ydb/table/values/ListValue.java @@ -117,80 +117,27 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this list type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare ListValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof ListValue)) { - throw new IllegalArgumentException("Cannot compare ListValue with " + other.getClass().getSimpleName()); - } + ListValue list = (ListValue) other; - ListValue otherList = (ListValue) other; - - // Compare elements lexicographically - int minLength = Math.min(items.length, otherList.items.length); + int minLength = Math.min(items.length, list.items.length); for (int i = 0; i < minLength; i++) { - Value thisItem = items[i]; - Value otherItem = otherList.items[i]; - - int itemComparison = compareValues(thisItem, otherItem); + int itemComparison = items[i].compareTo(list.items[i]); if (itemComparison != 0) { return itemComparison; } } - // If we reach here, one list is a prefix of the other - // The shorter list comes first - return Integer.compare(items.length, otherList.items.length); - } - - private static int compareValues(Value a, Value b) { - // Handle null values - if (a == null && b == null) { - return 0; - } - if (a == null) { - return -1; - } - if (b == null) { - return 1; - } - - // Check that the types are the same - if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getType() + " vs " + b.getType()); - } - - // Use the actual compareTo method of the values - if (a instanceof Comparable && b instanceof Comparable) { - try { - return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) { - // Fall back to error - } - } - - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + return Integer.compare(items.length, list.items.length); } } diff --git a/table/src/main/java/tech/ydb/table/values/NullValue.java b/table/src/main/java/tech/ydb/table/values/NullValue.java index b106688b2..c288ee6e4 100644 --- a/table/src/main/java/tech/ydb/table/values/NullValue.java +++ b/table/src/main/java/tech/ydb/table/values/NullValue.java @@ -36,31 +36,19 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this null type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare NullValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + return 0; } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof NullValue)) { - throw new IllegalArgumentException("Cannot compare NullValue with " + other.getClass().getSimpleName()); + if (!getType().equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); } // All NullValue instances are equal diff --git a/table/src/main/java/tech/ydb/table/values/OptionalValue.java b/table/src/main/java/tech/ydb/table/values/OptionalValue.java index dde545c60..dfb573559 100644 --- a/table/src/main/java/tech/ydb/table/values/OptionalValue.java +++ b/table/src/main/java/tech/ydb/table/values/OptionalValue.java @@ -100,61 +100,24 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with another OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item types are the same - if (!type.getItemType().equals(otherOptional.type.getItemType())) { - throw new IllegalArgumentException("Cannot compare OptionalValue with different item types: " + - type.getItemType() + " vs " + otherOptional.type.getItemType()); - } - - // Handle empty values: empty values are considered less than non-empty values - if (value == null && otherOptional.value == null) { - return 0; - } - if (value == null) { - return -1; + OptionalValue optional = (OptionalValue) other; + if (optional.value == null) { + if (value == null) { + return 0; + } + throw new NullPointerException("Cannot compare value " + value + " with NULL"); } - if (otherOptional.value == null) { - return 1; - } - - // Both values are non-null and have the same type, compare them using their compareTo method - return compareValues(value, otherOptional.value); + return compareTo(optional.value); } - // Handle comparison with non-optional values of the same underlying type - if (type.getItemType().equals(other.getType())) { - // This OptionalValue is empty, so it's less than any non-optional value - if (value == null) { - return -1; - } - - // This OptionalValue has a value, compare it with the non-optional value - return compareValues(value, other); + if (value == null) { + throw new NullPointerException("Cannot compare NULL with value " + other); } - // Types are incompatible - throw new IllegalArgumentException("Cannot compare OptionalValue with incompatible type: " + - type.getItemType() + " vs " + other.getType()); - } - - private static int compareValues(Value a, Value b) { - // Since we've already verified the types are the same, we can safely cast - // and use the compareTo method of the actual value type - if (a instanceof Comparable && b instanceof Comparable) { - try { - return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) { - // Fall back to error - } - } - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + return value.compareTo(other); } } diff --git a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java index 5e9d84b70..f0a43130b 100644 --- a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java +++ b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java @@ -437,97 +437,80 @@ private static int compareByteArrays(byte[] a, byte[] b) { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this primitive type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare PrimitiveValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof PrimitiveValue)) { - throw new IllegalArgumentException( - "Cannot compare PrimitiveValue with " + other.getClass().getSimpleName()); + if (!getType().equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); } - PrimitiveValue otherPrimitive = (PrimitiveValue) other; - if (getType() != otherPrimitive.getType()) { - throw new IllegalArgumentException("Cannot compare values of different types: " + - getType() + " vs " + otherPrimitive.getType()); - } + PrimitiveValue otherValue = (PrimitiveValue) other; // Compare based on the actual primitive type switch (getType()) { case Bool: - return Boolean.compare(getBool(), otherPrimitive.getBool()); + return Boolean.compare(getBool(), otherValue.getBool()); case Int8: - return Byte.compare(getInt8(), otherPrimitive.getInt8()); + return Byte.compare(getInt8(), otherValue.getInt8()); case Uint8: - return Integer.compare(getUint8(), otherPrimitive.getUint8()); + return Integer.compare(getUint8(), otherValue.getUint8()); case Int16: - return Short.compare(getInt16(), otherPrimitive.getInt16()); + return Short.compare(getInt16(), otherValue.getInt16()); case Uint16: - return Integer.compare(getUint16(), otherPrimitive.getUint16()); + return Integer.compare(getUint16(), otherValue.getUint16()); case Int32: - return Integer.compare(getInt32(), otherPrimitive.getInt32()); + return Integer.compare(getInt32(), otherValue.getInt32()); case Uint32: - return Long.compare(getUint32(), otherPrimitive.getUint32()); + return Long.compare(getUint32(), otherValue.getUint32()); case Int64: - return Long.compare(getInt64(), otherPrimitive.getInt64()); + return Long.compare(getInt64(), otherValue.getInt64()); case Uint64: - return Long.compare(getUint64(), otherPrimitive.getUint64()); + return Long.compare(getUint64(), otherValue.getUint64()); case Float: - return Float.compare(getFloat(), otherPrimitive.getFloat()); + return Float.compare(getFloat(), otherValue.getFloat()); case Double: - return Double.compare(getDouble(), otherPrimitive.getDouble()); + return Double.compare(getDouble(), otherValue.getDouble()); case Bytes: case Yson: - return compareByteArrays(getBytesUnsafe(), otherPrimitive.getBytesUnsafe()); + return compareByteArrays(getBytesUnsafe(), otherValue.getBytesUnsafe()); case Text: - return getText().compareTo(otherPrimitive.getText()); + return getText().compareTo(otherValue.getText()); case Json: - return getJson().compareTo(otherPrimitive.getJson()); + return getJson().compareTo(otherValue.getJson()); case JsonDocument: - return getJsonDocument().compareTo(otherPrimitive.getJsonDocument()); + return getJsonDocument().compareTo(otherValue.getJsonDocument()); case Uuid: - return getUuidJdk().compareTo(otherPrimitive.getUuidJdk()); + return getUuidJdk().compareTo(otherValue.getUuidJdk()); case Date: - return getDate().compareTo(otherPrimitive.getDate()); + return getDate().compareTo(otherValue.getDate()); case Date32: - return getDate32().compareTo(otherPrimitive.getDate32()); + return getDate32().compareTo(otherValue.getDate32()); case Datetime: - return getDatetime().compareTo(otherPrimitive.getDatetime()); + return getDatetime().compareTo(otherValue.getDatetime()); case Datetime64: - return getDatetime64().compareTo(otherPrimitive.getDatetime64()); + return getDatetime64().compareTo(otherValue.getDatetime64()); case Timestamp: - return getTimestamp().compareTo(otherPrimitive.getTimestamp()); + return getTimestamp().compareTo(otherValue.getTimestamp()); case Timestamp64: - return getTimestamp64().compareTo(otherPrimitive.getTimestamp64()); + return getTimestamp64().compareTo(otherValue.getTimestamp64()); case Interval: - return getInterval().compareTo(otherPrimitive.getInterval()); + return getInterval().compareTo(otherValue.getInterval()); case Interval64: - return getInterval64().compareTo(otherPrimitive.getInterval64()); + return getInterval64().compareTo(otherValue.getInterval64()); case TzDate: - return getTzDate().compareTo(otherPrimitive.getTzDate()); + return getTzDate().compareTo(otherValue.getTzDate()); case TzDatetime: - return getTzDatetime().compareTo(otherPrimitive.getTzDatetime()); + return getTzDatetime().compareTo(otherValue.getTzDatetime()); case TzTimestamp: - return getTzTimestamp().compareTo(otherPrimitive.getTzTimestamp()); + return getTzTimestamp().compareTo(otherValue.getTzTimestamp()); default: throw new UnsupportedOperationException("Comparison not supported for type: " + getType()); } diff --git a/table/src/main/java/tech/ydb/table/values/StructValue.java b/table/src/main/java/tech/ydb/table/values/StructValue.java index 196771c2d..2e24fe2f7 100644 --- a/table/src/main/java/tech/ydb/table/values/StructValue.java +++ b/table/src/main/java/tech/ydb/table/values/StructValue.java @@ -137,81 +137,30 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this struct type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare StructValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof StructValue)) { - throw new IllegalArgumentException("Cannot compare StructValue with " + other.getClass().getSimpleName()); + if (!type.equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + type + " with " + other.getType()); } - StructValue otherStruct = (StructValue) other; - - // Compare members lexicographically - int minLength = Math.min(members.length, otherStruct.members.length); - for (int i = 0; i < minLength; i++) { - Value thisMember = members[i]; - Value otherMember = otherStruct.members[i]; - - int memberComparison = compareValues(thisMember, otherMember); + StructValue struct = (StructValue) other; + for (int i = 0; i < type.getMembersCount(); i++) { + int memberComparison = members[i].compareTo(struct.members[i]); if (memberComparison != 0) { return memberComparison; } } - // If we reach here, one struct is a prefix of the other - // The shorter struct comes first - return Integer.compare(members.length, otherStruct.members.length); - } - - private static int compareValues(Value a, Value b) { - // Handle null values - if (a == null && b == null) { - return 0; - } - if (a == null) { - return -1; - } - if (b == null) { - return 1; - } - - // Check that the types are the same - if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getType() + " vs " + b.getType()); - } - - // Use the actual compareTo method of the values - if (a instanceof Comparable && b instanceof Comparable) { - try { - return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) { - // Fall back to error - } - } - - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + return 0; } private static StructValue newStruct(String[] names, Value[] values) { diff --git a/table/src/main/java/tech/ydb/table/values/TupleValue.java b/table/src/main/java/tech/ydb/table/values/TupleValue.java index 00f16ed45..d5b08fb0a 100644 --- a/table/src/main/java/tech/ydb/table/values/TupleValue.java +++ b/table/src/main/java/tech/ydb/table/values/TupleValue.java @@ -149,80 +149,30 @@ private static TupleValue fromArray(Value... items) { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this tuple type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare TupleValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof TupleValue)) { - throw new IllegalArgumentException("Cannot compare TupleValue with " + other.getClass().getSimpleName()); + if (!type.equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + type + " with " + other.getType()); } TupleValue otherTuple = (TupleValue) other; - // Compare elements lexicographically - int minLength = Math.min(items.length, otherTuple.items.length); - for (int i = 0; i < minLength; i++) { - Value thisItem = items[i]; - Value otherItem = otherTuple.items[i]; - - int itemComparison = compareValues(thisItem, otherItem); + for (int i = 0; i < getType().getElementsCount(); i++) { + int itemComparison = items[i].compareTo(otherTuple.items[i]); if (itemComparison != 0) { return itemComparison; } } - // If we reach here, one tuple is a prefix of the other - // The shorter tuple comes first - return Integer.compare(items.length, otherTuple.items.length); - } - - private static int compareValues(Value a, Value b) { - // Handle null values - if (a == null && b == null) { - return 0; - } - if (a == null) { - return -1; - } - if (b == null) { - return 1; - } - - // Check that the types are the same - if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getType() + " vs " + b.getType()); - } - - // Use the actual compareTo method of the values - if (a instanceof Comparable && b instanceof Comparable) { - try { - return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) { - // Fall back to error - } - } - - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + return 0; } } diff --git a/table/src/main/java/tech/ydb/table/values/Value.java b/table/src/main/java/tech/ydb/table/values/Value.java index d21867089..1c6a37e01 100644 --- a/table/src/main/java/tech/ydb/table/values/Value.java +++ b/table/src/main/java/tech/ydb/table/values/Value.java @@ -16,18 +16,6 @@ public interface Value extends Serializable, Comparable ValueProtos.Value toPb(); - /** - * Compares this value with another value. - * The comparison is based on the actual data type of the value stored. - * For complex types like ListValue and StructValue, the comparison follows lexicographical rules. - * For OptionalValue, comparison with non-optional values of the same underlying type is supported. - * - * @param other the value to compare with - * @return a negative integer, zero, or a positive integer as this value is less than, equal to, or greater than the other value - * @throws IllegalArgumentException if the other value is null or has an incompatible type - */ - int compareTo(Value other); - default PrimitiveValue asData() { return (PrimitiveValue) this; } diff --git a/table/src/main/java/tech/ydb/table/values/VariantType.java b/table/src/main/java/tech/ydb/table/values/VariantType.java index f885c077a..4186266e9 100644 --- a/table/src/main/java/tech/ydb/table/values/VariantType.java +++ b/table/src/main/java/tech/ydb/table/values/VariantType.java @@ -82,12 +82,12 @@ public int hashCode() { public String toString() { StringBuilder sb = new StringBuilder(128); sb.append("Variant<"); - int count = getItemsCount(); + int count = itemTypes.length; for (int i = 0; i < count; i++) { - sb.append(getItemType(i)).append(", "); - } - if (count != 0) { - sb.setLength(sb.length() - 1); // cut last comma + sb.append(itemTypes[i]); + if (i < count - 1) { + sb.append(", "); + } } sb.append('>'); return sb.toString(); diff --git a/table/src/main/java/tech/ydb/table/values/VariantValue.java b/table/src/main/java/tech/ydb/table/values/VariantValue.java index dda7842e1..e9da4f1d2 100644 --- a/table/src/main/java/tech/ydb/table/values/VariantValue.java +++ b/table/src/main/java/tech/ydb/table/values/VariantValue.java @@ -74,43 +74,31 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this variant type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare VariantValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); - } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + throw new NullPointerException("Cannot compare value " + this + " with NULL"); } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof VariantValue)) { - throw new IllegalArgumentException("Cannot compare VariantValue with " + other.getClass().getSimpleName()); + if (!getType().equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); } - VariantValue otherVariant = (VariantValue) other; + VariantValue variant = (VariantValue) other; // Compare type indices first - int indexComparison = Integer.compare(typeIndex, otherVariant.typeIndex); + int indexComparison = Integer.compare(typeIndex, variant.typeIndex); if (indexComparison != 0) { return indexComparison; } // If type indices are the same, compare the items - return compareValues(item, otherVariant.item); + return compareValues(item, variant.item); } private static int compareValues(Value a, Value b) { diff --git a/table/src/main/java/tech/ydb/table/values/VoidType.java b/table/src/main/java/tech/ydb/table/values/VoidType.java index 6c64334e5..98c88c75a 100644 --- a/table/src/main/java/tech/ydb/table/values/VoidType.java +++ b/table/src/main/java/tech/ydb/table/values/VoidType.java @@ -24,16 +24,6 @@ public Kind getKind() { return Kind.VOID; } - @Override - public boolean equals(Object o) { - return this == o; - } - - @Override - public int hashCode() { - return 31 * Kind.VOID.hashCode(); - } - @Override public String toString() { return "Void"; diff --git a/table/src/main/java/tech/ydb/table/values/VoidValue.java b/table/src/main/java/tech/ydb/table/values/VoidValue.java index 7997604e2..fa6945d67 100644 --- a/table/src/main/java/tech/ydb/table/values/VoidValue.java +++ b/table/src/main/java/tech/ydb/table/values/VoidValue.java @@ -18,16 +18,6 @@ public static VoidValue of() { return INSTANCE; } - @Override - public boolean equals(Object o) { - return o == this; - } - - @Override - public int hashCode() { - return 1987; - } - @Override public String toString() { return "Void"; @@ -46,31 +36,19 @@ public ValueProtos.Value toPb() { @Override public int compareTo(Value other) { if (other == null) { - throw new IllegalArgumentException("Cannot compare with null value"); + throw new NullPointerException("Cannot compare with null value"); } - // Handle comparison with OptionalValue if (other instanceof OptionalValue) { - OptionalValue otherOptional = (OptionalValue) other; - - // Check that the item type matches this void type - if (!getType().equals(otherOptional.getType().getItemType())) { - throw new IllegalArgumentException( - "Cannot compare VoidValue with OptionalValue of different item type: " + - getType() + " vs " + otherOptional.getType().getItemType()); + OptionalValue optional = (OptionalValue) other; + if (!optional.isPresent()) { + return 0; } - - // Non-empty value is greater than empty optional - if (!otherOptional.isPresent()) { - return 1; - } - - // Compare with the wrapped value - return compareTo(otherOptional.get()); + return compareTo(optional.get()); } - if (!(other instanceof VoidValue)) { - throw new IllegalArgumentException("Cannot compare VoidValue with " + other.getClass().getSimpleName()); + if (!getType().equals(other.getType())) { + throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); } // All VoidValue instances are equal diff --git a/table/src/main/java/tech/ydb/table/values/proto/ProtoValue.java b/table/src/main/java/tech/ydb/table/values/proto/ProtoValue.java index 7eef37bb5..14f6bcfff 100644 --- a/table/src/main/java/tech/ydb/table/values/proto/ProtoValue.java +++ b/table/src/main/java/tech/ydb/table/values/proto/ProtoValue.java @@ -966,9 +966,11 @@ public String toString() { @Override public String getUuidString() { long hiBe = LittleEndian.bswap(high); - return - digits(low, 8) + "-" + digits(low >>> 32, 4) + "-" + digits(low >>> 48, 4) + "-" + - digits(hiBe >> 48, 4) + "-" + digits(hiBe, 12); + return digits(low, 8) + "-" + + digits(low >>> 32, 4) + "-" + + digits(low >>> 48, 4) + "-" + + digits(hiBe >> 48, 4) + "-" + + digits(hiBe, 12); } @Override From 28e302cd2b6d12220d72b4df45659855b1239b27 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Mon, 11 Aug 2025 10:51:11 +0100 Subject: [PATCH 13/17] Updated tests for value comparasion --- .../ydb/table/values/ValueComparableTest.java | 382 ++++++++---------- 1 file changed, 165 insertions(+), 217 deletions(-) diff --git a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java index 8fbc9e57f..c5b22bbf4 100644 --- a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java +++ b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java @@ -2,14 +2,27 @@ import java.util.HashMap; import java.util.Map; + +import org.junit.Assert; import org.junit.Test; -import static org.junit.Assert.*; +import org.junit.function.ThrowingRunnable; + /** * Test for Comparable implementation of Value classes */ public class ValueComparableTest { + private void assertNpe(String message, ThrowingRunnable runnable) { + NullPointerException npe = Assert.assertThrows(NullPointerException.class, runnable); + Assert.assertEquals(message, npe.getMessage()); + } + + private void assertIllegalArgument(String message, ThrowingRunnable runnable) { + IllegalArgumentException ex = Assert.assertThrows(IllegalArgumentException.class, runnable); + Assert.assertEquals(message, ex.getMessage()); + } + @Test public void testPrimitiveValueComparison() { // Test numeric comparisons @@ -17,161 +30,130 @@ public void testPrimitiveValueComparison() { PrimitiveValue int2 = PrimitiveValue.newInt32(2); PrimitiveValue int3 = PrimitiveValue.newInt32(1); - assertTrue(int1.compareTo(int2) < 0); - assertTrue(int2.compareTo(int1) > 0); - assertEquals(0, int1.compareTo(int3)); + Assert.assertTrue(int1.compareTo(int2) < 0); + Assert.assertTrue(int2.compareTo(int1) > 0); + Assert.assertEquals(0, int1.compareTo(int3)); + + // Optional comparing + Assert.assertTrue(int1.makeOptional().compareTo(int2) < 0); + Assert.assertTrue(int2.compareTo(int1.makeOptional()) > 0); + Assert.assertEquals(0, int1.makeOptional().compareTo(int3)); + Assert.assertEquals(0, int1.compareTo(int3.makeOptional())); + + // Invalid values + assertNpe("Cannot compare with null value", () -> int1.compareTo(null)); // Test string comparisons PrimitiveValue text1 = PrimitiveValue.newText("abc"); PrimitiveValue text2 = PrimitiveValue.newText("def"); PrimitiveValue text3 = PrimitiveValue.newText("abc"); - assertTrue(text1.compareTo(text2) < 0); - assertTrue(text2.compareTo(text1) > 0); - assertEquals(0, text1.compareTo(text3)); + Assert.assertTrue(text1.compareTo(text2) < 0); + Assert.assertTrue(text2.compareTo(text1) > 0); + Assert.assertEquals(0, text1.compareTo(text3)); // Test boolean comparisons PrimitiveValue bool1 = PrimitiveValue.newBool(false); PrimitiveValue bool2 = PrimitiveValue.newBool(true); - assertTrue(bool1.compareTo(bool2) < 0); - assertTrue(bool2.compareTo(bool1) > 0); + Assert.assertTrue(bool1.compareTo(bool2) < 0); + Assert.assertTrue(bool2.compareTo(bool1) > 0); + + assertIllegalArgument("Cannot compare value Int32 with Text", () -> int1.compareTo(text1)); } @Test public void testListValueComparison() { ListValue list1 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); - ListValue list2 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); - ListValue list3 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); - ListValue list4 = ListValue.of(PrimitiveValue.newInt32(1)); - - assertTrue(list1.compareTo(list2) < 0); - assertTrue(list2.compareTo(list1) > 0); - assertEquals(0, list1.compareTo(list3)); - assertTrue(list4.compareTo(list1) < 0); // shorter list comes first - } - - @Test - public void testListValueLexicographical() { - // Test proper lexicographical ordering - ListValue list1 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); - ListValue list2 = ListValue.of(PrimitiveValue.newText("Z")); - - // ('Z') should be "bigger" than ('A','Z') in lexicographical order - assertTrue(list1.compareTo(list2) < 0); // ('A','Z') < ('Z') - assertTrue(list2.compareTo(list1) > 0); // ('Z') > ('A','Z') + ListValue list2 = ListValue.of( + PrimitiveValue.newInt32(1).makeOptional(), + PrimitiveValue.newInt32(2).makeOptional() + ); - // Test prefix ordering - ListValue list3 = ListValue.of(PrimitiveValue.newText("A")); - ListValue list4 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("B")); + Assert.assertEquals(0, list1.compareTo(list2)); + Assert.assertEquals(0, list1.makeOptional().compareTo(list2)); + Assert.assertEquals(0, list1.compareTo(list2.makeOptional())); - assertTrue(list3.compareTo(list4) < 0); // ('A') < ('A','B') - assertTrue(list4.compareTo(list3) > 0); // ('A','B') > ('A') - } + ListValue list3 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); + ListValue list4 = ListValue.of(PrimitiveValue.newInt32(2), PrimitiveValue.newInt32(2)); - @Test(expected = IllegalArgumentException.class) - public void testListValueDifferentTypes() { - ListValue list1 = ListValue.of(PrimitiveValue.newInt32(1)); - ListValue list2 = ListValue.of(PrimitiveValue.newText("abc")); - list1.compareTo(list2); // Should throw exception for different element types - } + Assert.assertTrue(list1.compareTo(list3) < 0); + Assert.assertTrue(list2.compareTo(list3) < 0); + Assert.assertTrue(list4.compareTo(list3) > 0); + Assert.assertTrue(list3.compareTo(list4) < 0); - @Test - public void testStructValueComparison() { - StructValue struct1 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); - StructValue struct2 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(3)); - StructValue struct3 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); + ListValue list5 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); + ListValue list6 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); + ListValue list7 = ListValue.of(PrimitiveValue.newText("A")); + ListValue list8 = ListValue.of(PrimitiveValue.newText("Z")); - assertTrue(struct1.compareTo(struct2) < 0); - assertTrue(struct2.compareTo(struct1) > 0); - assertEquals(0, struct1.compareTo(struct3)); - } + Assert.assertEquals(0, list5.compareTo(list6)); + Assert.assertEquals(0, list6.compareTo(list5)); + Assert.assertTrue(list7.compareTo(list5) < 0); // shorter list comes first - @Test - public void testStructValueLexicographical() { // Test proper lexicographical ordering - StructValue struct1 = StructValue.of("a", PrimitiveValue.newText("A"), "b", PrimitiveValue.newText("Z")); - StructValue struct2 = StructValue.of("a", PrimitiveValue.newText("Z")); // ('Z') should be "bigger" than ('A','Z') in lexicographical order - assertTrue(struct1.compareTo(struct2) < 0); // ('A','Z') < ('Z') - assertTrue(struct2.compareTo(struct1) > 0); // ('Z') > ('A','Z') - } + Assert.assertTrue(list5.compareTo(list8) < 0); // ('A','Z') < ('Z') + Assert.assertTrue(list8.compareTo(list5) > 0); // ('Z') > ('A','Z') - @Test(expected = IllegalArgumentException.class) - public void testStructValueDifferentTypes() { - StructValue struct1 = StructValue.of("a", PrimitiveValue.newInt32(1)); - StructValue struct2 = StructValue.of("a", PrimitiveValue.newText("abc")); - struct1.compareTo(struct2); // Should throw exception for different member types + assertNpe("Cannot compare with null value", () -> list1.compareTo(null)); + assertIllegalArgument("Cannot compare value Int32 with Text", () -> list1.compareTo(list5)); + assertIllegalArgument("Cannot compare value Int32 with Text", () -> list2.compareTo(list5)); } @Test - public void testDictValueComparison() { - DictValue dict1 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - DictValue dict2 = DictValue.of(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); - DictValue dict3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - - assertTrue(dict1.compareTo(dict2) < 0); - assertTrue(dict2.compareTo(dict1) > 0); - assertEquals(0, dict1.compareTo(dict3)); - } - - @Test(expected = IllegalArgumentException.class) - public void testDictValueDifferentTypes() { - DictValue dict1 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - DictValue dict2 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newText("abc")); - dict1.compareTo(dict2); // Should throw exception for different value types + public void testStructValueComparison() { + StructValue s1 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); + StructValue s2 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newInt32(2)); + StructValue s3 = StructValue.of("a", PrimitiveValue.newInt32(2), "b", PrimitiveValue.newInt32(1)); + StructValue s4 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newText("a")); + StructValue s5 = StructValue.of("a", PrimitiveValue.newInt32(1)); + + Assert.assertEquals(0, s1.compareTo(s2)); + Assert.assertEquals(0, s1.compareTo(s2.makeOptional())); + Assert.assertEquals(0, s1.makeOptional().compareTo(s2)); + + Assert.assertTrue(s1.compareTo(s3) < 0); + Assert.assertTrue(s3.compareTo(s1) > 0); + + assertNpe("Cannot compare with null value", () -> s1.compareTo(null)); + assertIllegalArgument("Cannot compare value Struct<'a': Int32, 'b': Int32> with Struct<'a': Int32, 'b': Text>", + () -> s1.compareTo(s4)); + assertIllegalArgument("Cannot compare value Struct<'a': Int32, 'b': Int32> with Struct<'a': Int32>", + () -> s1.compareTo(s5)); } @Test - public void testDictValueLexicographical() { - // Test proper lexicographical ordering - content matters more than size - - // Case 1: Shorter dict with "larger" key should be greater than longer dict with "smaller" keys - Map, Value> map1 = new HashMap<>(); - map1.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - map1.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(2)); - DictValue dict1 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map1); + public void testDictValueComparison() { + DictValue d1 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + DictValue d2 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(2)); + DictValue d3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - DictValue dict2 = DictValue.of(PrimitiveValue.newText("z"), PrimitiveValue.newInt32(1)); + Assert.assertTrue(d1.compareTo(d2) < 0); + Assert.assertTrue(d2.compareTo(d1) > 0); - // {"z": 1} should be "bigger" than {"a": 1, "b": 2} in lexicographical order - assertTrue(dict1.compareTo(dict2) < 0); // {"a": 1, "b": 2} < {"z": 1} - assertTrue(dict2.compareTo(dict1) > 0); // {"z": 1} > {"a": 1, "b": 2} + Assert.assertEquals(0, d1.compareTo(d3)); + Assert.assertEquals(0, d1.compareTo(d3.makeOptional())); + Assert.assertEquals(0, d1.makeOptional().compareTo(d3)); - // Case 2: Same keys, different values - value comparison matters - DictValue dict3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - DictValue dict4 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(2)); + DictValue d4 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newText("abc")); + assertIllegalArgument("Cannot compare value Dict with Dict", () -> d1.compareTo(d4)); + assertIllegalArgument("Cannot compare value Dict with Dict", () -> d4.compareTo(d1)); - assertTrue(dict3.compareTo(dict4) < 0); // {"a": 1} < {"a": 2} - assertTrue(dict4.compareTo(dict3) > 0); // {"a": 2} > {"a": 1} + DictValue d5 = DictValue.of(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); - // Case 3: One dict is a prefix of another - shorter comes first only if it's a prefix - DictValue dict5 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); + // {"a": 1} should be "bigger" than {"b": 1 } in lexicographical order + Assert.assertTrue(d1.compareTo(d5) > 0); + Assert.assertTrue(d5.compareTo(d1) < 0); Map, Value> map6 = new HashMap<>(); map6.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); map6.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(2)); - DictValue dict6 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map6); - - assertTrue(dict5.compareTo(dict6) < 0); // {"a": 1} < {"a": 1, "b": 2} (prefix case) - assertTrue(dict6.compareTo(dict5) > 0); // {"a": 1, "b": 2} > {"a": 1} - - // Case 4: Multiple entries with different ordering - Map, Value> map7 = new HashMap<>(); - map7.put(PrimitiveValue.newText("x"), PrimitiveValue.newInt32(1)); - map7.put(PrimitiveValue.newText("y"), PrimitiveValue.newInt32(1)); - DictValue dict7 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map7); - - Map, Value> map8 = new HashMap<>(); - map8.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - map8.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); - map8.put(PrimitiveValue.newText("c"), PrimitiveValue.newInt32(1)); - DictValue dict8 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map8); - - // {"x": 1, "y": 1} should be greater than {"a": 1, "b": 1, "c": 1} despite being shorter - assertTrue(dict8.compareTo(dict7) < 0); // {"a": 1, "b": 1, "c": 1} < {"x": 1, "y": 1} - assertTrue(dict7.compareTo(dict8) > 0); // {"x": 1, "y": 1} > {"a": 1, "b": 1, "c": 1} + DictValue d6 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map6); + + Assert.assertTrue(d1.compareTo(d6) < 0); // {"a": 1} < {"a": 1, "b": 2} (prefix case) + Assert.assertTrue(d6.compareTo(d1) > 0); // {"a": 1, "b": 2} > {"a": 1} } @Test @@ -180,102 +162,71 @@ public void testOptionalValueComparison() { OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newInt32(2)); OptionalValue opt3 = OptionalValue.of(PrimitiveValue.newInt32(1)); OptionalValue opt4 = PrimitiveType.Int32.makeOptional().emptyValue(); + OptionalValue opt5 = OptionalValue.of(PrimitiveValue.newText("abc")); + OptionalValue opt6 = PrimitiveType.Text.makeOptional().emptyValue(); - assertTrue(opt1.compareTo(opt2) < 0); - assertTrue(opt2.compareTo(opt1) > 0); - assertEquals(0, opt1.compareTo(opt3)); - assertTrue(opt4.compareTo(opt1) < 0); // empty values come first - } + Assert.assertTrue(opt1.compareTo(opt2) < 0); + Assert.assertTrue(opt2.compareTo(opt1) > 0); + Assert.assertEquals(0, opt1.compareTo(opt3)); - @Test(expected = IllegalArgumentException.class) - public void testOptionalValueDifferentTypes() { - OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); - OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newText("abc")); - opt1.compareTo(opt2); // Should throw exception for different item types - } + assertNpe("Cannot compare NULL with value 1", () -> opt4.compareTo(opt1)); + assertNpe("Cannot compare value 1 with NULL", () -> opt1.compareTo(opt4)); - @Test - public void testOptionalValueWithNonOptional() { - OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); - OptionalValue opt2 = OptionalValue.of(PrimitiveValue.newInt32(2)); - OptionalValue opt3 = PrimitiveType.Int32.makeOptional().emptyValue(); - PrimitiveValue prim1 = PrimitiveValue.newInt32(1); - PrimitiveValue prim2 = PrimitiveValue.newInt32(2); - - // Optional with non-optional of same type - assertEquals(0, opt1.compareTo(prim1)); // Same value - assertTrue(opt1.compareTo(prim2) < 0); // Optional value less than non-optional - assertTrue(opt2.compareTo(prim1) > 0); // Optional value greater than non-optional - - // Empty optional with non-optional - assertTrue(opt3.compareTo(prim1) < 0); // Empty < non-empty - - // Non-optional with optional - assertEquals(0, prim1.compareTo(opt1)); // Same value - assertTrue(prim1.compareTo(opt2) < 0); // Non-optional less than optional - assertTrue(prim2.compareTo(opt1) > 0); // Non-optional greater than optional - assertTrue(prim1.compareTo(opt3) > 0); // Non-empty > empty - } + assertIllegalArgument("Cannot compare value Int32 with Text", () -> opt1.compareTo(opt5)); + assertIllegalArgument("Cannot compare value Text with Int32", () -> opt5.compareTo(opt1)); - @Test(expected = IllegalArgumentException.class) - public void testOptionalValueWithIncompatibleType() { - OptionalValue opt1 = OptionalValue.of(PrimitiveValue.newInt32(1)); - PrimitiveValue prim1 = PrimitiveValue.newText("abc"); - opt1.compareTo(prim1); // Should throw exception for incompatible types - } + Assert.assertEquals(0, opt4.compareTo(opt6)); + Assert.assertEquals(0, opt6.compareTo(opt4)); - @Test - public void testTupleValueComparison() { - TupleValue tuple1 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); - TupleValue tuple2 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); - TupleValue tuple3 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); - TupleValue tuple4 = TupleValue.of(PrimitiveValue.newInt32(1)); - - assertTrue(tuple1.compareTo(tuple2) < 0); - assertTrue(tuple2.compareTo(tuple1) > 0); - assertEquals(0, tuple1.compareTo(tuple3)); - assertTrue(tuple4.compareTo(tuple1) < 0); // shorter tuple comes first + assertNpe("Cannot compare with null value", () -> opt1.compareTo(null)); } @Test - public void testTupleValueLexicographical() { - // Test proper lexicographical ordering - TupleValue tuple1 = TupleValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); - TupleValue tuple2 = TupleValue.of(PrimitiveValue.newText("Z")); - - // ('Z') should be "bigger" than ('A','Z') in lexicographical order - assertTrue(tuple1.compareTo(tuple2) < 0); // ('A','Z') < ('Z') - assertTrue(tuple2.compareTo(tuple1) > 0); // ('Z') > ('A','Z') - } - - @Test(expected = IllegalArgumentException.class) - public void testTupleValueDifferentTypes() { - TupleValue tuple1 = TupleValue.of(PrimitiveValue.newInt32(1)); - TupleValue tuple2 = TupleValue.of(PrimitiveValue.newText("abc")); - tuple1.compareTo(tuple2); // Should throw exception for different element types + public void testTupleValueComparison() { + TupleValue t1 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); + TupleValue t2 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); + TupleValue t3 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(2)); + TupleValue t4 = TupleValue.of(PrimitiveValue.newInt32(1)); + TupleValue t5 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newUint32(2)); + + Assert.assertTrue(t1.compareTo(t2) < 0); + Assert.assertTrue(t2.compareTo(t1) > 0); + Assert.assertEquals(0, t1.compareTo(t3)); + Assert.assertEquals(0, t1.compareTo(t3.makeOptional())); + Assert.assertEquals(0, t1.makeOptional().compareTo(t3)); + + assertNpe("Cannot compare with null value", () -> t1.compareTo(null)); + assertIllegalArgument("Cannot compare value Tuple with Tuple", () -> t4.compareTo(t1)); + assertIllegalArgument("Cannot compare value Tuple with Tuple", + () -> t5.compareTo(t1)); } @Test public void testVariantValueComparison() { - VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), - PrimitiveValue.newInt32(1), 0); - VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), - PrimitiveValue.newText("abc"), 1); - VariantValue variant3 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), - PrimitiveValue.newInt32(2), 0); - - assertTrue(variant1.compareTo(variant2) < 0); // type index 0 < 1 - assertTrue(variant2.compareTo(variant1) > 0); - assertTrue(variant1.compareTo(variant3) < 0); // same type index, compare values - } - - @Test(expected = IllegalArgumentException.class) - public void testVariantValueDifferentTypes() { - VariantValue variant1 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), - PrimitiveValue.newInt32(1), 0); - VariantValue variant2 = new VariantValue(VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text), - PrimitiveValue.newText("abc"), 0); - variant1.compareTo(variant2); // Should throw exception for different item types + VariantType t1 = VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text); + VariantType t2 = VariantType.ofOwn(PrimitiveType.Int32, PrimitiveType.Text, PrimitiveType.Int32); + + VariantValue v1 = new VariantValue(t1, PrimitiveValue.newInt32(1), 0); + VariantValue v2 = new VariantValue(t1, PrimitiveValue.newText("abc"), 1); + VariantValue v3 = new VariantValue(t1, PrimitiveValue.newInt32(1), 0); + VariantValue v4 = new VariantValue(t1, PrimitiveValue.newText("aBc"), 1); + VariantValue v5 = new VariantValue(t2, PrimitiveValue.newInt32(1), 0); + + Assert.assertTrue(v1.compareTo(v2) < 0); // type index 0 < 1 + Assert.assertTrue(v2.compareTo(v1) > 0); + Assert.assertEquals(0, v1.compareTo(v3)); + Assert.assertEquals(0, v3.compareTo(v1)); + Assert.assertTrue(v2.compareTo(v4.makeOptional()) > 0); + Assert.assertTrue(v4.makeOptional().compareTo(v2) < 0); + + assertNpe("Cannot compare with null value", () -> v1.compareTo(null)); + assertNpe("Cannot compare value Variant[0; 1] with NULL", () -> v1.compareTo(t1.makeOptional().emptyValue())); + assertNpe("Cannot compare NULL with value Variant[0; 1]", () -> t1.makeOptional().emptyValue().compareTo(v1)); + + assertIllegalArgument("Cannot compare value Variant with Variant", + () -> v1.compareTo(v5)); + assertIllegalArgument("Cannot compare value Variant with Variant", + () -> v5.compareTo(v1)); } @Test @@ -283,7 +234,12 @@ public void testVoidValueComparison() { VoidValue void1 = VoidValue.of(); VoidValue void2 = VoidValue.of(); - assertEquals(0, void1.compareTo(void2)); + Assert.assertEquals(0, void1.compareTo(void2)); + Assert.assertEquals(0, void1.compareTo(void2.makeOptional())); + Assert.assertEquals(0, void1.makeOptional().compareTo(void2)); + + assertNpe("Cannot compare with null value", () -> void1.compareTo(null)); + assertIllegalArgument("Cannot compare value Void with Null", () -> void1.compareTo(NullValue.of())); } @Test @@ -291,7 +247,12 @@ public void testNullValueComparison() { NullValue null1 = NullValue.of(); NullValue null2 = NullValue.of(); - assertEquals(0, null1.compareTo(null2)); + Assert.assertEquals(0, null1.compareTo(null2)); + Assert.assertEquals(0, null1.compareTo(null2.makeOptional())); + Assert.assertEquals(0, null1.makeOptional().compareTo(null2)); + + assertNpe("Cannot compare with null value", () -> null1.compareTo(null)); + assertIllegalArgument("Cannot compare value Null with Void", () -> null1.compareTo(VoidValue.of())); } @Test @@ -300,21 +261,8 @@ public void testDecimalValueComparison() { DecimalValue decimal2 = DecimalValue.fromLong(DecimalType.of(10, 2), 200); DecimalValue decimal3 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); - assertTrue(decimal1.compareTo(decimal2) < 0); - assertTrue(decimal2.compareTo(decimal1) > 0); - assertEquals(0, decimal1.compareTo(decimal3)); - } - - @Test(expected = IllegalArgumentException.class) - public void testCompareWithNull() { - PrimitiveValue value = PrimitiveValue.newInt32(1); - value.compareTo(null); - } - - @Test(expected = IllegalArgumentException.class) - public void testCompareDifferentTypes() { - PrimitiveValue intValue = PrimitiveValue.newInt32(1); - PrimitiveValue textValue = PrimitiveValue.newText("abc"); - intValue.compareTo(textValue); + Assert.assertTrue(decimal1.compareTo(decimal2) < 0); + Assert.assertTrue(decimal2.compareTo(decimal1) > 0); + Assert.assertEquals(0, decimal1.compareTo(decimal3)); } } \ No newline at end of file From d91d52ca0e7ed4da89ff83101c2a19170c8122b1 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 12 Aug 2025 10:44:34 +0100 Subject: [PATCH 14/17] Added UUID sorting intergration test --- .../ydb/table/integration/ValuesReadTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/table/src/test/java/tech/ydb/table/integration/ValuesReadTest.java b/table/src/test/java/tech/ydb/table/integration/ValuesReadTest.java index 00dba0698..b8e31f2a2 100644 --- a/table/src/test/java/tech/ydb/table/integration/ValuesReadTest.java +++ b/table/src/test/java/tech/ydb/table/integration/ValuesReadTest.java @@ -24,11 +24,14 @@ import tech.ydb.table.transaction.TxControl; import tech.ydb.table.values.DecimalType; import tech.ydb.table.values.DecimalValue; +import tech.ydb.table.values.ListValue; import tech.ydb.table.values.NullType; import tech.ydb.table.values.NullValue; import tech.ydb.table.values.PrimitiveType; import tech.ydb.table.values.PrimitiveValue; +import tech.ydb.table.values.StructValue; import tech.ydb.table.values.Type; +import tech.ydb.table.values.Value; import tech.ydb.test.junit4.GrpcTransportRule; /** @@ -113,6 +116,89 @@ public void uuidReadTest() { Assert.assertEquals(0x6e73b41c4ede4d08L, v2.getUuidHigh()); } + @Test + public void uuidSortTest() { + String[] sorted = new String[] { + "00000000-0000-0000-0000-000000000001", + "00000000-0000-0000-0000-000000000010", + "00000000-0000-0000-0000-000000000100", + "00000000-0000-0000-0000-000000001000", + "00000000-0000-0000-0000-000000010000", + "00000000-0000-0000-0000-000000100000", + "00000000-0000-0000-0000-000001000000", + "00000000-0000-0000-0000-000010000000", + "00000000-0000-0000-0000-000100000000", + "00000000-0000-0000-0000-001000000000", + "00000000-0000-0000-0000-010000000000", + "00000000-0000-0000-0000-100000000000", + + "00000000-0000-0000-0001-000000000000", + "00000000-0000-0000-0010-000000000000", + "00000000-0000-0000-0100-000000000000", + "00000000-0000-0000-1000-000000000000", + + "00000000-0000-0100-0000-000000000000", + "00000000-0000-1000-0000-000000000000", + "00000000-0000-0001-0000-000000000000", + "00000000-0000-0010-0000-000000000000", + + "00000000-0100-0000-0000-000000000000", + "00000000-1000-0000-0000-000000000000", + "00000000-0001-0000-0000-000000000000", + "00000000-0010-0000-0000-000000000000", + + "01000000-0000-0000-0000-000000000000", + "10000000-0000-0000-0000-000000000000", + "00010000-0000-0000-0000-000000000000", + "00100000-0000-0000-0000-000000000000", + "00000100-0000-0000-0000-000000000000", + "00001000-0000-0000-0000-000000000000", + "00000001-0000-0000-0000-000000000000", + "00000010-0000-0000-0000-000000000000", + }; + + StructValue[] sv = new StructValue[sorted.length]; + for (int idx = 0; idx < sorted.length; idx++) { + sv[idx] = StructValue.of("uuid", PrimitiveValue.newUuid(sorted[idx])); + } + ListValue list = ListValue.of(sv); + + DataQueryResult result = CTX.supplyResult(s -> s.executeDataQuery("" + + "DECLARE $input AS List>;" + + "SELECT uuid FROM AS_TABLE($input) ORDER BY uuid ASC;" + + "SELECT uuid FROM AS_TABLE($input) ORDER BY uuid DESC;", + TxControl.snapshotRo(), Params.of("$input", list) + )).join().getValue(); + + Assert.assertEquals(2, result.getResultSetCount()); + ResultSetReader rs1 = result.getResultSet(0); + ResultSetReader rs2 = result.getResultSet(1); + + Value p1 = null; + Value p2 = null; + for (int idx = 0; idx < sorted.length; idx++) { + Assert.assertTrue(rs1.next()); + Assert.assertTrue(rs2.next()); + + Assert.assertEquals(UUID.fromString(sorted[idx]), rs1.getColumn(0).getUuid()); + Assert.assertEquals(UUID.fromString(sorted[sorted.length - 1 - idx]), rs2.getColumn(0).getUuid()); + + Value v1 = rs1.getColumn(0).getValue(); + Value v2 = rs2.getColumn(0).getValue(); + + if (idx != 0) { + Assert.assertTrue("" + v1 + " > " + p1, v1.compareTo(p1) > 0); + Assert.assertTrue("" + v2 + " < " + p2, v2.compareTo(p2) < 0); + } + + p1 = v1; + p2 = v2; + } + + Assert.assertFalse(rs1.next()); + Assert.assertFalse(rs2.next()); + } + private void assertTimestamp(ValueReader vr, boolean optional, Instant expected) { Assert.assertNotNull(vr); if (optional) { From 6cda2f707e25d5da22bfbfaec6f445e2adb90397 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 12 Aug 2025 10:45:28 +0100 Subject: [PATCH 15/17] Updated compareTo implementation for UUID, Decimal, Void and Null --- .../tech/ydb/table/values/DecimalValue.java | 50 +++---------------- .../java/tech/ydb/table/values/NullValue.java | 8 +-- .../tech/ydb/table/values/PrimitiveValue.java | 29 +++++------ .../tech/ydb/table/values/VariantValue.java | 34 +------------ .../java/tech/ydb/table/values/VoidValue.java | 8 +-- 5 files changed, 30 insertions(+), 99 deletions(-) diff --git a/table/src/main/java/tech/ydb/table/values/DecimalValue.java b/table/src/main/java/tech/ydb/table/values/DecimalValue.java index eb4ad5d9c..72ef4d097 100644 --- a/table/src/main/java/tech/ydb/table/values/DecimalValue.java +++ b/table/src/main/java/tech/ydb/table/values/DecimalValue.java @@ -289,52 +289,14 @@ public int compareTo(Value other) { DecimalValue decimal = (DecimalValue) other; - // Handle special values first - if (isNan() || decimal.isNan()) { - // NaN is not comparable, but we need to provide a consistent ordering - if (isNan() && decimal.isNan()) { - return 0; - } - if (isNan()) { - return 1; // NaN is considered greater than any other value - } - return -1; - } - - if (isInf() && decimal.isInf()) { - return 0; - } - if (isInf()) { - return 1; // Positive infinity is greater than any finite value - } - if (decimal.isInf()) { - return -1; - } - - if (isNegativeInf() && decimal.isNegativeInf()) { - return 0; - } - if (isNegativeInf()) { - return -1; // Negative infinity is less than any finite value - } - - if (decimal.isNegativeInf()) { - return 1; - } - - // Compare finite values - if (isNegative() != decimal.isNegative()) { - return isNegative() ? -1 : 1; - } - - // Both have the same sign, compare magnitudes - int highComparison = Long.compareUnsigned(high, decimal.high); - if (highComparison != 0) { - return isNegative() ? -highComparison : highComparison; + // Fast way to compare decimals with the same scale or with special values + boolean isSpecial = isNan() || isInf() || isNegativeInf(); + boolean otherIsSpecial = decimal.isNan() || decimal.isInf() || decimal.isNegativeInf(); + if (isSpecial || otherIsSpecial || (getType().getScale() == decimal.getType().getScale())) { + return high != decimal.high ? Long.compare(high, decimal.high) : Long.compare(low, decimal.low); } - int lowComparison = Long.compareUnsigned(low, decimal.low); - return isNegative() ? -lowComparison : lowComparison; + return toBigDecimal().compareTo(decimal.toBigDecimal()); } /** diff --git a/table/src/main/java/tech/ydb/table/values/NullValue.java b/table/src/main/java/tech/ydb/table/values/NullValue.java index c288ee6e4..b4cf6ca63 100644 --- a/table/src/main/java/tech/ydb/table/values/NullValue.java +++ b/table/src/main/java/tech/ydb/table/values/NullValue.java @@ -47,11 +47,11 @@ public int compareTo(Value other) { return compareTo(optional.get()); } - if (!getType().equals(other.getType())) { - throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); + if (other instanceof VoidValue || other instanceof NullValue) { + // All VoidValue and NullValue are equal + return 0; } - // All NullValue instances are equal - return 0; + throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); } } diff --git a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java index f0a43130b..92c305d4d 100644 --- a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java +++ b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java @@ -20,6 +20,7 @@ import com.google.protobuf.UnsafeByteOperations; import tech.ydb.proto.ValueProtos; +import tech.ydb.table.utils.LittleEndian; import tech.ydb.table.values.proto.ProtoValue; @@ -423,17 +424,6 @@ private static void checkType(PrimitiveType expected, PrimitiveType actual) { } } - private static int compareByteArrays(byte[] a, byte[] b) { - int minLength = Math.min(a.length, b.length); - for (int i = 0; i < minLength; i++) { - int comparison = Byte.compare(a[i], b[i]); - if (comparison != 0) { - return comparison; - } - } - return Integer.compare(a.length, b.length); - } - @Override public int compareTo(Value other) { if (other == null) { @@ -473,14 +463,15 @@ public int compareTo(Value other) { case Int64: return Long.compare(getInt64(), otherValue.getInt64()); case Uint64: - return Long.compare(getUint64(), otherValue.getUint64()); + return Long.compareUnsigned(getUint64(), otherValue.getUint64()); case Float: return Float.compare(getFloat(), otherValue.getFloat()); case Double: return Double.compare(getDouble(), otherValue.getDouble()); case Bytes: + return Arrays.compare(getBytesUnsafe(), otherValue.getBytesUnsafe()); case Yson: - return compareByteArrays(getBytesUnsafe(), otherValue.getBytesUnsafe()); + return Arrays.compare(getYsonUnsafe(), otherValue.getYsonUnsafe()); case Text: return getText().compareTo(otherValue.getText()); case Json: @@ -488,7 +479,7 @@ public int compareTo(Value other) { case JsonDocument: return getJsonDocument().compareTo(otherValue.getJsonDocument()); case Uuid: - return getUuidJdk().compareTo(otherValue.getUuidJdk()); + return compareUUID(this, otherValue); case Date: return getDate().compareTo(otherValue.getDate()); case Date32: @@ -516,6 +507,16 @@ public int compareTo(Value other) { } } + @SuppressWarnings("deprecation") + private static int compareUUID(PrimitiveValue a, PrimitiveValue b) { + long ah = LittleEndian.bswap(a.getUuidHigh()); + long bh = LittleEndian.bswap(b.getUuidHigh()); + long al = LittleEndian.bswap(a.getUuidLow()); + long bl = LittleEndian.bswap(b.getUuidLow()); + + return (al != bl) ? Long.compareUnsigned(al, bl) : Long.compareUnsigned(ah, bh); + } + // -- implementations -- private static final class Bool extends PrimitiveValue { diff --git a/table/src/main/java/tech/ydb/table/values/VariantValue.java b/table/src/main/java/tech/ydb/table/values/VariantValue.java index e9da4f1d2..7112aed49 100644 --- a/table/src/main/java/tech/ydb/table/values/VariantValue.java +++ b/table/src/main/java/tech/ydb/table/values/VariantValue.java @@ -97,38 +97,6 @@ public int compareTo(Value other) { return indexComparison; } - // If type indices are the same, compare the items - return compareValues(item, variant.item); - } - - private static int compareValues(Value a, Value b) { - // Handle null values - if (a == null && b == null) { - return 0; - } - if (a == null) { - return -1; - } - if (b == null) { - return 1; - } - - // Check that the types are the same - if (!a.getType().equals(b.getType())) { - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getType() + " vs " + b.getType()); - } - - // Use the actual compareTo method of the values - if (a instanceof Comparable && b instanceof Comparable) { - try { - return ((Comparable>) a).compareTo((Value) b); - } catch (ClassCastException e) { - // Fall back to error - } - } - - throw new IllegalArgumentException("Cannot compare values of different types: " + - a.getClass().getSimpleName() + " vs " + b.getClass().getSimpleName()); + return item.compareTo(variant.item); } } diff --git a/table/src/main/java/tech/ydb/table/values/VoidValue.java b/table/src/main/java/tech/ydb/table/values/VoidValue.java index fa6945d67..496ab8171 100644 --- a/table/src/main/java/tech/ydb/table/values/VoidValue.java +++ b/table/src/main/java/tech/ydb/table/values/VoidValue.java @@ -47,11 +47,11 @@ public int compareTo(Value other) { return compareTo(optional.get()); } - if (!getType().equals(other.getType())) { - throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); + if (other instanceof VoidValue || other instanceof NullValue) { + // All VoidValue and NullValue are equal + return 0; } - // All VoidValue instances are equal - return 0; + throw new IllegalArgumentException("Cannot compare value " + getType() + " with " + other.getType()); } } From 39206af58319a3103fb34dcb6a4e1f8a028e0526 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 12 Aug 2025 10:46:17 +0100 Subject: [PATCH 16/17] Updated tests for value comparing --- .../ydb/table/values/ValueComparableTest.java | 325 ++++++++++++------ 1 file changed, 220 insertions(+), 105 deletions(-) diff --git a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java index c5b22bbf4..cebee0299 100644 --- a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java +++ b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java @@ -1,11 +1,11 @@ package tech.ydb.table.values; +import java.time.Instant; import java.util.HashMap; import java.util.Map; import org.junit.Assert; import org.junit.Test; -import org.junit.function.ThrowingRunnable; /** @@ -13,16 +13,28 @@ */ public class ValueComparableTest { - private void assertNpe(String message, ThrowingRunnable runnable) { - NullPointerException npe = Assert.assertThrows(NullPointerException.class, runnable); + private void assertNpe(String message, Comparable one, T other) { + NullPointerException npe = Assert.assertThrows(NullPointerException.class, () -> one.compareTo(other)); Assert.assertEquals(message, npe.getMessage()); } - private void assertIllegalArgument(String message, ThrowingRunnable runnable) { - IllegalArgumentException ex = Assert.assertThrows(IllegalArgumentException.class, runnable); + private void assertIllegalArgument(String message, Comparable one, T other) { + IllegalArgumentException ex = Assert.assertThrows(IllegalArgumentException.class, () -> one.compareTo(other)); Assert.assertEquals(message, ex.getMessage()); } + private void assertLess(Comparable one, T other) { + Assert.assertTrue("" + one + " < " + other + " FAILED", one.compareTo(other) < 0); + } + + private void assertGreater(Comparable one, T other) { + Assert.assertTrue("" + one + " > " + other + " FAILED", one.compareTo(other) > 0); + } + + private void assertEquals(Comparable one, T other) { + Assert.assertEquals("" + one + " = " + other + " FAILED", 0, one.compareTo(other)); + } + @Test public void testPrimitiveValueComparison() { // Test numeric comparisons @@ -30,36 +42,99 @@ public void testPrimitiveValueComparison() { PrimitiveValue int2 = PrimitiveValue.newInt32(2); PrimitiveValue int3 = PrimitiveValue.newInt32(1); - Assert.assertTrue(int1.compareTo(int2) < 0); - Assert.assertTrue(int2.compareTo(int1) > 0); - Assert.assertEquals(0, int1.compareTo(int3)); + assertLess(int1, int2); + assertGreater(int2, int1); + assertEquals(int1, int3); // Optional comparing - Assert.assertTrue(int1.makeOptional().compareTo(int2) < 0); - Assert.assertTrue(int2.compareTo(int1.makeOptional()) > 0); - Assert.assertEquals(0, int1.makeOptional().compareTo(int3)); - Assert.assertEquals(0, int1.compareTo(int3.makeOptional())); + assertLess(int1.makeOptional(), int2); + assertGreater(int2, int1.makeOptional()); + assertEquals(int1.makeOptional(), int3); + assertEquals(int1, int3.makeOptional()); // Invalid values - assertNpe("Cannot compare with null value", () -> int1.compareTo(null)); + assertNpe("Cannot compare with null value", int1, null); // Test string comparisons PrimitiveValue text1 = PrimitiveValue.newText("abc"); PrimitiveValue text2 = PrimitiveValue.newText("def"); PrimitiveValue text3 = PrimitiveValue.newText("abc"); - Assert.assertTrue(text1.compareTo(text2) < 0); - Assert.assertTrue(text2.compareTo(text1) > 0); - Assert.assertEquals(0, text1.compareTo(text3)); + assertLess(text1, text2); + assertGreater(text2, text1); + assertEquals(text1, text3); // Test boolean comparisons PrimitiveValue bool1 = PrimitiveValue.newBool(false); PrimitiveValue bool2 = PrimitiveValue.newBool(true); - Assert.assertTrue(bool1.compareTo(bool2) < 0); - Assert.assertTrue(bool2.compareTo(bool1) > 0); + assertLess(bool1, bool2); + assertGreater(bool2, bool1); + + assertIllegalArgument("Cannot compare value Int32 with Text", int1, text1); + assertNpe("Cannot compare value 1 with NULL", int1, PrimitiveType.Int32.makeOptional().emptyValue()); + + // All types check + assertLess(PrimitiveValue.newInt8(Byte.MIN_VALUE), PrimitiveValue.newInt8(Byte.MAX_VALUE)); + assertLess(PrimitiveValue.newInt16(Short.MIN_VALUE), PrimitiveValue.newInt16(Short.MAX_VALUE)); + assertLess(PrimitiveValue.newInt32(Integer.MIN_VALUE), PrimitiveValue.newInt32(Integer.MAX_VALUE)); + assertLess(PrimitiveValue.newInt64(Long.MIN_VALUE), PrimitiveValue.newInt64(Long.MAX_VALUE)); + + assertGreater(PrimitiveValue.newUint8(Byte.MIN_VALUE), PrimitiveValue.newUint8(Byte.MAX_VALUE)); + assertGreater(PrimitiveValue.newUint16(Short.MIN_VALUE), PrimitiveValue.newUint16(Short.MAX_VALUE)); + assertGreater(PrimitiveValue.newUint32(Integer.MIN_VALUE), PrimitiveValue.newUint32(Integer.MAX_VALUE)); + assertGreater(PrimitiveValue.newUint64(Long.MIN_VALUE), PrimitiveValue.newUint64(Long.MAX_VALUE)); + + assertLess(PrimitiveValue.newFloat(1e-3f), PrimitiveValue.newFloat(1e-2f)); + assertLess(PrimitiveValue.newDouble(1e-3d), PrimitiveValue.newDouble(1e-2d)); + + assertLess(PrimitiveValue.newBytes(new byte[] { 0x01 }), PrimitiveValue.newBytes(new byte[] { 0x02 })); + assertLess(PrimitiveValue.newYson(new byte[] { 0x01 }), PrimitiveValue.newYson(new byte[] { 0x02 })); + + assertLess(PrimitiveValue.newText("abc"), PrimitiveValue.newText("abcd")); + assertLess(PrimitiveValue.newJson("['abc']"), PrimitiveValue.newJson("['abcd']")); + assertLess(PrimitiveValue.newJsonDocument("['abc']"), PrimitiveValue.newJsonDocument("['abcd']")); + + for (String[] uuid : new String[][] { + // Sort by 3 2 1 0 5 4 7 6 8 9 A B C D E F + new String[] { "FFFFFFFE-FFFF-FFFF-FFFF-FFFFFFFFFFFF", "000000FF-0000-0000-0000-000000000000" }, + new String[] { "FFFF3000-FFFF-FFFF-FFFF-FFFFFFFFFFFF", "00003100-0000-0000-0000-000000000000" }, + new String[] { "FF390000-FFFF-FFFF-FFFF-FFFFFFFFFFFF", "00400000-0000-0000-0000-000000000000" }, + new String[] { "00000000-FFFF-FFFF-FFFF-FFFFFFFFFFFF", "01000000-0000-0000-0000-000000000000" }, + + new String[] { "00000000-FFFE-FFFF-FFFF-FFFFFFFFFFFF", "00000000-00FF-0000-0000-000000000000" }, + new String[] { "00000000-A000-FFFF-FFFF-FFFFFFFFFFFF", "00000000-A100-0000-0000-000000000000" }, + new String[] { "00000000-0000-FF00-FFFF-FFFFFFFFFFFF", "00000000-0000-0001-0000-000000000000" }, + new String[] { "00000000-0000-0200-FFFF-FFFFFFFFFFFF", "00000000-0000-2000-0000-000000000000" }, + + new String[] { "00000000-0000-0000-F0FF-FFFFFFFFFFFF", "00000000-0000-0000-F100-000000000000" }, + new String[] { "00000000-0000-0000-00FE-FFFFFFFFFFFF", "00000000-0000-0000-00FF-000000000000" }, + + new String[] { "00000000-0000-0000-0000-FEFFFFFFFFFF", "00000000-0000-0000-0000-FF0000000000" }, + new String[] { "00000000-0000-0000-0000-00ABFFFFFFFF", "00000000-0000-0000-0000-00AC00000000" }, + new String[] { "00000000-0000-0000-0000-000050FFFFFF", "00000000-0000-0000-0000-000060000000" }, + new String[] { "00000000-0000-0000-0000-00000045FFFF", "00000000-0000-0000-0000-000004600000" }, + new String[] { "00000000-0000-0000-0000-0000000012FF", "00000000-0000-0000-0000-000000001300" }, + new String[] { "00000000-0000-0000-0000-000000000000", "00000000-0000-0000-0000-000000000001" }, + }) { + assertLess(PrimitiveValue.newUuid(uuid[0]), PrimitiveValue.newUuid(uuid[1])); + } + + assertLess(PrimitiveValue.newDate(20000), PrimitiveValue.newDate(20001)); + assertLess(PrimitiveValue.newDatetime(1728000000), PrimitiveValue.newDatetime(1728000001)); + assertLess( + PrimitiveValue.newTimestamp(Instant.ofEpochSecond(1728000000, 123456000)), + PrimitiveValue.newTimestamp(Instant.ofEpochSecond(1728000000, 123457000)) + ); + assertLess(PrimitiveValue.newInterval(20000), PrimitiveValue.newInterval(20001)); - assertIllegalArgument("Cannot compare value Int32 with Text", () -> int1.compareTo(text1)); + assertLess(PrimitiveValue.newDate32(20000), PrimitiveValue.newDate32(20001)); + assertLess(PrimitiveValue.newDatetime64(1728000000), PrimitiveValue.newDatetime64(1728000001)); + assertLess( + PrimitiveValue.newTimestamp64(Instant.ofEpochSecond(1728000000, 123456000)), + PrimitiveValue.newTimestamp64(Instant.ofEpochSecond(1728000000, 123457000)) + ); + assertLess(PrimitiveValue.newInterval64(20000), PrimitiveValue.newInterval64(20001)); } @Test @@ -70,36 +145,36 @@ public void testListValueComparison() { PrimitiveValue.newInt32(2).makeOptional() ); - Assert.assertEquals(0, list1.compareTo(list2)); - Assert.assertEquals(0, list1.makeOptional().compareTo(list2)); - Assert.assertEquals(0, list1.compareTo(list2.makeOptional())); + assertEquals(list1, list2); + assertEquals(list1.makeOptional(), list2); + assertEquals(list1, list2.makeOptional()); ListValue list3 = ListValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newInt32(3)); ListValue list4 = ListValue.of(PrimitiveValue.newInt32(2), PrimitiveValue.newInt32(2)); - Assert.assertTrue(list1.compareTo(list3) < 0); - Assert.assertTrue(list2.compareTo(list3) < 0); - Assert.assertTrue(list4.compareTo(list3) > 0); - Assert.assertTrue(list3.compareTo(list4) < 0); + assertLess(list1, list3); + assertLess(list2, list3); + assertGreater(list4, list3); + assertLess(list3, list4); ListValue list5 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); ListValue list6 = ListValue.of(PrimitiveValue.newText("A"), PrimitiveValue.newText("Z")); ListValue list7 = ListValue.of(PrimitiveValue.newText("A")); ListValue list8 = ListValue.of(PrimitiveValue.newText("Z")); - Assert.assertEquals(0, list5.compareTo(list6)); - Assert.assertEquals(0, list6.compareTo(list5)); - Assert.assertTrue(list7.compareTo(list5) < 0); // shorter list comes first + assertEquals(list5, list6); + assertEquals(list6, list5); + assertLess(list7, list5); // shorter list comes first // Test proper lexicographical ordering // ('Z') should be "bigger" than ('A','Z') in lexicographical order - Assert.assertTrue(list5.compareTo(list8) < 0); // ('A','Z') < ('Z') - Assert.assertTrue(list8.compareTo(list5) > 0); // ('Z') > ('A','Z') + assertLess(list5, list8); // ('A','Z') < ('Z') + assertGreater(list8, list5); // ('Z') > ('A','Z') - assertNpe("Cannot compare with null value", () -> list1.compareTo(null)); - assertIllegalArgument("Cannot compare value Int32 with Text", () -> list1.compareTo(list5)); - assertIllegalArgument("Cannot compare value Int32 with Text", () -> list2.compareTo(list5)); + assertNpe("Cannot compare with null value", list1, null); + assertIllegalArgument("Cannot compare value Int32 with Text", list1, list5); + assertIllegalArgument("Cannot compare value Int32 with Text", list2, list5); } @Test @@ -110,18 +185,17 @@ public void testStructValueComparison() { StructValue s4 = StructValue.of("a", PrimitiveValue.newInt32(1), "b", PrimitiveValue.newText("a")); StructValue s5 = StructValue.of("a", PrimitiveValue.newInt32(1)); - Assert.assertEquals(0, s1.compareTo(s2)); - Assert.assertEquals(0, s1.compareTo(s2.makeOptional())); - Assert.assertEquals(0, s1.makeOptional().compareTo(s2)); + assertEquals(s1, s2); + assertEquals(s1, s2.makeOptional()); + assertEquals(s1.makeOptional(), s2); - Assert.assertTrue(s1.compareTo(s3) < 0); - Assert.assertTrue(s3.compareTo(s1) > 0); + assertLess(s1, s3); + assertGreater(s3, s1); - assertNpe("Cannot compare with null value", () -> s1.compareTo(null)); + assertNpe("Cannot compare with null value", s1, null); assertIllegalArgument("Cannot compare value Struct<'a': Int32, 'b': Int32> with Struct<'a': Int32, 'b': Text>", - () -> s1.compareTo(s4)); - assertIllegalArgument("Cannot compare value Struct<'a': Int32, 'b': Int32> with Struct<'a': Int32>", - () -> s1.compareTo(s5)); + s1, s4); + assertIllegalArgument("Cannot compare value Struct<'a': Int32, 'b': Int32> with Struct<'a': Int32>", s1, s5); } @Test @@ -130,30 +204,30 @@ public void testDictValueComparison() { DictValue d2 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(2)); DictValue d3 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); - Assert.assertTrue(d1.compareTo(d2) < 0); - Assert.assertTrue(d2.compareTo(d1) > 0); + assertLess(d1, d2); + assertGreater(d2, d1); - Assert.assertEquals(0, d1.compareTo(d3)); - Assert.assertEquals(0, d1.compareTo(d3.makeOptional())); - Assert.assertEquals(0, d1.makeOptional().compareTo(d3)); + assertEquals(d1, d3); + assertEquals(d1, d3.makeOptional()); + assertEquals(d1.makeOptional(), d3); DictValue d4 = DictValue.of(PrimitiveValue.newText("a"), PrimitiveValue.newText("abc")); - assertIllegalArgument("Cannot compare value Dict with Dict", () -> d1.compareTo(d4)); - assertIllegalArgument("Cannot compare value Dict with Dict", () -> d4.compareTo(d1)); + assertIllegalArgument("Cannot compare value Dict with Dict", d1, d4); + assertIllegalArgument("Cannot compare value Dict with Dict", d4, d1); DictValue d5 = DictValue.of(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(1)); // {"a": 1} should be "bigger" than {"b": 1 } in lexicographical order - Assert.assertTrue(d1.compareTo(d5) > 0); - Assert.assertTrue(d5.compareTo(d1) < 0); + assertGreater(d1, d5); + assertLess(d5, d1); Map, Value> map6 = new HashMap<>(); map6.put(PrimitiveValue.newText("a"), PrimitiveValue.newInt32(1)); map6.put(PrimitiveValue.newText("b"), PrimitiveValue.newInt32(2)); DictValue d6 = DictType.of(PrimitiveType.Text, PrimitiveType.Int32).newValueOwn(map6); - Assert.assertTrue(d1.compareTo(d6) < 0); // {"a": 1} < {"a": 1, "b": 2} (prefix case) - Assert.assertTrue(d6.compareTo(d1) > 0); // {"a": 1, "b": 2} > {"a": 1} + assertLess(d1, d6); // {"a": 1} < {"a": 1, "b": 2} (prefix case) + assertGreater(d6, d1); // {"a": 1, "b": 2} > {"a": 1} } @Test @@ -165,20 +239,20 @@ public void testOptionalValueComparison() { OptionalValue opt5 = OptionalValue.of(PrimitiveValue.newText("abc")); OptionalValue opt6 = PrimitiveType.Text.makeOptional().emptyValue(); - Assert.assertTrue(opt1.compareTo(opt2) < 0); - Assert.assertTrue(opt2.compareTo(opt1) > 0); - Assert.assertEquals(0, opt1.compareTo(opt3)); + assertLess(opt1, opt2); + assertGreater(opt2, opt1); + assertEquals(opt1, opt3); - assertNpe("Cannot compare NULL with value 1", () -> opt4.compareTo(opt1)); - assertNpe("Cannot compare value 1 with NULL", () -> opt1.compareTo(opt4)); + assertNpe("Cannot compare NULL with value 1", opt4, opt1); + assertNpe("Cannot compare value 1 with NULL", opt1, opt4); - assertIllegalArgument("Cannot compare value Int32 with Text", () -> opt1.compareTo(opt5)); - assertIllegalArgument("Cannot compare value Text with Int32", () -> opt5.compareTo(opt1)); + assertIllegalArgument("Cannot compare value Int32 with Text", opt1, opt5); + assertIllegalArgument("Cannot compare value Text with Int32", opt5, opt1); - Assert.assertEquals(0, opt4.compareTo(opt6)); - Assert.assertEquals(0, opt6.compareTo(opt4)); + assertEquals(opt4, opt6); + assertEquals(opt6, opt4); - assertNpe("Cannot compare with null value", () -> opt1.compareTo(null)); + assertNpe("Cannot compare with null value", opt1, null); } @Test @@ -189,16 +263,15 @@ public void testTupleValueComparison() { TupleValue t4 = TupleValue.of(PrimitiveValue.newInt32(1)); TupleValue t5 = TupleValue.of(PrimitiveValue.newInt32(1), PrimitiveValue.newUint32(2)); - Assert.assertTrue(t1.compareTo(t2) < 0); - Assert.assertTrue(t2.compareTo(t1) > 0); - Assert.assertEquals(0, t1.compareTo(t3)); - Assert.assertEquals(0, t1.compareTo(t3.makeOptional())); - Assert.assertEquals(0, t1.makeOptional().compareTo(t3)); + assertLess(t1, t2); + assertGreater(t2, t1); + assertEquals(t1, t3); + assertEquals(t1, t3.makeOptional()); + assertEquals(t1.makeOptional(), t3); - assertNpe("Cannot compare with null value", () -> t1.compareTo(null)); - assertIllegalArgument("Cannot compare value Tuple with Tuple", () -> t4.compareTo(t1)); - assertIllegalArgument("Cannot compare value Tuple with Tuple", - () -> t5.compareTo(t1)); + assertNpe("Cannot compare with null value", t1, null); + assertIllegalArgument("Cannot compare value Tuple with Tuple", t4, t1); + assertIllegalArgument("Cannot compare value Tuple with Tuple", t5, t1); } @Test @@ -212,21 +285,19 @@ public void testVariantValueComparison() { VariantValue v4 = new VariantValue(t1, PrimitiveValue.newText("aBc"), 1); VariantValue v5 = new VariantValue(t2, PrimitiveValue.newInt32(1), 0); - Assert.assertTrue(v1.compareTo(v2) < 0); // type index 0 < 1 - Assert.assertTrue(v2.compareTo(v1) > 0); - Assert.assertEquals(0, v1.compareTo(v3)); - Assert.assertEquals(0, v3.compareTo(v1)); - Assert.assertTrue(v2.compareTo(v4.makeOptional()) > 0); - Assert.assertTrue(v4.makeOptional().compareTo(v2) < 0); - - assertNpe("Cannot compare with null value", () -> v1.compareTo(null)); - assertNpe("Cannot compare value Variant[0; 1] with NULL", () -> v1.compareTo(t1.makeOptional().emptyValue())); - assertNpe("Cannot compare NULL with value Variant[0; 1]", () -> t1.makeOptional().emptyValue().compareTo(v1)); - - assertIllegalArgument("Cannot compare value Variant with Variant", - () -> v1.compareTo(v5)); - assertIllegalArgument("Cannot compare value Variant with Variant", - () -> v5.compareTo(v1)); + assertLess(v1, v2); // type index 0 < 1 + assertGreater(v2, v1); + assertEquals(v1, v3); + assertEquals(v3, v1); + assertGreater(v2, v4.makeOptional()); + assertLess(v4.makeOptional(), v2); + + assertNpe("Cannot compare with null value", v1, null); + assertNpe("Cannot compare value Variant[0; 1] with NULL", v1, t1.makeOptional().emptyValue()); + assertNpe("Cannot compare NULL with value Variant[0; 1]", t1.makeOptional().emptyValue(), v1); + + assertIllegalArgument("Cannot compare value Variant with Variant", v1, v5); + assertIllegalArgument("Cannot compare value Variant with Variant", v5, v1); } @Test @@ -234,12 +305,16 @@ public void testVoidValueComparison() { VoidValue void1 = VoidValue.of(); VoidValue void2 = VoidValue.of(); - Assert.assertEquals(0, void1.compareTo(void2)); - Assert.assertEquals(0, void1.compareTo(void2.makeOptional())); - Assert.assertEquals(0, void1.makeOptional().compareTo(void2)); + assertEquals(void1, void2); + assertEquals(void1, void2.makeOptional()); + assertEquals(void1.makeOptional(), void2); - assertNpe("Cannot compare with null value", () -> void1.compareTo(null)); - assertIllegalArgument("Cannot compare value Void with Null", () -> void1.compareTo(NullValue.of())); + assertEquals(void1, NullValue.of()); + assertEquals(void1, NullValue.of().makeOptional()); + assertEquals(void1, PrimitiveType.Int32.makeOptional().emptyValue()); + + assertNpe("Cannot compare with null value", void1, null); + assertIllegalArgument("Cannot compare value Void with Int32", void1, PrimitiveValue.newInt32(1)); } @Test @@ -247,22 +322,62 @@ public void testNullValueComparison() { NullValue null1 = NullValue.of(); NullValue null2 = NullValue.of(); - Assert.assertEquals(0, null1.compareTo(null2)); - Assert.assertEquals(0, null1.compareTo(null2.makeOptional())); - Assert.assertEquals(0, null1.makeOptional().compareTo(null2)); + assertEquals(null1, null2); + assertEquals(null1, null2.makeOptional()); + assertEquals(null1.makeOptional(), null2); + + assertEquals(null1, VoidValue.of()); + assertEquals(null1, VoidValue.of().makeOptional()); + assertEquals(null1, PrimitiveType.Int32.makeOptional().emptyValue()); - assertNpe("Cannot compare with null value", () -> null1.compareTo(null)); - assertIllegalArgument("Cannot compare value Null with Void", () -> null1.compareTo(VoidValue.of())); + assertNpe("Cannot compare with null value", null1, null); + assertIllegalArgument("Cannot compare value Null with Int32", null1, PrimitiveValue.newInt32(1)); } @Test public void testDecimalValueComparison() { - DecimalValue decimal1 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); - DecimalValue decimal2 = DecimalValue.fromLong(DecimalType.of(10, 2), 200); - DecimalValue decimal3 = DecimalValue.fromLong(DecimalType.of(10, 2), 100); - - Assert.assertTrue(decimal1.compareTo(decimal2) < 0); - Assert.assertTrue(decimal2.compareTo(decimal1) > 0); - Assert.assertEquals(0, decimal1.compareTo(decimal3)); + DecimalType t1 = DecimalType.of(33, 2); + DecimalType t2 = DecimalType.of(30, 9); + DecimalType t3 = DecimalType.of(11, 2); + + assertEquals(t1.newValue("1"), t2.newValue("1")); + assertEquals(t1.newValue("1.23").makeOptional(), t2.newValue("1.23")); + assertEquals(t1.newValue("-1.23"), t2.newValue("-1.23").makeOptional()); + + // the same scale + assertEquals(t3.newValue("999999999.99"), t1.newValue("999999999.99")); + assertEquals(t3.newValue("-999999999.99"), t1.newValue("-999999999.99")); + assertLess(t3.newValue("999999999.98"), t1.newValue("999999999.99")); + assertLess(t3.newValue("899999999.99"), t1.newValue("999999999.99")); + assertGreater(t3.newValue("-999999999.98"), t1.newValue("-999999999.99")); + assertGreater(t3.newValue("-899999999.99"), t1.newValue("-999999999.99")); + + // the differnt scales + assertEquals(t3.newValue("999999999.99"), t2.newValue("999999999.99")); + assertEquals(t3.newValue("-999999999.99"), t2.newValue("-999999999.99")); + assertLess(t3.newValue("999999999.99"), t2.newValue("1000000000")); + assertGreater(t3.newValue("-999999999.99"), t2.newValue("-1000000000")); + assertLess(t1.newValue("1.23"), t2.newValue("1.234")); + assertGreater(t1.newValue("-1.23"), t2.newValue("-1.234")); + + // special values + assertEquals(t1.getInf(), t2.getInf()); + assertEquals(t1.getNegInf(), t2.getNegInf()); + assertEquals(t1.getNaN(), t2.getNaN()); + + // type bounds + assertLess(t1.getNegInf(), t2.newValue("-999999999999999999999.999999999")); + assertEquals(t1.getNegInf(), t2.newValue("-1000000000000000000000")); + + assertGreater(t1.getInf(), t2.newValue("999999999999999999999.999999999")); + assertEquals(t1.getInf(), t2.newValue("1000000000000000000000")); + + assertGreater(t1.getNaN(), t2.newValue("999999999999999999999.999999999")); + assertGreater(t1.getNaN(), t2.newValue("9000000000000000000000")); + + // errors + assertNpe("Cannot compare with null value", t1.newValue("1"), null); + assertNpe("Cannot compare value 1.00 with NULL", t1.newValue("1"), t1.makeOptional().emptyValue()); + assertIllegalArgument("Cannot compare DecimalValue with Int32", t1.newValue("1"), PrimitiveValue.newInt32(1)); } } \ No newline at end of file From 2043e3ce6e500dca2c03fec7b4c4e3e67838fa27 Mon Sep 17 00:00:00 2001 From: Alexandr Gorshenin Date: Tue, 12 Aug 2025 10:57:51 +0100 Subject: [PATCH 17/17] Reverted internal array comparison --- .../tech/ydb/table/values/PrimitiveValue.java | 22 +++++++++++++++++-- .../ydb/table/values/ValueComparableTest.java | 9 ++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java index 92c305d4d..923d0e251 100644 --- a/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java +++ b/table/src/main/java/tech/ydb/table/values/PrimitiveValue.java @@ -469,9 +469,9 @@ public int compareTo(Value other) { case Double: return Double.compare(getDouble(), otherValue.getDouble()); case Bytes: - return Arrays.compare(getBytesUnsafe(), otherValue.getBytesUnsafe()); + return compareArrays(getBytesUnsafe(), otherValue.getBytesUnsafe()); case Yson: - return Arrays.compare(getYsonUnsafe(), otherValue.getYsonUnsafe()); + return compareArrays(getYsonUnsafe(), otherValue.getYsonUnsafe()); case Text: return getText().compareTo(otherValue.getText()); case Json: @@ -517,6 +517,24 @@ private static int compareUUID(PrimitiveValue a, PrimitiveValue b) { return (al != bl) ? Long.compareUnsigned(al, bl) : Long.compareUnsigned(ah, bh); } + private static int compareArrays(byte[] a, byte[] b) { + if (a == b) { + return 0; + } + + int i = 0; + int len = Math.min(a.length, b.length); + while (i < len && a[i] == b[i]) { + i++; + } + + if (i < len) { + return Byte.compare(a[i], b[i]); + } + + return a.length - b.length; + } + // -- implementations -- private static final class Bool extends PrimitiveValue { diff --git a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java index cebee0299..ad826c09d 100644 --- a/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java +++ b/table/src/test/java/tech/ydb/table/values/ValueComparableTest.java @@ -88,8 +88,13 @@ public void testPrimitiveValueComparison() { assertLess(PrimitiveValue.newFloat(1e-3f), PrimitiveValue.newFloat(1e-2f)); assertLess(PrimitiveValue.newDouble(1e-3d), PrimitiveValue.newDouble(1e-2d)); - assertLess(PrimitiveValue.newBytes(new byte[] { 0x01 }), PrimitiveValue.newBytes(new byte[] { 0x02 })); - assertLess(PrimitiveValue.newYson(new byte[] { 0x01 }), PrimitiveValue.newYson(new byte[] { 0x02 })); + byte[] b1 = new byte[] { 0x01, 0x02 }; + assertEquals(PrimitiveValue.newBytesOwn(b1), PrimitiveValue.newBytesOwn(b1)); + assertEquals(PrimitiveValue.newBytes(b1), PrimitiveValue.newBytes(new byte[] { 0x01, 0x02 })); + assertLess(PrimitiveValue.newBytes(b1), PrimitiveValue.newBytes(new byte[] { 0x01, 0x02, 0x1 })); + assertLess(PrimitiveValue.newBytes(b1), PrimitiveValue.newBytes(new byte[] { 0x02 })); + + assertLess(PrimitiveValue.newYson(b1), PrimitiveValue.newYson(new byte[] { 0x01, 0x03 })); assertLess(PrimitiveValue.newText("abc"), PrimitiveValue.newText("abcd")); assertLess(PrimitiveValue.newJson("['abc']"), PrimitiveValue.newJson("['abcd']"));