Add `Is_Empty`, `Not_Empty`, `Like` and `Not_Like` to `Filter_Condition` #3775

radeusgd · 2022-10-06T18:46:59Z

Pull Request Description

Implements https://www.pivotaltracker.com/story/show/183389890

Important Notes

Checklist

Ensure that the SQL-like -> Regex conversion happens only once in the in-memory vectorized operation when comparing with a scalar (currently it is recomputed on each element which is very wasteful).
The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

hubertp · 2022-10-07T10:02:59Z

build.sbt

@@ -2039,7 +2039,7 @@ buildEngineDistribution := {
  log.info(s"Engine package created at $root")
 }

-val stdBitsProjects = List("Base", "Database", "Google_Api", "Image", "Table")
+val stdBitsProjects = List("Base", "Database", "Google_Api", "Image", "Table", "All")


Sadly it is not possible to skip the suffix so buildStdLibAll looks good!

hubertp · 2022-10-07T10:05:52Z

distribution/lib/Standard/Database/0.0.0-dev/src/Data/Column.enso

+       Returns a column with boolean values indicating whether values of this
+       column fit between the lower and upper bounds (both ends inclusive).
+    between : (Column | Any) -> (Column | Any) -> Column
+    between self lower upper =


suggestion: maybe we could think of adding a parameter if it shouldn't be inclusive by default

The SQL BETWEEN this is more or less supposed to emulate is always inclusive. So if we added an option, we'd have to make it unsupported on Database backend or emulate it using regular comparison operators.

But we already can always just apply two filters: Greater+Less to get an exclusive between.

So I'm not sure if it is necessary to add it, I guess it's mostly a question to @jdunkerley who designed the original set of Filter_Conditions.

was aiming to keep BETWEEN simple and just match SQLs behavior with a view that is what our users would expect.

jdunkerley

Looks good.

One thing to check on the like conversion to regex to ensure it matches only the whole string, possible it does and I read the code wrongly.

jdunkerley · 2022-10-07T22:23:50Z

std-bits/base/src/main/java/org/enso/base/Regex_Utils.java

+   * <p>Special regex characters present in the input pattern are quoted to match them literally
+   * according to the SQL-like format.
+   */
+  public static String sql_like_pattern_to_regex(String sql_pattern) {


think the pattern needs to include ^ and $ to indicate the start and end of the value.

hello_ ==> ^hello_$ as SQL like patterns mean that.

As explained below, I already test that the LIKE matches only the whole string.

I prefer to use the specific 'match whole string' regex function and keep the expression itself free of ^$ because they can have different meaning (sometimes they can also mean match beginning/end of line which may give us slightly different semantics.

jdunkerley · 2022-10-07T22:25:32Z

distribution/lib/Standard/Database/0.0.0-dev/src/Data/Column.enso

+       Returns a column with boolean values indicating whether values of this
+       column fit between the lower and upper bounds (both ends inclusive).
+    between : (Column | Any) -> (Column | Any) -> Column
+    between self lower upper =


was aiming to keep BETWEEN simple and just match SQLs behavior with a view that is what our users would expect.

radeusgd · 2022-10-10T08:06:45Z

test/Tests/src/Data/Vector_Spec.enso

@@ -165,9 +168,21 @@ spec = Test.group "Vectors" <|
        txtvec.filter (Filter_Condition.Between "b" "c") . should_equal ["bbb", "baaa"]
        Test.expect_panic_with (txtvec.filter (Filter_Condition.Starts_With 42)) Unsupported_Argument_Types_Data

+        ["", Nothing, " ", "a"].filter (Filter_Condition.Is_Empty) . should_equal ["", Nothing]
+        ["", Nothing, " ", "a"].filter (Filter_Condition.Not_Empty) . should_equal [" ", "a"]
+        ["abab", "aaabaaaa", "ba"].filter (Filter_Condition.Like "ba") . should_equal ["ba"]


@jdunkerley this is the test that verifies that our LIKE matches the whole string if no wildcards are added. We don't need ^$ because we do the regex matches check which itself is meant to check if the whole text matches the pattern. We'd need ^$ if we used something like search instead

great - thanks just wanted to check.

…183389890

Base automatically changed from wip/radeusgd/base-types-filter-condition-183389901 to develop October 7, 2022 04:02

radeusgd force-pushed the wip/radeusgd/filter-condition-empty-like-183389890 branch from a4f9ae9 to 3c16613 Compare October 7, 2022 08:40

radeusgd marked this pull request as ready for review October 7, 2022 09:45

radeusgd requested review from jdunkerley, 4e6, JaroslavTulach and hubertp as code owners October 7, 2022 09:45

radeusgd force-pushed the wip/radeusgd/filter-condition-empty-like-183389890 branch from 54111d3 to 62ab69e Compare October 7, 2022 09:52

hubertp approved these changes Oct 7, 2022

View reviewed changes

radeusgd force-pushed the wip/radeusgd/filter-condition-empty-like-183389890 branch 2 times, most recently from c6010aa to 38b9b21 Compare October 7, 2022 11:45

jdunkerley approved these changes Oct 7, 2022

View reviewed changes

radeusgd self-assigned this Oct 10, 2022

radeusgd commented Oct 10, 2022

View reviewed changes

radeusgd force-pushed the wip/radeusgd/filter-condition-empty-like-183389890 branch from 4bb1a02 to fbce8ad Compare October 10, 2022 11:23

radeusgd added 15 commits October 10, 2022 18:12

Preliminary implementation for Base types

9906bbf

Fix like, move to Java in anticipation of vectorized ops

b082949

tests, initial impl of like, empty, update between

6ea5061

Add empty, like tests for Table

6ab3f08

shortcut to rebuild all stdlib quickly

9d5e271

update previously skipped packages too

eeb02d3

Add is_empty and like to Database

28f0542

fix for postgres

d3ec514

Ensure SQL BETWEEN is used for the Between filter in Database

a28b63e

fix after rebase

72458e0

Precompile pattern only once

777fa28

changelog

50b7828

add a test

1bbece0

formatting

4a184a5

Fix after rebase

593ef02

radeusgd added 4 commits October 10, 2022 18:12

Add test for newlines in wildcards

6711066

More tests, fix regex in Table

78906f0

Fix Like in Base types

07be26a

Add some Unicode tests, some of them pending due to a bug

0cbf8d1

radeusgd force-pushed the wip/radeusgd/filter-condition-empty-like-183389890 branch from 6c45b90 to 0cbf8d1 Compare October 10, 2022 16:13

radeusgd added the CI: Ready to merge This PR is eligible for automatic merge label Oct 10, 2022

mergify bot and others added 6 commits October 10, 2022 16:37

Merge branch 'develop' into wip/radeusgd/filter-condition-empty-like-…

7388c49

…183389890

Add comments on the known bug, make the fix better

f0a82e0

Merge branch 'develop' into wip/radeusgd/filter-condition-empty-like-…

7a91fd1

…183389890

Merge branch 'develop' into wip/radeusgd/filter-condition-empty-like-…

18b9bdd

…183389890

Merge branch 'develop' into wip/radeusgd/filter-condition-empty-like-…

fa3e2c1

…183389890

Merge branch 'develop' into wip/radeusgd/filter-condition-empty-like-…

4264689

…183389890

mergify bot merged commit 592a851 into develop Oct 10, 2022

mergify bot deleted the wip/radeusgd/filter-condition-empty-like-183389890 branch October 10, 2022 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `Is_Empty`, `Not_Empty`, `Like` and `Not_Like` to `Filter_Condition` #3775

Add `Is_Empty`, `Not_Empty`, `Like` and `Not_Like` to `Filter_Condition` #3775

radeusgd commented Oct 6, 2022 •

edited

Loading

hubertp Oct 7, 2022

hubertp Oct 7, 2022

radeusgd Oct 7, 2022

jdunkerley Oct 7, 2022

jdunkerley left a comment

jdunkerley Oct 7, 2022

radeusgd Oct 10, 2022

jdunkerley Oct 7, 2022

radeusgd Oct 10, 2022

jdunkerley Oct 10, 2022

Add Is_Empty, Not_Empty, Like and Not_Like to Filter_Condition #3775

Add Is_Empty, Not_Empty, Like and Not_Like to Filter_Condition #3775

Conversation

radeusgd commented Oct 6, 2022 • edited Loading

Pull Request Description

Important Notes

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdunkerley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add `Is_Empty`, `Not_Empty`, `Like` and `Not_Like` to `Filter_Condition` #3775

Add `Is_Empty`, `Not_Empty`, `Like` and `Not_Like` to `Filter_Condition` #3775

radeusgd commented Oct 6, 2022 •

edited

Loading