From 237aae33c767fd974b8dda76ea0a8c961cc09c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 6 Nov 2023 12:00:01 +0100 Subject: [PATCH] Simplify internal logic of `Table.order_by`, avoid unnecessary warning (#8221) - Fixes #8213 --- .../Table/0.0.0-dev/src/Data/Table.enso | 3 +-- .../table/data/index/MultiValueIndex.java | 20 ------------------- .../table/data/index/MultiValueKeyBase.java | 4 ++++ .../java/org/enso/table/data/table/Table.java | 19 +++++++++++++----- .../Order_By_Spec.enso | 3 ++- 5 files changed, 21 insertions(+), 28 deletions(-) diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso index 7fd22e609d..1e467fb866 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso @@ -790,8 +790,7 @@ type Table Case_Sensitivity.Insensitive locale -> ObjectComparator.new False locale.java_locale java_table = Illegal_Argument.handle_java_exception <| Incomparable_Values.handle_errors <| - Java_Problems.with_problem_aggregator on_problems java_aggregator-> - self.java_table.orderBy java_columns directions comparator java_aggregator + self.java_table.orderBy java_columns directions comparator Table.Value java_table ## GROUP Standard.Base.Selections diff --git a/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueIndex.java b/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueIndex.java index c9a32433e3..7500ba6918 100644 --- a/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueIndex.java +++ b/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueIndex.java @@ -140,26 +140,6 @@ public class MultiValueIndex { .toArray(Column[]::new)); } - public int[] makeOrderMap(int rowCount) { - if (this.locs.size() == 0) { - return new int[0]; - } - - int[] output = new int[rowCount]; - - int idx = 0; - Context context = Context.getCurrent(); - for (List rowIndexes : this.locs.values()) { - for (Integer rowIndex : rowIndexes) { - assert idx < rowCount; - output[idx++] = rowIndex; - context.safepoint(); - } - } - - return output; - } - public Set keys() { return locs.keySet(); } diff --git a/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueKeyBase.java b/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueKeyBase.java index e55bc8a7dd..e8e33c4725 100644 --- a/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueKeyBase.java +++ b/std-bits/table/src/main/java/org/enso/table/data/index/MultiValueKeyBase.java @@ -36,6 +36,10 @@ public abstract class MultiValueKeyBase { return result; } + public int getRowIndex() { + return rowIndex; + } + @Override public abstract boolean equals(Object o); diff --git a/std-bits/table/src/main/java/org/enso/table/data/table/Table.java b/std-bits/table/src/main/java/org/enso/table/data/table/Table.java index 1609689951..25fd46c2fd 100644 --- a/std-bits/table/src/main/java/org/enso/table/data/table/Table.java +++ b/std-bits/table/src/main/java/org/enso/table/data/table/Table.java @@ -13,6 +13,8 @@ import org.enso.table.data.index.CrossTabIndex; import org.enso.table.data.index.DefaultIndex; import org.enso.table.data.index.Index; import org.enso.table.data.index.MultiValueIndex; +import org.enso.table.data.index.MultiValueKeyBase; +import org.enso.table.data.index.OrderedMultiValueKey; import org.enso.table.data.mask.OrderMask; import org.enso.table.data.mask.SliceRange; import org.enso.table.data.table.join.CrossJoin; @@ -207,12 +209,19 @@ public class Table { * @param objectComparator Object comparator allowing calling back to `compare_to` when needed. * @return a table indexed by the proper column */ - public Table orderBy(Column[] columns, Long[] directions, Comparator objectComparator, - ProblemAggregator problemAggregator) { + public Table orderBy(Column[] columns, Long[] directions, Comparator objectComparator) { int[] directionInts = Arrays.stream(directions).mapToInt(Long::intValue).toArray(); - MultiValueIndex index = MultiValueIndex.makeOrderedIndex(columns, this.rowCount(), directionInts, - objectComparator, problemAggregator); - OrderMask mask = new OrderMask(index.makeOrderMap(this.rowCount())); + int n = rowCount(); + Context context = Context.getCurrent(); + final Storage[] storages = Arrays.stream(columns).map(Column::getStorage).toArray(Storage[]::new); + OrderedMultiValueKey[] keys = new OrderedMultiValueKey[n]; + for (int i = 0; i < n; i++) { + keys[i] = new OrderedMultiValueKey(storages, i, directionInts, objectComparator); + context.safepoint(); + } + Arrays.sort(keys); + int[] positions = Arrays.stream(keys).mapToInt(MultiValueKeyBase::getRowIndex).toArray(); + OrderMask mask = new OrderMask(positions); return this.applyMask(mask); } diff --git a/test/Table_Tests/src/Common_Table_Operations/Order_By_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Order_By_Spec.enso index d27610919f..d5ec2b6c97 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Order_By_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Order_By_Spec.enso @@ -198,10 +198,11 @@ spec setup = t4.at "alpha" . to_vector . should_equal [1, 3, 0, 2] t4.at "gamma" . to_vector . should_equal [3, 1, 4, 2] - Test.specify "should deal with real numbers" <| + Test.specify "should deal with real numbers, and not warn when ordering by floats" <| t1 = table.order_by ["tau"] t1.at "tau" . to_vector . should_equal [-0.1, 0.5, 1.6, 32.0] t1.at "alpha" . to_vector . should_equal [1, 2, 0, 3] + Problems.assume_no_problems t1 Test.specify "should deal with nulls" <| t1 = table.order_by ["xi"]