Skip to content

Commit c36ea9e

Browse files
opensearch-trigger-bot[bot]github-actions[bot]
opensearch-trigger-bot[bot]
and
github-actions[bot]
authored
Followup: Change ComparableLinkedHashMap to compare Key than Value (#4648) (#4655)
(cherry picked from commit d719670) Signed-off-by: Lantao Jin <ltjin@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 8d0dd94 commit c36ea9e

File tree

2 files changed

+61
-36
lines changed

2 files changed

+61
-36
lines changed

‎core/src/main/java/org/opensearch/sql/data/utils/ComparableLinkedHashMap.java‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,29 @@ private int compareRecursive(
4242
if (!thisHasNext) return -1;
4343
if (!otherHasNext) return 1;
4444

45-
V thisValue = thisIterator.next().getValue();
46-
V otherValue = otherIterator.next().getValue();
47-
int comparison = compareValues(thisValue, otherValue);
45+
Map.Entry<K, V> thisEntry = thisIterator.next();
46+
Map.Entry<K, V> otherEntry = otherIterator.next();
47+
K thisKey = thisEntry.getKey();
48+
K otherKey = otherEntry.getKey();
49+
V thisValue = thisEntry.getValue();
50+
V otherValue = otherEntry.getValue();
51+
int comparison = compareKV(thisKey, otherKey, thisValue, otherValue);
4852
if (comparison != 0) return comparison;
4953
return compareRecursive(thisIterator, otherIterator);
5054
}
5155

5256
@SuppressWarnings("unchecked")
53-
private int compareValues(V value1, V value2) {
57+
private int compareKV(K key1, K key2, V value1, V value2) {
58+
int keyCompare;
59+
if (key1 instanceof Comparable) {
60+
keyCompare = ((Comparable<K>) key1).compareTo(key2);
61+
} else {
62+
keyCompare = key1.toString().compareTo(key2.toString());
63+
}
64+
if (keyCompare != 0) {
65+
return keyCompare;
66+
}
67+
5468
if (value1 == null && value2 == null) return 0;
5569
if (value1 == null) return -1;
5670
if (value2 == null) return 1;

‎core/src/test/java/org/opensearch/sql/utils/ComparableLinkedHashMapTest.java‎

Lines changed: 43 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,26 @@ public void testOneEmptyMap() {
3434
assertTrue(map2.compareTo(map1) > 0);
3535
}
3636

37+
@Test
38+
public void testDifferentKeys() {
39+
ComparableLinkedHashMap<String, Integer> map1 = new ComparableLinkedHashMap<>();
40+
map1.put("a", 1);
41+
42+
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
43+
map2.put("b", 1);
44+
45+
assertTrue(map1.compareTo(map2) < 0);
46+
}
47+
3748
@Test
3849
public void testEqualMaps() {
3950
ComparableLinkedHashMap<String, Integer> map1 = new ComparableLinkedHashMap<>();
4051
map1.put("a", 1);
4152
map1.put("b", 2);
4253

4354
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
44-
map2.put("c", 1);
45-
map2.put("d", 2);
55+
map2.put("a", 1);
56+
map2.put("b", 2);
4657

4758
assertEquals(0, map1.compareTo(map2));
4859
}
@@ -54,8 +65,8 @@ public void testDifferentFirstValue() {
5465
map1.put("b", 2);
5566

5667
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
57-
map2.put("c", 3);
58-
map2.put("d", 2);
68+
map2.put("a", 3);
69+
map2.put("b", 2);
5970

6071
assertTrue(map1.compareTo(map2) < 0);
6172
assertTrue(map2.compareTo(map1) > 0);
@@ -69,9 +80,9 @@ public void testDifferentLaterValue() {
6980
map1.put("c", 3);
7081

7182
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
72-
map2.put("d", 1);
73-
map2.put("e", 3);
74-
map2.put("f", 3);
83+
map2.put("a", 1);
84+
map2.put("b", 3);
85+
map2.put("c", 3);
7586

7687
assertTrue(map1.compareTo(map2) < 0);
7788
assertTrue(map2.compareTo(map1) > 0);
@@ -84,7 +95,7 @@ public void testDifferentSizes() {
8495
map1.put("b", 2);
8596

8697
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
87-
map2.put("c", 1);
98+
map2.put("a", 1);
8899

89100
assertTrue(map1.compareTo(map2) > 0);
90101
assertTrue(map2.compareTo(map1) < 0);
@@ -97,15 +108,15 @@ public void testNullValues() {
97108
map1.put("b", 2);
98109

99110
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
100-
map2.put("c", 1);
101-
map2.put("d", 2);
111+
map2.put("a", 1);
112+
map2.put("b", 2);
102113

103114
assertTrue(map1.compareTo(map2) < 0);
104115
assertTrue(map2.compareTo(map1) > 0);
105116

106117
ComparableLinkedHashMap<String, Integer> map3 = new ComparableLinkedHashMap<>();
107-
map3.put("e", null);
108-
map3.put("f", 2);
118+
map3.put("a", null);
119+
map3.put("b", 2);
109120

110121
assertEquals(0, map1.compareTo(map3));
111122
}
@@ -130,8 +141,8 @@ public String toString() {
130141
map1.put("b", new Person("Bob"));
131142

132143
ComparableLinkedHashMap<String, Person> map2 = new ComparableLinkedHashMap<>();
133-
map2.put("c", new Person("Alice"));
134-
map2.put("d", new Person("Charlie"));
144+
map2.put("a", new Person("Alice"));
145+
map2.put("b", new Person("Charlie"));
135146

136147
assertTrue(map1.compareTo(map2) < 0);
137148
assertTrue(map2.compareTo(map1) > 0);
@@ -144,14 +155,14 @@ public void testMixedTypes() {
144155
map1.put("b", "test");
145156

146157
ComparableLinkedHashMap<String, Object> map2 = new ComparableLinkedHashMap<>();
147-
map2.put("c", 1);
148-
map2.put("d", "test");
158+
map2.put("a", 1);
159+
map2.put("b", "test");
149160

150161
assertEquals(0, map1.compareTo(map2));
151162

152163
ComparableLinkedHashMap<String, Object> map3 = new ComparableLinkedHashMap<>();
153-
map3.put("e", 1);
154-
map3.put("f", "test2");
164+
map3.put("a", 1);
165+
map3.put("b", "test2");
155166

156167
assertTrue(map1.compareTo(map3) < 0);
157168
assertTrue(map3.compareTo(map1) > 0);
@@ -166,12 +177,12 @@ public void testWithTreeSet() {
166177
map1.put("b", 2);
167178

168179
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
169-
map2.put("c", 1);
170-
map2.put("d", 3);
180+
map2.put("a", 1);
181+
map2.put("b", 3);
171182

172183
ComparableLinkedHashMap<String, Integer> map3 = new ComparableLinkedHashMap<>();
173-
map3.put("e", 0);
174-
map3.put("f", 4);
184+
map3.put("a", 0);
185+
map3.put("b", 4);
175186

176187
set.add(map2);
177188
set.add(map1);
@@ -182,14 +193,14 @@ public void testWithTreeSet() {
182193
ComparableLinkedHashMap<String, Integer> second = iterator.next();
183194
ComparableLinkedHashMap<String, Integer> third = iterator.next();
184195

185-
assertEquals(Integer.valueOf(0), first.get("e"));
186-
assertEquals(Integer.valueOf(4), first.get("f"));
196+
assertEquals(Integer.valueOf(0), first.get("a"));
197+
assertEquals(Integer.valueOf(4), first.get("b"));
187198

188199
assertEquals(Integer.valueOf(1), second.get("a"));
189200
assertEquals(Integer.valueOf(2), second.get("b"));
190201

191-
assertEquals(Integer.valueOf(1), third.get("c"));
192-
assertEquals(Integer.valueOf(3), third.get("d"));
202+
assertEquals(Integer.valueOf(1), third.get("a"));
203+
assertEquals(Integer.valueOf(3), third.get("b"));
193204

194205
assertEquals(3, set.size());
195206
}
@@ -203,7 +214,7 @@ public void testWithComparator() {
203214
map1.put("a", 5);
204215

205216
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
206-
map2.put("b", 3);
217+
map2.put("a", 3);
207218

208219
assertTrue(comparator.compare(map1, map2) > 0);
209220
assertTrue(comparator.compare(map2, map1) < 0);
@@ -221,7 +232,7 @@ public void testEqualValuesDifferentKeys() {
221232
map2.put("e", 2);
222233
map2.put("f", 3);
223234

224-
assertEquals(0, map1.compareTo(map2));
235+
assertTrue(map1.compareTo(map2) < 0);
225236
}
226237

227238
@Test
@@ -231,8 +242,8 @@ public void testDifferentOrder() {
231242
map1.put("b", 2);
232243

233244
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
234-
map2.put("c", 2);
235-
map2.put("d", 1);
245+
map2.put("a", 2);
246+
map2.put("b", 1);
236247

237248
assertTrue(map1.compareTo(map2) < 0);
238249
assertTrue(map2.compareTo(map1) > 0);
@@ -247,12 +258,12 @@ public void testLargeMaps() {
247258

248259
ComparableLinkedHashMap<String, Integer> map2 = new ComparableLinkedHashMap<>();
249260
for (int i = 0; i < 100; i++) {
250-
map2.put("differentKey" + i, i);
261+
map2.put("key" + i, i);
251262
}
252263

253264
assertEquals(0, map1.compareTo(map2));
254265

255-
map2.put("differentKey99", 100);
266+
map2.put("key", 100);
256267
assertTrue(map1.compareTo(map2) < 0);
257268
}
258269
}

0 commit comments

Comments
 (0)