Skip to content

Commit b88e7b0

Browse files
committed
fix: correct comparison logic in Sorter and add unit tests for OrderKey
comparisons Signed-off-by: Stefan Bischof <[email protected]>
1 parent 9f38757 commit b88e7b0

File tree

2 files changed

+82
-1
lines changed

2 files changed

+82
-1
lines changed

common/src/main/java/org/eclipse/daanse/olap/fun/sort/Sorter.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,7 +644,7 @@ public static int compareValues( Object value0, Object value1 ) {
644644
return ( (Date) value0 ).compareTo( (Date) value1 );
645645
} else if ( value0 instanceof LocalDateTime ) {
646646
return ( (LocalDateTime) value0 ).compareTo( (LocalDateTime) value1 );
647-
} else if ( value0 instanceof OrderKey orderKey0 && value0 instanceof OrderKey orderKey1 ) {
647+
} else if ( value0 instanceof OrderKey orderKey0 && value1 instanceof OrderKey orderKey1 ) {
648648
return orderKey0.compareTo( orderKey1);
649649
} else {
650650
throw newInternal( "cannot compare " + value0 );
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Copyright (c) 2026 Contributors to the Eclipse Foundation.
3+
*
4+
* This program and the accompanying materials are made
5+
* available under the terms of the Eclipse Public License 2.0
6+
* which is available at https://www.eclipse.org/legal/epl-2.0/
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
*/
10+
package org.eclipse.daanse.olap.fun.sort;
11+
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
import static org.mockito.Mockito.mock;
14+
import static org.mockito.Mockito.when;
15+
16+
import org.eclipse.daanse.olap.api.element.Member;
17+
import org.junit.jupiter.api.Test;
18+
import org.mockito.Mockito;
19+
20+
/**
21+
* Tests for {@link Sorter#compareValues(Object, Object)} method.
22+
*/
23+
class SorterCompareValuesTest {
24+
25+
/**
26+
* Test that compareValues correctly compares two different OrderKey objects.
27+
* This test exposes the bug where value0 was checked twice instead of value0 and value1,
28+
* causing all OrderKey comparisons to incorrectly return 0.
29+
*/
30+
@Test
31+
void testCompareValuesWithDifferentOrderKeys() {
32+
// Create two members with different order keys
33+
Member memberA = mock(Member.class, Mockito.withSettings().stubOnly());
34+
Member memberB = mock(Member.class, Mockito.withSettings().stubOnly());
35+
36+
when(memberA.isCalculatedInQuery()).thenReturn(false);
37+
when(memberB.isCalculatedInQuery()).thenReturn(false);
38+
when(memberA.getOrderKey()).thenReturn("A");
39+
when(memberB.getOrderKey()).thenReturn("B");
40+
41+
OrderKey orderKey1 = new OrderKey(memberA);
42+
OrderKey orderKey2 = new OrderKey(memberB);
43+
44+
// The comparison should NOT be 0 since they have different order keys
45+
int result = Sorter.compareValues(orderKey1, orderKey2);
46+
47+
// "A".compareTo("B") should return negative value
48+
assertThat(result)
49+
.as("Comparing OrderKey with 'A' to OrderKey with 'B' should return negative")
50+
.isLessThan(0);
51+
52+
// Also test the reverse
53+
int reverseResult = Sorter.compareValues(orderKey2, orderKey1);
54+
assertThat(reverseResult)
55+
.as("Comparing OrderKey with 'B' to OrderKey with 'A' should return positive")
56+
.isGreaterThan(0);
57+
}
58+
59+
/**
60+
* Test that compareValues returns 0 for equal OrderKey objects.
61+
*/
62+
@Test
63+
void testCompareValuesWithEqualOrderKeys() {
64+
Member memberA = mock(Member.class, Mockito.withSettings().stubOnly());
65+
Member memberB = mock(Member.class, Mockito.withSettings().stubOnly());
66+
67+
when(memberA.isCalculatedInQuery()).thenReturn(false);
68+
when(memberB.isCalculatedInQuery()).thenReturn(false);
69+
when(memberA.getOrderKey()).thenReturn("Same");
70+
when(memberB.getOrderKey()).thenReturn("Same");
71+
72+
OrderKey orderKey1 = new OrderKey(memberA);
73+
OrderKey orderKey2 = new OrderKey(memberB);
74+
75+
int result = Sorter.compareValues(orderKey1, orderKey2);
76+
77+
assertThat(result)
78+
.as("Comparing OrderKeys with same value 'Same' should return 0")
79+
.isEqualTo(0);
80+
}
81+
}

0 commit comments

Comments
 (0)