enso-org / enso

Enso Analytics is a self-service data prep and analysis platform designed for data teams.
https://ensoanalytics.com
Apache License 2.0
7.39k stars 323 forks source link

Fix Sorting in Snowflake #11636

Closed AdRiley closed 14 hours ago

AdRiley commented 6 days ago

Pull Request Description

Fixes #10835

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

radeusgd commented 6 days ago

Can we add a test for case insensitive ordering ? As mentioned in the ticket we currently seem to be lacking one.

It'd be good to have one so that we avoid regressions, and ensure that we correctly handle this edge case when adding future dialects.

I was originally planning to add a test case as part of this ticket, if we want to add such test in separate PR I'm not against that, but let's then create a ticket to keep track of it - otherwise I'd be worried we may forget to do it.

AdRiley commented 13 hours ago

I think there is a case insensitive test

    group_builder.specify "should support natural and case insensitive ordering at the same time" <|
        t1 = data.table.sort [..Name "psi"] text_ordering=(..Case_Insensitive sort_digits_as_numbers=True)
        case setup.flagged ..Supports_Sort_Digits_As_Numbers of
            True -> t1.at "psi" . to_vector . should_equal [Nothing, "c01", "C2", "c10"]
            False -> t1.should_fail_with (Unsupported_Database_Operation.Error "sort_digits_as_numbers")

        t2 = data.table.sort [..Name "psi"] text_ordering=(..Default sort_digits_as_numbers=True)
        case setup.flagged ..Supports_Sort_Digits_As_Numbers of
            True -> t2.at "psi" . to_vector . should_equal [Nothing, "C2", "c01", "c10"]
            False -> t2.should_fail_with (Unsupported_Database_Operation.Error "sort_digits_as_numbers")

        t3 = data.table.sort [..Name "psi"] text_ordering=(..Case_Insensitive)
        t3.at "psi" . to_vector . should_equal [Nothing, "c01", "c10", "C2"]

        t4 = data.table.sort [..Name "psi"]
        case setup.flagged ..Case_Insensitive_Ordering of
            True -> t4.at "psi" . to_vector . should_equal [Nothing, "c01", "c10", "C2"]
            False -> t4.at "psi" . to_vector . should_equal [Nothing, "C2", "c01", "c10"]