Skip to content

Commit

Permalink
[#23612] YSQL: Fix java unit test misuse of == for string comparison
Browse files Browse the repository at this point in the history
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
  • Loading branch information
myang2021 committed Aug 26, 2024
1 parent a9466df commit b9597b3
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion java/yb-client/src/main/java/org/yb/client/YBClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -1063,9 +1063,9 @@ public String getFlag(HostAndPort hp, String flag) throws Exception {
Deferred<GetFlagResponse> 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 "";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit b9597b3

Please sign in to comment.