diff --git a/api/src/main/java/org/eclipse/daanse/olap/api/execution/Execution.java b/api/src/main/java/org/eclipse/daanse/olap/api/execution/Execution.java index 75e7fee..1635641 100644 --- a/api/src/main/java/org/eclipse/daanse/olap/api/execution/Execution.java +++ b/api/src/main/java/org/eclipse/daanse/olap/api/execution/Execution.java @@ -91,4 +91,8 @@ public enum State { */ FRESH, RUNNING, ERROR, CANCELED, TIMEOUT, DONE, } + + public enum Purpose { + DRILL_THROUGH, CELL_SEGMENT, TUPLES, OTHER + } } diff --git a/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadata.java b/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadata.java index 537ffff..83f2735 100644 --- a/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadata.java +++ b/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadata.java @@ -13,8 +13,6 @@ */ package org.eclipse.daanse.olap.api.execution; -import org.eclipse.daanse.olap.api.monitor.event.SqlStatementEvent; - /** * Metadata associated with an execution context for tracing, monitoring, and * debugging. @@ -54,7 +52,7 @@ public interface ExecutionMetadata { * * @return the purpose, or null if not set */ - SqlStatementEvent.Purpose purpose(); + Execution.Purpose purpose(); /** * Returns the number of cell requests associated with this execution. Returns 0 @@ -82,7 +80,7 @@ static ExecutionMetadata empty() { * @param cellRequestCount the cell request count * @return an ExecutionMetadata instance */ - static ExecutionMetadata of(String component, String message, SqlStatementEvent.Purpose purpose, + static ExecutionMetadata of(String component, String message, Execution.Purpose purpose, int cellRequestCount) { return new ExecutionMetadataRecord(component, message, purpose, cellRequestCount); } diff --git a/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadataRecord.java b/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadataRecord.java index 8aa2ab7..1c1b6ec 100644 --- a/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadataRecord.java +++ b/api/src/main/java/org/eclipse/daanse/olap/api/execution/ExecutionMetadataRecord.java @@ -13,8 +13,6 @@ */ package org.eclipse.daanse.olap.api.execution; -import org.eclipse.daanse.olap.api.monitor.event.SqlStatementEvent; - /** * Record implementation of ExecutionMetadata. * @@ -23,6 +21,6 @@ * @param purpose the SQL statement purpose (may be null) * @param cellRequestCount the cell request count */ -record ExecutionMetadataRecord(String component, String message, SqlStatementEvent.Purpose purpose, +record ExecutionMetadataRecord(String component, String message, Execution.Purpose purpose, int cellRequestCount) implements ExecutionMetadata { } diff --git a/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEvent.java b/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEvent.java index 0849552..d8e147d 100644 --- a/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEvent.java +++ b/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEvent.java @@ -30,7 +30,5 @@ public sealed interface SqlStatementEvent extends OlapEvent permits SqlStatementStartEvent, SqlStatementEndEvent, SqlStatementExecuteEvent { - public enum Purpose { - DRILL_THROUGH, CELL_SEGMENT, TUPLES, OTHER - } + } diff --git a/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEventCommon.java b/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEventCommon.java index 627f9bc..d1e07b3 100644 --- a/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEventCommon.java +++ b/api/src/main/java/org/eclipse/daanse/olap/api/monitor/event/SqlStatementEventCommon.java @@ -13,7 +13,7 @@ package org.eclipse.daanse.olap.api.monitor.event; -import org.eclipse.daanse.olap.api.monitor.event.SqlStatementEvent.Purpose; +import org.eclipse.daanse.olap.api.execution.Execution.Purpose; public record SqlStatementEventCommon(EventCommon eventCommon, long mdxStatementId, long sqlStatementId, String sql, Purpose purpose) { diff --git a/common/src/main/java/org/eclipse/daanse/olap/fun/sort/Sorter.java b/common/src/main/java/org/eclipse/daanse/olap/fun/sort/Sorter.java index 146679f..22308ef 100644 --- a/common/src/main/java/org/eclipse/daanse/olap/fun/sort/Sorter.java +++ b/common/src/main/java/org/eclipse/daanse/olap/fun/sort/Sorter.java @@ -644,7 +644,7 @@ public static int compareValues( Object value0, Object value1 ) { return ( (Date) value0 ).compareTo( (Date) value1 ); } else if ( value0 instanceof LocalDateTime ) { return ( (LocalDateTime) value0 ).compareTo( (LocalDateTime) value1 ); - } else if ( value0 instanceof OrderKey orderKey0 && value0 instanceof OrderKey orderKey1 ) { + } else if ( value0 instanceof OrderKey orderKey0 && value1 instanceof OrderKey orderKey1 ) { return orderKey0.compareTo( orderKey1); } else { throw newInternal( "cannot compare " + value0 ); diff --git a/common/src/test/java/org/eclipse/daanse/olap/fun/sort/SorterCompareValuesTest.java b/common/src/test/java/org/eclipse/daanse/olap/fun/sort/SorterCompareValuesTest.java new file mode 100644 index 0000000..ce8b4ee --- /dev/null +++ b/common/src/test/java/org/eclipse/daanse/olap/fun/sort/SorterCompareValuesTest.java @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.eclipse.daanse.olap.fun.sort; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.eclipse.daanse.olap.api.element.Member; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +/** + * Tests for {@link Sorter#compareValues(Object, Object)} method. + */ +class SorterCompareValuesTest { + + /** + * Test that compareValues correctly compares two different OrderKey objects. + * This test exposes the bug where value0 was checked twice instead of value0 and value1, + * causing all OrderKey comparisons to incorrectly return 0. + */ + @Test + void testCompareValuesWithDifferentOrderKeys() { + // Create two members with different order keys + Member memberA = mock(Member.class, Mockito.withSettings().stubOnly()); + Member memberB = mock(Member.class, Mockito.withSettings().stubOnly()); + + when(memberA.isCalculatedInQuery()).thenReturn(false); + when(memberB.isCalculatedInQuery()).thenReturn(false); + when(memberA.getOrderKey()).thenReturn("A"); + when(memberB.getOrderKey()).thenReturn("B"); + + OrderKey orderKey1 = new OrderKey(memberA); + OrderKey orderKey2 = new OrderKey(memberB); + + // The comparison should NOT be 0 since they have different order keys + int result = Sorter.compareValues(orderKey1, orderKey2); + + // "A".compareTo("B") should return negative value + assertThat(result) + .as("Comparing OrderKey with 'A' to OrderKey with 'B' should return negative") + .isLessThan(0); + + // Also test the reverse + int reverseResult = Sorter.compareValues(orderKey2, orderKey1); + assertThat(reverseResult) + .as("Comparing OrderKey with 'B' to OrderKey with 'A' should return positive") + .isGreaterThan(0); + } + + /** + * Test that compareValues returns 0 for equal OrderKey objects. + */ + @Test + void testCompareValuesWithEqualOrderKeys() { + Member memberA = mock(Member.class, Mockito.withSettings().stubOnly()); + Member memberB = mock(Member.class, Mockito.withSettings().stubOnly()); + + when(memberA.isCalculatedInQuery()).thenReturn(false); + when(memberB.isCalculatedInQuery()).thenReturn(false); + when(memberA.getOrderKey()).thenReturn("Same"); + when(memberB.getOrderKey()).thenReturn("Same"); + + OrderKey orderKey1 = new OrderKey(memberA); + OrderKey orderKey2 = new OrderKey(memberB); + + int result = Sorter.compareValues(orderKey1, orderKey2); + + assertThat(result) + .as("Comparing OrderKeys with same value 'Same' should return 0") + .isEqualTo(0); + } +}