Skip to content

Commit

Permalink
Attach a warning when Nothing is used in a Filter_Condition (#8865)
Browse files Browse the repository at this point in the history
Attach a warning when Nothing is used as a value in a comparison or `is_in` Filter_Condition.
  • Loading branch information
GregoryTravis authored Jan 27, 2024
1 parent 9a37357 commit 644c9af
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 15 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,8 @@
- [Added text_left and text_right to Column][8691]
- [Implement relational `NULL` semantics for `Nothing` for in-memory Column
operations.][5156]
- [Attach a warning when Nothing is used as a value in a comparison or `is_in`
`Filter_Condition`.][8865]

[debug-shortcuts]:
https://github.com/enso-org/enso/blob/develop/app/gui/docs/product/shortcuts.md#debug
Expand Down Expand Up @@ -869,6 +871,7 @@
[8606]: https://github.com/enso-org/enso/pull/8606
[8627]: https://github.com/enso-org/enso/pull/8627
[8691]: https://github.com/enso-org/enso/pull/8691
[8865]: https://github.com/enso-org/enso/pull/8865

#### Enso Compiler

Expand Down
11 changes: 11 additions & 0 deletions distribution/lib/Standard/Table/0.0.0-dev/src/Errors.enso
Original file line number Diff line number Diff line change
Expand Up @@ -793,3 +793,14 @@ type Not_All_Rows_Downloaded
to_display_text : Text
to_display_text self =
"The query has returned more than the maximum of "+self.max_rows.to_text+" rows, so some rows have been dropped from the result. If you want to get the full result, change the row limit."

type Nothing_Value_In_Filter_Condition
## PRIVATE
Indicates that a Nothing/NULL value was used as a parameter to a
`Filter_Condition` comparison or `is_in` operation.
Error (filter_condition:Filter_Condition)

## PRIVATE
to_display_text : Text
to_display_text self =
"Using `Nothing` as an argument to a `"+self.filter_condition.to_text+"` cannot match anything."
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Standard.Base.Errors.Common.Type_Error
from Standard.Base.Data.Filter_Condition.Filter_Condition import all

import project.Data.Type.Value_Type.Value_Type
from project.Errors import Nothing_Value_In_Filter_Condition

## PRIVATE
A helper function gathering the common logic that generates a boolean mask
Expand All @@ -15,15 +16,17 @@ import project.Data.Type.Value_Type.Value_Type
make_filter_column source_column filter_condition on_problems = case filter_condition of
# Equality
Equal value ->
Warning.with_suspended source_column source_column->
Warning.with_suspended value value->
on_problems.escalate_warnings <|
source_column == value
warn_on_nothing_in_comparison filter_condition value <|
Warning.with_suspended source_column source_column->
Warning.with_suspended value value->
on_problems.escalate_warnings <|
source_column == value
Not_Equal value ->
Warning.with_suspended source_column source_column->
Warning.with_suspended value value->
on_problems.escalate_warnings <|
source_column != value
warn_on_nothing_in_comparison filter_condition value <|
Warning.with_suspended source_column source_column->
Warning.with_suspended value value->
on_problems.escalate_warnings <|
source_column != value
# Nothing
Is_Nothing -> source_column.is_nothing
Not_Nothing -> source_column.is_nothing.not
Expand All @@ -32,11 +35,22 @@ make_filter_column source_column filter_condition on_problems = case filter_cond
Value_Type.expect_boolean source_column <| source_column
Is_False -> source_column.not
# Comparisons
Less value -> (source_column < value)
Equal_Or_Less value -> (source_column <= value)
Equal_Or_Greater value -> (source_column >= value)
Greater value -> (source_column > value)
Between lower upper -> source_column.between lower upper
Less value ->
warn_on_nothing_in_comparison filter_condition value <|
source_column < value
Equal_Or_Less value ->
warn_on_nothing_in_comparison filter_condition value <|
source_column <= value
Equal_Or_Greater value ->
warn_on_nothing_in_comparison filter_condition value <|
source_column >= value
Greater value ->
warn_on_nothing_in_comparison filter_condition value <|
source_column > value
Between lower upper ->
warn_on_nothing_in_comparison filter_condition lower <|
warn_on_nothing_in_comparison filter_condition upper <|
source_column.between lower upper
# Text
Equal_Ignore_Case value locale ->
source_column.equals_ignore_case value locale
Expand Down Expand Up @@ -67,5 +81,24 @@ make_filter_column source_column filter_condition on_problems = case filter_cond
if is_nan_column.is_error then is_infinite_column.not else
(is_infinite_column || is_nan_column).not
# Vector
Is_In values -> source_column.is_in values
Not_In values -> source_column.is_in values . not
Is_In values ->
warn_on_nothing_in_comparison_vector filter_condition values <|
source_column.is_in values
Not_In values ->
warn_on_nothing_in_comparison_vector filter_condition values <|
source_column.is_in values . not

## Attach a warning if the provided value is `Nothing`.
warn_on_nothing_in_comparison : Filter_Condition -> Any -> Any -> Any
warn_on_nothing_in_comparison filter_condition value ~action =
case value of
Nothing -> Warning.attach (Nothing_Value_In_Filter_Condition.Error filter_condition) action
_ -> action

## Attach a warning if the provided value is a `Vector` that contains `Nothing`.
warn_on_nothing_in_comparison_vector : Filter_Condition -> Vector Any -> Any -> Any
warn_on_nothing_in_comparison_vector filter_condition values ~action =
case values of
_ : Vector ->
values.fold action (flip (warn_on_nothing_in_comparison filter_condition))
_ -> action
22 changes: 22 additions & 0 deletions test/Table_Tests/src/Common_Table_Operations/Filter_Spec.enso
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,28 @@ add_specs suite_builder setup =
err2 = t2.filter "Y" (Filter_Condition.Not_Equal 5) on_problems=Problem_Behavior.Report_Error
err2.should_fail_with Floating_Point_Equality

group_builder.specify "should attach a warning when Nothing is used as a value in a comparison or `is_in` `Filter_Condition`" <|
t = table_builder [["x", [1, 2, 3]]]
fcs = [Filter_Condition.Equal Nothing, Filter_Condition.Not_Equal Nothing]
+ [Filter_Condition.Less Nothing, Filter_Condition.Equal_Or_Less Nothing]
+ [Filter_Condition.Equal_Or_Greater Nothing, Filter_Condition.Greater Nothing]
+ [Filter_Condition.Between Nothing Nothing , Filter_Condition.Is_In [Nothing]]
+ [Filter_Condition.Not_In [Nothing]]
+ [Filter_Condition.Is_In [1, Nothing, 2]]
fcs.map fc->
Test.with_clue fc.to_text <|
Problems.expect_warning Nothing_Value_In_Filter_Condition (t.filter "x" fc . at "x")

group_builder.specify "should not attach a warning when comparing with a column containing Nothing in a comparison `Filter_Condition`" <|
t = table_builder [["x", [1, 2, 3]], ["y", [1, Nothing, 2]]]
Problems.assume_no_problems (t.filter "x" (Filter_Condition.Equal (t.at "y")))

group_builder.specify "should not attach a warning when Nothing is not used as a value in a comparison or `is_in` `Filter_Condition`" <|
t = table_builder [["x", [1, 2, 3]], ["y", [1, Nothing, 2]]]
Problems.assume_no_problems (t.filter "x" (Filter_Condition.Equal 12))
Problems.assume_no_problems (t.filter "x" (Filter_Condition.Equal [Nothing, Nothing]))
Problems.assume_no_problems (t.filter "x" (Filter_Condition.Is_In [[Nothing, Nothing]]))

suite_builder.group prefix+"Table.filter_by_expression" group_builder->
data = Data.setup create_connection_fn

Expand Down

0 comments on commit 644c9af

Please sign in to comment.