make null sorting consistent in Postgres, fix tests

enso-org · Jun 7, 2022 · dcca186 · dcca186
1 parent 4c8f580
commit dcca186
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 27 deletions.
diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Data/Dialect/Postgres.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Data/Dialect/Postgres.enso
@@ -57,7 +57,8 @@ type Postgresql_Dialect
 
 ## PRIVATE
 make_internal_generator_dialect =
-    text = [here.starts_with, here.contains, here.ends_with, here.agg_shortest, here.agg_longest]+here.concat_ops
+    cases = [["LOWER", Base_Generator.make_function "LOWER"], ["UPPER", Base_Generator.make_function "UPPER"]]
+    text = [here.starts_with, here.contains, here.ends_with, here.agg_shortest, here.agg_longest]+here.concat_ops+cases
     counts = [here.agg_count_is_null, here.agg_count_empty, here.agg_count_not_empty, ["COUNT_DISTINCT", here.agg_count_distinct], ["COUNT_DISTINCT_INCLUDE_NULL", here.agg_count_distinct_include_null]]
 
     stddev_pop = ["STDDEV_POP", Base_Generator.make_function "stddev_pop"]
@@ -221,22 +222,26 @@ make_contains_expr expr substring =
 contains = Base_Generator.lift_binary_op "contains" here.make_contains_expr
 
 ## PRIVATE
-make_order_descriptor internal_column sort_direction text_ordering = case internal_column.sql_type.is_likely_text of
-    True ->
-        ## In the future we can modify this error to suggest using a custom defined collation.
-        if text_ordering.sort_digits_as_numbers then Error.throw (Unsupported_Database_Operation_Error "Natural ordering is currently not supported. You may need to materialize the Table to perform this operation.") else
-            case text_ordering.case_sensitive of
-                Nothing ->
-                    IR.Order_Descriptor internal_column.expression sort_direction collation=Nothing
-                True ->
-                    IR.Order_Descriptor internal_column.expression sort_direction collation="ucs_basic"
-                Case_Insensitive locale -> case Locale.default.java_locale.equals locale.java_locale of
-                    False ->
-                        # TODO do we want to at least check for existing standard ICU collations?
-                        Error.throw (Unsupported_Database_Operation_Error "Case insensitive ordering with custom locale is currently not supported. You may need to materialize the Table to perform this operation.")
+make_order_descriptor internal_column sort_direction text_ordering =
+    nulls = case sort_direction of
+        Sort_Direction.Ascending -> IR.Nulls_First
+        Sort_Direction.Descending -> IR.Nulls_Last
+    case internal_column.sql_type.is_likely_text of
+        True ->
+            ## In the future we can modify this error to suggest using a custom defined collation.
+            if text_ordering.sort_digits_as_numbers then Error.throw (Unsupported_Database_Operation_Error "Natural ordering is currently not supported. You may need to materialize the Table to perform this operation.") else
+                case text_ordering.case_sensitive of
+                    Nothing ->
+                        IR.Order_Descriptor internal_column.expression sort_direction nulls_order=nulls collation=Nothing
                     True ->
-                        upper = IR.Operation "upper" [internal_column.expression]
-                        folded_expression = IR.Operation "lower" [upper]
-                        IR.Order_Descriptor folded_expression sort_direction collation=Nothing
-    False ->
-        IR.Order_Descriptor internal_column.expression sort_direction collation=Nothing
+                        IR.Order_Descriptor internal_column.expression sort_direction nulls_order=nulls collation="ucs_basic"
+                    Case_Insensitive locale -> case Locale.default.java_locale.equals locale.java_locale of
+                        False ->
+                            # TODO do we want to at least check for existing standard ICU collations?
+                            Error.throw (Unsupported_Database_Operation_Error "Case insensitive ordering with custom locale is currently not supported. You may need to materialize the Table to perform this operation.")
+                        True ->
+                            upper = IR.Operation "UPPER" [internal_column.expression]
+                            folded_expression = IR.Operation "LOWER" [upper]
+                            IR.Order_Descriptor folded_expression sort_direction nulls_order=nulls collation=Nothing
+        False ->
+            IR.Order_Descriptor internal_column.expression sort_direction nulls_order=nulls collation=Nothing
diff --git a/distribution/lib/Standard/Database/0.0.0-dev/src/Data/Table.enso b/distribution/lib/Standard/Database/0.0.0-dev/src/Data/Table.enso
@@ -465,6 +465,8 @@ type Table
          - If values do not implement an ordering, an
            `Incomparable_Values_Error`.
 
+       Missing (`Nothing`) values are sorted as less than any other object.
+
        > Example
          Order the table by the column "alpha" in ascending order.
 

diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso
@@ -527,14 +527,15 @@ spec prefix table_builder test_selection pending=Nothing =
             col7 = ["psi", [Nothing, "c01", "c10", "C2"]]
             col8 = ["phi", ["śc", Nothing, 's\u0301b', "śa"]]
             col9 = ["tau", [32.0, 0.5, -0.1, 1.6]]
-            table_builder [col1, col2, col3, col4, col5, col6, col7, col8, col9]
+            col10 = ["rho", ["BB", Nothing, Nothing, "B"]]
+            table_builder [col1, col2, col3, col4, col5, col6, col7, col8, col9, col10]
 
         Test.specify "should work as shown in the doc examples" <|
             t1 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "alpha"])
             t1.at "alpha" . to_vector . should_equal [0, 1, 2, 3]
             t1.at "gamma" . to_vector . should_equal [4, 3, 2, 1]
 
-            t2 = table.order_by (Sort_Column_Selector.By_Index [Sort_Column.Index 1, Sort_Column.Index -7 Sort_Direction.Descending])
+            t2 = table.order_by (Sort_Column_Selector.By_Index [Sort_Column.Index 1, Sort_Column.Index -8 Sort_Direction.Descending])
             t2.at "beta" . to_vector . should_equal ["a", "a", "b", "b"]
             t2.at "gamma" . to_vector . should_equal [3, 1, 4, 2]
             t2.at "alpha" . to_vector . should_equal [1, 3, 0, 2]
@@ -564,13 +565,13 @@ spec prefix table_builder test_selection pending=Nothing =
             Problems.test_problem_handling action problems tester
 
         Test.specify "should correctly handle problems: aliased indices" <|
-            selector = Sort_Column_Selector.By_Index [Sort_Column.Index 1, Sort_Column.Index -8 Sort_Direction.Descending, Sort_Column.Index -7 Sort_Direction.Descending, Sort_Column.Index 2 Sort_Direction.Ascending]
+            selector = Sort_Column_Selector.By_Index [Sort_Column.Index 1, Sort_Column.Index -9 Sort_Direction.Descending, Sort_Column.Index -8 Sort_Direction.Descending, Sort_Column.Index 2 Sort_Direction.Ascending]
             action = table.order_by selector on_problems=_
             tester table =
                 table.at "beta" . to_vector . should_equal ["a", "a", "b", "b"]
                 table.at "gamma" . to_vector . should_equal [3, 1, 4, 2]
                 table.at "alpha" . to_vector . should_equal [1, 3, 0, 2]
-            problems = [Input_Indices_Already_Matched [Sort_Column.Index -8 Sort_Direction.Descending, Sort_Column.Index 2]]
+            problems = [Input_Indices_Already_Matched [Sort_Column.Index -9 Sort_Direction.Descending, Sort_Column.Index 2]]
             Problems.test_problem_handling action problems tester
 
         Test.specify "should correctly handle problems: duplicate names" <|
@@ -679,9 +680,11 @@ spec prefix table_builder test_selection pending=Nothing =
             t1.at "xi" . to_vector . should_equal [Nothing, 0.5, 1.0, 1.5]
             t1.at "alpha" . to_vector . should_equal [1, 0, 3, 2]
 
-            t2 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "psi"])
-            t2.at "psi" . to_vector . should_equal [Nothing, "C2", "c01", "c10"]
-            t2.at "alpha" . to_vector . should_equal [3, 0, 2, 1]
+            t2 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "rho"])
+            t2.at "rho" . to_vector . should_equal [Nothing, Nothing, "B", "BB"]
+
+            t3 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "rho" Sort_Direction.Descending])
+            t3.at "rho" . to_vector . should_equal ["BB", "B", Nothing, Nothing]
 
         Test.specify "should behave as expected with Unicode normalization, depending on the defaults settings" <|
             t1 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "phi"])
@@ -712,6 +715,12 @@ spec prefix table_builder test_selection pending=Nothing =
             t2 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "eta"]) text_ordering=(Text_Ordering case_sensitive=True)
             t2.at "eta" . to_vector . should_equal ["Aleph", "Beta", "alpha", "bądź"]
 
+            t3 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "psi"]) text_ordering=(Text_Ordering case_sensitive=Case_Insensitive)
+            t3.at "psi" . to_vector . should_equal [Nothing, "c01", "c10", "C2"]
+
+            t4 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "psi" Sort_Direction.Descending]) text_ordering=(Text_Ordering case_sensitive=True)
+            t4.at "psi" . to_vector . should_equal ["c10", "c01", "C2", Nothing]
+
         Test.specify "should support natural and case insensitive ordering at the same time" pending=(if (test_selection.natural_ordering.not || test_selection.case_insensitive_ordering.not) then "Natural ordering or case sensitive ordering is not supported.") <|
             t1 = table.order_by (Sort_Column_Selector.By_Name [Sort_Column.Name "psi"]) text_ordering=(Text_Ordering sort_digits_as_numbers=True case_sensitive=Case_Insensitive)
             t1.at "psi" . to_vector . should_equal ["c01", "C2", "c10", Nothing]

diff --git a/test/Table_Tests/src/Database/Postgresql_Spec.enso b/test/Table_Tests/src/Database/Postgresql_Spec.enso
@@ -99,7 +99,7 @@ run_tests connection pending=Nothing =
 
     Common_Spec.spec prefix connection pending=pending
     here.postgres_specific_spec connection pending=pending
-    common_selection = Common_Table_Spec.Test_Selection supports_case_sensitive_columns=True
+    common_selection = Common_Table_Spec.Test_Selection supports_case_sensitive_columns=True order_by_unicode_normalization_by_default=True
     Common_Table_Spec.spec prefix table_builder test_selection=common_selection pending=pending
 
     selection = Aggregate_Spec.Test_Selection first_last_row_order=False aggregation_problems=False