From f0adc945717c54f7912efd2faa2cc58ff8c68a7f Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Tue, 21 Jan 2025 11:33:48 +0000 Subject: [PATCH 1/9] Column offset --- .../Database/0.0.0-dev/src/DB_Column.enso | 20 +++++++ .../Standard/Table/0.0.0-dev/src/Column.enso | 19 ++++++ .../0.0.0-dev/src/Internal/Offset_Helper.enso | 7 +++ .../expressions/ExpressionVisitorImpl.java | 51 ++++++++++++++++ .../org/enso/table/operations/Offset.java | 60 +++++++++++++++++-- .../Common_Table_Operations/Offset_Spec.enso | 33 ++++++++-- 6 files changed, 180 insertions(+), 10 deletions(-) diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso index 8913dd4e9164..3c139e902d92 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso @@ -5,6 +5,7 @@ import Standard.Base.Errors.Common.Index_Out_Of_Bounds import Standard.Base.Errors.Illegal_Argument.Illegal_Argument import Standard.Base.Errors.Illegal_State.Illegal_State import Standard.Base.Internal.Rounding_Helpers +from Standard.Base.Metadata.Widget import Numeric_Input from Standard.Base.Widget_Helpers import make_data_cleanse_vector_selector, make_format_chooser, make_regex_text_widget import Standard.Table.Internal.Column_Naming_Helper.Column_Naming_Helper @@ -13,6 +14,7 @@ import Standard.Table.Internal.Java_Problems import Standard.Table.Internal.Problem_Builder.Problem_Builder import Standard.Table.Internal.Value_Type_Helpers import Standard.Table.Internal.Widget_Helpers +import Standard.Table.Fill_With.Fill_With import Standard.Table.Rows_To_Read.Rows_To_Read from Standard.Table import Auto, Column, Data_Formatter, Previous_Value, Sort_Column, Table, Value_Type from Standard.Table.Column import default_date_period @@ -1320,6 +1322,24 @@ type DB_Column result = self.is_empty.iif default self result.rename self.name + ## ALIAS shift, lead, lag, slide, displace + GROUP Standard.Base.Values + ICON column_add + + Returns a new column offset by n rows, where missing values have been replaced with the provided fill_with strategy. + + Arguments: + - n: The number of rows to offset the new column by. Negative n slides the values down in the column, adding records at the start. + Positive n slides the values up in the column, adding records at the end. Defaults to -1. + - fill_with: The value to replace missing values with. Defaults to adding Nothing Values. + - - ..Nothing - Add Nothing values in the spaces created by sliding the existing values. + - - ..Closest_Value - If n is negative the first value gets used, if n is negative the last value gets used. + - - ..Wrap_Around - In this mode values that slide off the top or bottom reappear at the other end. So no values get lost they are just rotated. + @n (Numeric_Input display=..Always) + offset self n=-1:Integer fill_with:Fill_With=..Nothing -> Column = + _ = [n, fill_with] + Error.throw (Unsupported_Database_Operation.Error "offset") + ## GROUP Standard.Base.Metadata ICON text_input Returns a new column, containing the same elements as `self`, but with diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso index 6cf1bf731e1d..149ccd236e24 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso @@ -23,11 +23,13 @@ import project.Internal.Column_Naming_Helper.Column_Naming_Helper import project.Internal.Column_Ops import project.Internal.Date_Time_Helpers import project.Internal.Java_Problems +import project.Internal.Offset_Helper import project.Internal.Parse_Values_Helper import project.Internal.Read_Many_Helpers import project.Internal.Storage import project.Internal.Value_Type_Helpers import project.Internal.Widget_Helpers +import project.Fill_With.Fill_With import project.Rows_To_Read.Rows_To_Read import project.Table.Table import project.Value_Type.Auto @@ -2608,6 +2610,23 @@ type Column data = Statistic.running self.to_vector statistic Column.from_vector name data + ## ALIAS shift, lead, lag, slide, displace + GROUP Standard.Base.Values + ICON column_add + + Returns a new column offset by n rows, where missing values have been replaced with the provided fill_with strategy. + + Arguments: + - n: The number of rows to offset the new column by. Negative n slides the values down in the column, adding records at the start. + Positive n slides the values up in the column, adding records at the end. Defaults to -1. + - fill_with: The value to replace missing values with. Defaults to adding Nothing Values. + - - ..Nothing - Add Nothing values in the spaces created by sliding the existing values. + - - ..Closest_Value - If n is negative the first value gets used, if n is negative the last value gets used. + - - ..Wrap_Around - In this mode values that slide off the top or bottom reappear at the other end. So no values get lost they are just rotated. + @n (Numeric_Input display=..Always) + offset self n=-1:Integer fill_with:Fill_With=..Nothing -> Column = + Offset_Helper.column_offset_impl self n fill_with + ## PRIVATE pretty : Text pretty self = diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso index df6c8da657ed..6df67e522219 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso @@ -32,3 +32,10 @@ table_offset_impl table:Table (columns:(Vector | Text | Integer | Regex)=[]) n:I new_columns = new_names.zip new_storages.to_vector new_name-> new_storage -> Column.from_storage new_name new_storage new_columns.fold table t-> c-> t.set c c.name set_mode + +column_offset_impl column:Column n:Integer=-1 fillWith:Fill_With=..Nothing = + java_column = column.java_column + fillWith_java = fillWith.to_java + new_storage = Java_Offset.offset_single_column java_column n fillWith_java + new_name = Column_Naming_Helper.in_memory.function_name "offset" [column, n, fillWith] + Column.from_storage new_name new_storage \ No newline at end of file diff --git a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java index d7e55682a836..0ca9b5ff6513 100644 --- a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java +++ b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java @@ -119,6 +119,9 @@ private Value wrapAsColumn(Value value) { } private Value executeMethod(String name, Value... args) { + var ret = tryDirectNumericCalculation(name, args); + if (ret != null) return ret; + Value method = getMethod.apply(name); if (!method.canExecute()) { throw new UnsupportedOperationException(name); @@ -148,6 +151,54 @@ private Value executeMethod(String name, Value... args) { } } + private Value tryDirectNumericCalculation(String name, Value... args) { + // Check for arithmetic operations + if (args.length == 2 && args[0].isNumber() && args[1].isNumber()) { + if (isInt(args[0]) && isInt(args[1])) { + switch (name) { + case "+" -> { + return Value.asValue(args[0].asInt() + args[1].asInt()); + } + case "-" -> { + return Value.asValue(args[0].asInt() - args[1].asInt()); + } + case "*" -> { + return Value.asValue(args[0].asInt() * args[1].asInt()); + } + case "/" -> { + return Value.asValue(args[0].asInt() / args[1].asInt()); + } + default -> { + return null; + } + } + } else { + switch (name) { + case "+" -> { + return Value.asValue(args[0].asDouble() + args[1].asDouble()); + } + case "-" -> { + return Value.asValue(args[0].asDouble() - args[1].asDouble()); + } + case "*" -> { + return Value.asValue(args[0].asDouble() * args[1].asDouble()); + } + case "/" -> { + return Value.asValue(args[0].asDouble() / args[1].asDouble()); + } + default -> { + return null; + } + } + } + } + return null; + } + + private Boolean isInt(Value v) { + return v.isNumber() && v.asDouble() == v.asInt(); + } + @Override public Value visitProg(ExpressionParser.ProgContext ctx) { Value base = visit(ctx.expr()); diff --git a/std-bits/table/src/main/java/org/enso/table/operations/Offset.java b/std-bits/table/src/main/java/org/enso/table/operations/Offset.java index f8979986a31b..b2568b7408b5 100644 --- a/std-bits/table/src/main/java/org/enso/table/operations/Offset.java +++ b/std-bits/table/src/main/java/org/enso/table/operations/Offset.java @@ -3,6 +3,8 @@ import java.util.Arrays; import java.util.LinkedList; import java.util.Queue; +import java.util.stream.IntStream; + import org.enso.table.data.column.storage.Storage; import org.enso.table.data.mask.OrderMask; import org.enso.table.data.table.Column; @@ -19,6 +21,58 @@ public static Storage[] offset( ProblemAggregator problemAggregator) { if (n == 0 || sourceColumns.length == 0) return Arrays.stream(sourceColumns).map(c -> c.getStorage()).toArray(Storage[]::new); + var rowOrderMask = + groupingColumns.length == 0 && orderingColumns.length == 0 + ? calculate_ungrouped_unordered_mask(sourceColumns[0].getSize(), n, fillWith) + : calculate_grouped_ordered_mask( + sourceColumns, + n, + fillWith, + groupingColumns, + orderingColumns, + directions, + problemAggregator); + return Arrays.stream(sourceColumns) + .map(c -> c.getStorage().applyMask(OrderMask.fromArray(rowOrderMask))) + .toArray(Storage[]::new); + } + + public static Storage offset_single_column(Column sourceColumn, int n, FillWith fillWith) { + if (n == 0) return sourceColumn.getStorage(); + var rowOrderMask = calculate_ungrouped_unordered_mask(sourceColumn.getSize(), n, fillWith); + return sourceColumn.getStorage().applyMask(OrderMask.fromArray(rowOrderMask)); + } + + private static int[] calculate_ungrouped_unordered_mask(int size, int n, FillWith fillWith) { + return IntStream.range(0, size).map(i -> calculate_row_offset(i, n, fillWith, size)).toArray(); + } + + private static int calculate_row_offset(int i, int n, FillWith fillWith, int size) { + int result = i + n; + if (result < 0) { + return switch (fillWith) { + case NOTHING -> Storage.NOT_FOUND_INDEX; + case CLOSEST_VALUE -> 0; + case WRAP_AROUND -> (result % size) == 0 ? 0 : (result % size) + size; + }; + } else if (result >= size) { + return switch (fillWith) { + case NOTHING -> Storage.NOT_FOUND_INDEX; + case CLOSEST_VALUE -> size - 1; + case WRAP_AROUND -> result % size; + }; + } + return result; + } + + private static int[] calculate_grouped_ordered_mask( + Column[] sourceColumns, + int n, + FillWith fillWith, + Column[] groupingColumns, + Column[] orderingColumns, + int[] directions, + ProblemAggregator problemAggregator) { var offsetRowVisitorFactory = new OffsetRowVisitorFactory(sourceColumns[0], n, fillWith); GroupingOrderingVisitor.visit( groupingColumns, @@ -27,11 +81,7 @@ public static Storage[] offset( problemAggregator, offsetRowVisitorFactory, sourceColumns[0].getSize()); - return Arrays.stream(sourceColumns) - .map( - c -> - c.getStorage().applyMask(OrderMask.fromArray(offsetRowVisitorFactory.rowOrderMask))) - .toArray(Storage[]::new); + return offsetRowVisitorFactory.rowOrderMask; } private static class OffsetRowVisitorFactory implements RowVisitorFactory { diff --git a/test/Table_Tests/src/Common_Table_Operations/Offset_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Offset_Spec.enso index 048b67f4309e..1ab77de5a6d7 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Offset_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Offset_Spec.enso @@ -1,5 +1,6 @@ from Standard.Base import all from Standard.Test import all +from Standard.Table import all from Standard.Database.Errors import Unsupported_Database_Operation import Standard.Database.Feature.Feature @@ -19,6 +20,9 @@ add_specs suite_builder setup = t = table_builder [["ix", [1, 2, 3, 4, 5]], ["X", [100, 3, Nothing, 4, 12]], ["Y", [100, 4, 2, Nothing, 11]]] t2 = t.offset ["X"] t2.should_fail_with (Unsupported_Database_Operation.Error "offset") + c = t.at 0 + c2 = c.offset + c2.should_fail_with (Unsupported_Database_Operation.Error "offset") add_offset_specs suite_builder setup = prefix = setup.prefix @@ -27,11 +31,11 @@ add_offset_specs suite_builder setup = c = t.at 0 c_nothings = t.at 1 c_zero_rows = t.take 0 . at 0 - suite_builder.group prefix+"Column.Offset with default fill strategy" pending="TODO Column.Offset" group_builder-> + suite_builder.group prefix+"Column.Offset with default fill strategy" group_builder-> group_builder.specify "Works with default values" <| r = c.offset r.to_vector . should_equal [Nothing, 1, 2] - r.name . should_equal c.name + r.name . should_equal "offset([A], -1, Fill_With.Nothing)" group_builder.specify "Negative n shifts the values down" <| r = c.offset -1 r.to_vector . should_equal [Nothing, 1, 2] @@ -50,7 +54,7 @@ add_offset_specs suite_builder setup = group_builder.specify "Works with zero rows" <| r = c_zero_rows.offset r.to_vector . should_equal [] - suite_builder.group prefix+"Column.Offset with closest value fill strategy" pending="TODO Column.Offset" group_builder-> + suite_builder.group prefix+"Column.Offset with closest value fill strategy" group_builder-> group_builder.specify "Negative n shifts the values down" <| r = c.offset -1 ..Closest_Value r.to_vector . should_equal [1, 1, 2] @@ -75,7 +79,7 @@ add_offset_specs suite_builder setup = group_builder.specify "Works with positive n and column of nothings" <| r = c_nothings.offset 1 ..Closest_Value r.to_vector . should_equal [Nothing, Nothing, Nothing] - suite_builder.group prefix+"Column.Offset with wrap around fill strategy" pending="TODO Column.Offset" group_builder-> + suite_builder.group prefix+"Column.Offset with wrap around fill strategy" group_builder-> group_builder.specify "Negative n shifts the values down" <| r = c.offset -1 ..Wrap_Around r.to_vector . should_equal [3, 1, 2] @@ -106,7 +110,7 @@ add_offset_specs suite_builder setup = group_builder.specify "Works with positive n and column of nothings" <| r = c_nothings.offset 1 ..Wrap_Around r.to_vector . should_equal [Nothing, Nothing, Nothing] - suite_builder.group prefix+"Column.Offset with constant fill strategy" pending="TODO Column.Offset" group_builder-> + suite_builder.group prefix+"Column.Offset with constant fill strategy" pending="TODO - constant fill strategy" group_builder-> group_builder.specify "Negative n shifts the values down" <| r = c.offset -1 42 r.to_vector . should_equal [42, 1, 2] @@ -134,6 +138,25 @@ add_offset_specs suite_builder setup = group_builder.specify "Can create mixed colums" <| r = c.offset -1 "42" r.to_vector . should_equal ["42", 1, 2] + suite_builder.group prefix+"Works in Table.set expressions with default" group_builder-> + group_builder.specify "Works with default values" <| + r = t.set (expr 'offset([A])') . at "offset([A])" + r.to_vector . should_equal [Nothing, 1, 2] + group_builder.specify "Negative n shifts the values down" <| + r = t.set (expr 'offset([A], -1)') . at "offset([A], -1)" + r.to_vector . should_equal [Nothing, 1, 2] + group_builder.specify "Positive n shifts the values up" <| + r = t.set (expr 'offset([A], 1)') . at "offset([A], 1)" + r.to_vector . should_equal [2, 3, Nothing] + group_builder.specify "Zero n is a no-op" <| + r = t.set (expr 'offset([A], 0)') . at "offset([A], 0)" + r.to_vector . should_equal [1, 2, 3] + group_builder.specify "Large negative n values work" <| + r = t.set (expr 'offset([A], -1024)') . at "offset([A], -1024)" + r.to_vector . should_equal [Nothing, Nothing, Nothing] + group_builder.specify "Large positive n values work" <| + r = t.set (expr 'offset([A], 1024)') . at "offset([A], 1024)" + r.to_vector . should_equal [Nothing, Nothing, Nothing] suite_builder.group prefix+"Table.Offset with default fill strategy" group_builder-> group_builder.specify "Works with default values" <| r = t.offset ["A"] From 9160007d2bbc4dadcdd69a7e05a30ce97d0cd00f Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Tue, 21 Jan 2025 11:36:23 +0000 Subject: [PATCH 2/9] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99cfeee6c379..62c36a4bb573 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,9 +24,11 @@ - [Allow using `/` to access files inside a directory reached through a data link.][11926] - [Added Table.Offset][12071] +- [Added Column.Offset][12092] [11926]: https://github.com/enso-org/enso/pull/11926 [12071]: https://github.com/enso-org/enso/pull/12071 +[12092]: https://github.com/enso-org/enso/pull/12092 #### Enso Language & Runtime From 5de39ee98111beedeefc96f2e1a3f9619e11465f Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Tue, 21 Jan 2025 11:55:30 +0000 Subject: [PATCH 3/9] Final tweaks --- distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso | 2 +- distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso | 2 +- .../table/src/main/java/org/enso/table/operations/Offset.java | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso index 3c139e902d92..3e1c6c11a665 100644 --- a/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso +++ b/distribution/lib/Standard/Database/0.0.0-dev/src/DB_Column.enso @@ -1335,7 +1335,7 @@ type DB_Column - - ..Nothing - Add Nothing values in the spaces created by sliding the existing values. - - ..Closest_Value - If n is negative the first value gets used, if n is negative the last value gets used. - - ..Wrap_Around - In this mode values that slide off the top or bottom reappear at the other end. So no values get lost they are just rotated. - @n (Numeric_Input display=..Always) + @n (self-> Numeric_Input minimum=0-self.length maximum=self.length-1) offset self n=-1:Integer fill_with:Fill_With=..Nothing -> Column = _ = [n, fill_with] Error.throw (Unsupported_Database_Operation.Error "offset") diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso index 149ccd236e24..02b342bbb664 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Column.enso @@ -2623,7 +2623,7 @@ type Column - - ..Nothing - Add Nothing values in the spaces created by sliding the existing values. - - ..Closest_Value - If n is negative the first value gets used, if n is negative the last value gets used. - - ..Wrap_Around - In this mode values that slide off the top or bottom reappear at the other end. So no values get lost they are just rotated. - @n (Numeric_Input display=..Always) + @n (self-> Numeric_Input minimum=0-self.length maximum=self.length) offset self n=-1:Integer fill_with:Fill_With=..Nothing -> Column = Offset_Helper.column_offset_impl self n fill_with diff --git a/std-bits/table/src/main/java/org/enso/table/operations/Offset.java b/std-bits/table/src/main/java/org/enso/table/operations/Offset.java index b2568b7408b5..8ff4ed722e98 100644 --- a/std-bits/table/src/main/java/org/enso/table/operations/Offset.java +++ b/std-bits/table/src/main/java/org/enso/table/operations/Offset.java @@ -4,7 +4,6 @@ import java.util.LinkedList; import java.util.Queue; import java.util.stream.IntStream; - import org.enso.table.data.column.storage.Storage; import org.enso.table.data.mask.OrderMask; import org.enso.table.data.table.Column; From 7ee185e010fe52a648ee814546d956cb5b61f428 Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Tue, 21 Jan 2025 11:56:18 +0000 Subject: [PATCH 4/9] Newline --- .../Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso index 6df67e522219..1349528aceb7 100644 --- a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso +++ b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Offset_Helper.enso @@ -38,4 +38,4 @@ column_offset_impl column:Column n:Integer=-1 fillWith:Fill_With=..Nothing = fillWith_java = fillWith.to_java new_storage = Java_Offset.offset_single_column java_column n fillWith_java new_name = Column_Naming_Helper.in_memory.function_name "offset" [column, n, fillWith] - Column.from_storage new_name new_storage \ No newline at end of file + Column.from_storage new_name new_storage From 5f9e50c5efcfb29c21249d1ec1d15dce9d81ef72 Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Tue, 21 Jan 2025 13:58:45 +0000 Subject: [PATCH 5/9] Refactor --- .../org/enso/table/operations/Offset.java | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/std-bits/table/src/main/java/org/enso/table/operations/Offset.java b/std-bits/table/src/main/java/org/enso/table/operations/Offset.java index 8ff4ed722e98..78a73dabc410 100644 --- a/std-bits/table/src/main/java/org/enso/table/operations/Offset.java +++ b/std-bits/table/src/main/java/org/enso/table/operations/Offset.java @@ -24,7 +24,7 @@ public static Storage[] offset( groupingColumns.length == 0 && orderingColumns.length == 0 ? calculate_ungrouped_unordered_mask(sourceColumns[0].getSize(), n, fillWith) : calculate_grouped_ordered_mask( - sourceColumns, + sourceColumns[0].getSize(), n, fillWith, groupingColumns, @@ -42,44 +42,46 @@ public static Storage offset_single_column(Column sourceColumn, int n, FillWi return sourceColumn.getStorage().applyMask(OrderMask.fromArray(rowOrderMask)); } - private static int[] calculate_ungrouped_unordered_mask(int size, int n, FillWith fillWith) { - return IntStream.range(0, size).map(i -> calculate_row_offset(i, n, fillWith, size)).toArray(); + private static int[] calculate_ungrouped_unordered_mask(int numRows, int n, FillWith fillWith) { + return IntStream.range(0, numRows) + .map(i -> calculate_row_offset(i, n, fillWith, numRows)) + .toArray(); } - private static int calculate_row_offset(int i, int n, FillWith fillWith, int size) { - int result = i + n; + private static int calculate_row_offset(int rowIndex, int n, FillWith fillWith, int numRows) { + int result = rowIndex + n; if (result < 0) { return switch (fillWith) { case NOTHING -> Storage.NOT_FOUND_INDEX; case CLOSEST_VALUE -> 0; - case WRAP_AROUND -> (result % size) == 0 ? 0 : (result % size) + size; + case WRAP_AROUND -> (result % numRows) == 0 ? 0 : (result % numRows) + numRows; }; - } else if (result >= size) { + } else if (result >= numRows) { return switch (fillWith) { case NOTHING -> Storage.NOT_FOUND_INDEX; - case CLOSEST_VALUE -> size - 1; - case WRAP_AROUND -> result % size; + case CLOSEST_VALUE -> numRows - 1; + case WRAP_AROUND -> result % numRows; }; } return result; } private static int[] calculate_grouped_ordered_mask( - Column[] sourceColumns, + int numRows, int n, FillWith fillWith, Column[] groupingColumns, Column[] orderingColumns, int[] directions, ProblemAggregator problemAggregator) { - var offsetRowVisitorFactory = new OffsetRowVisitorFactory(sourceColumns[0], n, fillWith); + var offsetRowVisitorFactory = new OffsetRowVisitorFactory(numRows, n, fillWith); GroupingOrderingVisitor.visit( groupingColumns, orderingColumns, directions, problemAggregator, offsetRowVisitorFactory, - sourceColumns[0].getSize()); + numRows); return offsetRowVisitorFactory.rowOrderMask; } @@ -89,8 +91,8 @@ private static class OffsetRowVisitorFactory implements RowVisitorFactory { int n; FillWith fillWith; - OffsetRowVisitorFactory(Column sourceColumn, int n, FillWith fillWith) { - rowOrderMask = new int[sourceColumn.getSize()]; + OffsetRowVisitorFactory(int numRows, int n, FillWith fillWith) { + rowOrderMask = new int[numRows]; this.n = n; this.fillWith = fillWith; } From 47bd37311b24d56d88aec7862df1c4a446bf6b65 Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Wed, 22 Jan 2025 11:11:06 +0000 Subject: [PATCH 6/9] Different approach --- .../expressions/ExpressionVisitorImpl.java | 62 +++---------------- 1 file changed, 10 insertions(+), 52 deletions(-) diff --git a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java index 0ca9b5ff6513..7a51aa1cdf98 100644 --- a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java +++ b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java @@ -10,6 +10,7 @@ import java.util.Set; import java.util.function.Function; import java.util.regex.Pattern; + import org.antlr.v4.runtime.BaseErrorListener; import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; @@ -119,9 +120,6 @@ private Value wrapAsColumn(Value value) { } private Value executeMethod(String name, Value... args) { - var ret = tryDirectNumericCalculation(name, args); - if (ret != null) return ret; - Value method = getMethod.apply(name); if (!method.canExecute()) { throw new UnsupportedOperationException(name); @@ -151,54 +149,6 @@ private Value executeMethod(String name, Value... args) { } } - private Value tryDirectNumericCalculation(String name, Value... args) { - // Check for arithmetic operations - if (args.length == 2 && args[0].isNumber() && args[1].isNumber()) { - if (isInt(args[0]) && isInt(args[1])) { - switch (name) { - case "+" -> { - return Value.asValue(args[0].asInt() + args[1].asInt()); - } - case "-" -> { - return Value.asValue(args[0].asInt() - args[1].asInt()); - } - case "*" -> { - return Value.asValue(args[0].asInt() * args[1].asInt()); - } - case "/" -> { - return Value.asValue(args[0].asInt() / args[1].asInt()); - } - default -> { - return null; - } - } - } else { - switch (name) { - case "+" -> { - return Value.asValue(args[0].asDouble() + args[1].asDouble()); - } - case "-" -> { - return Value.asValue(args[0].asDouble() - args[1].asDouble()); - } - case "*" -> { - return Value.asValue(args[0].asDouble() * args[1].asDouble()); - } - case "/" -> { - return Value.asValue(args[0].asDouble() / args[1].asDouble()); - } - default -> { - return null; - } - } - } - } - return null; - } - - private Boolean isInt(Value v) { - return v.isNumber() && v.asDouble() == v.asInt(); - } - @Override public Value visitProg(ExpressionParser.ProgContext ctx) { Value base = visit(ctx.expr()); @@ -276,7 +226,15 @@ public Value visitUnaryNot(ExpressionParser.UnaryNotContext ctx) { @Override public Value visitUnaryMinus(ExpressionParser.UnaryMinusContext ctx) { - return executeMethod("*", visit(ctx.expr()), Value.asValue(-1)); + var v = visit(ctx.expr()); + if (v.isNumber()) { + if (v.asDouble() == v.asLong()) { + return Value.asValue(-1 * v.asLong()); + } else { + return Value.asValue(-1 * v.asDouble()); + } + } + return executeMethod("*", v, Value.asValue(-1)); } @Override From 8a55c14482f25e3e3af272e50128e4cfb7094967 Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Wed, 22 Jan 2025 11:11:22 +0000 Subject: [PATCH 7/9] fmt --- .../java/org/enso/table/expressions/ExpressionVisitorImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java index 7a51aa1cdf98..3cacc7064b3a 100644 --- a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java +++ b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java @@ -10,7 +10,6 @@ import java.util.Set; import java.util.function.Function; import java.util.regex.Pattern; - import org.antlr.v4.runtime.BaseErrorListener; import org.antlr.v4.runtime.CharStreams; import org.antlr.v4.runtime.CommonTokenStream; From f06fc8481fb6c8682aeb135406651df98bb2429b Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Wed, 22 Jan 2025 11:44:57 +0000 Subject: [PATCH 8/9] Code review chages --- .../org/enso/table/expressions/ExpressionVisitorImpl.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java index 3cacc7064b3a..baa06d599c88 100644 --- a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java +++ b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java @@ -226,12 +226,8 @@ public Value visitUnaryNot(ExpressionParser.UnaryNotContext ctx) { @Override public Value visitUnaryMinus(ExpressionParser.UnaryMinusContext ctx) { var v = visit(ctx.expr()); - if (v.isNumber()) { - if (v.asDouble() == v.asLong()) { - return Value.asValue(-1 * v.asLong()); - } else { - return Value.asValue(-1 * v.asDouble()); - } + if (v.isNumber() && v.asDouble() == v.asLong()) { + return Value.asValue(Math.negateExact(v.asLong())); } return executeMethod("*", v, Value.asValue(-1)); } From 23df93ec8d9db133eaf134f18065fe7edaac0434 Mon Sep 17 00:00:00 2001 From: Adam Riley Date: Wed, 22 Jan 2025 13:15:55 +0000 Subject: [PATCH 9/9] Fix --- .../java/org/enso/table/expressions/ExpressionVisitorImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java index baa06d599c88..b3bb37b81bc8 100644 --- a/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java +++ b/std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java @@ -226,7 +226,7 @@ public Value visitUnaryNot(ExpressionParser.UnaryNotContext ctx) { @Override public Value visitUnaryMinus(ExpressionParser.UnaryMinusContext ctx) { var v = visit(ctx.expr()); - if (v.isNumber() && v.asDouble() == v.asLong()) { + if (v.isNumber() && v.fitsInLong()) { return Value.asValue(Math.negateExact(v.asLong())); } return executeMethod("*", v, Value.asValue(-1));