From b9597b3dea9bdb0a1ee16116e5fe982a944db7f6 Mon Sep 17 00:00:00 2001 From: Minghui Yang Date: Fri, 23 Aug 2024 22:48:41 +0000 Subject: [PATCH] [#23612] YSQL: Fix java unit test misuse of == for string comparison Summary: The buggy code is ``` boolean is_wait_on_conflict_concurrency_control = (enable_wait_queues == "true"); ``` One answer from stackoverflow: In java, "==" compares Object references with each other and not their literal values. If both the variables point to same object, it will return true. So, String s1 = new String("hello"); String s2 = new String("hello"); Here s1==s2, will return false as both are different objects. When you use equals(), it will compare the literal values of the content and give its results. So the test is not doing what it intended to do because is_wait_on_conflict_concurrency_control is false. Also fixed another place where we can see misleading WARNING log for a valid gflag. Jira: DB-12524 Test Plan: ./yb_build.sh --java-test org.yb.pgsql.TestPgTransparentRestarts Reviewers: kfranz Reviewed By: kfranz Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D37516 --- java/yb-client/src/main/java/org/yb/client/YBClient.java | 2 +- .../src/test/java/org/yb/pgsql/TestPgTransparentRestarts.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/java/yb-client/src/main/java/org/yb/client/YBClient.java b/java/yb-client/src/main/java/org/yb/client/YBClient.java index 96df769d628a..8781cfb20262 100644 --- a/java/yb-client/src/main/java/org/yb/client/YBClient.java +++ b/java/yb-client/src/main/java/org/yb/client/YBClient.java @@ -1063,9 +1063,9 @@ public String getFlag(HostAndPort hp, String flag) throws Exception { Deferred d = asyncClient.getFlag(hp, flag); GetFlagResponse result = d.join(getDefaultAdminOperationTimeoutMs()); if (result.getValid()) { - LOG.warn("Invalid flag {}", flag); return result.getValue(); } + LOG.warn("Invalid flag {}", flag); return ""; } diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgTransparentRestarts.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgTransparentRestarts.java index 7849a60457d0..a190016bf6ca 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgTransparentRestarts.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgTransparentRestarts.java @@ -966,7 +966,7 @@ private Boolean expectConflictErrors(IsolationLevel isolation) throws Exception String enable_wait_queues = client.getFlag( miniCluster.getTabletServers().keySet().iterator().next(), "enable_wait_queues"); - boolean is_wait_on_conflict_concurrency_control = (enable_wait_queues == "true"); + boolean is_wait_on_conflict_concurrency_control = (enable_wait_queues.equals("true")); // We never expect REPEATABLE READ/READ COMMITTED transaction to result in "conflict" for pure // reads.