[Feature]Support array_contains_seq functions like trino contains_sequence and ck hasSubstr function #33929

leoyy0316 · 2023-10-30T08:57:53Z

Fixes #issue
ck:https://clickhouse.com/docs/en/sql-reference/functions/array-functions#hassubstr
trino:https://trino.io/docs/current/functions/array.html?highlight=contains_sequence#contains_sequence

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

be/src/exprs/array_functions.cpp

LiShuMing · 2023-10-30T10:50:28Z

be/src/exprs/array_functions.cpp

-                        elements, element_offsets_ptr[i], element_offsets_ptr[i + 1], targets, target_offsets_ptr[0],
-                        target_offsets_ptr[1], null_map_elements, null_map_targets);
-            } else {
+            if (ContainsSeq) {


Suggested change

if (ContainsSeq) {

if constexpr (ContainsSeq) {

test/sql/test_array_fn/R/test_array_fn

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

LiShuMing

LGTM

fzhedu · 2023-11-01T08:37:05Z

test/sql/test_array_fn/T/test_array_fn

@@ -825,3 +825,19 @@ select array_intersect([row(1,2,3), row(3,4,5)], [row(3,4,5)]);
 select array_contains_all([row(1,2,3), row(3,4,5)], [row(3,4,5)]);
 select array_filter([row(1,2,3), row(3,4,5), row(4,5,6)], [0,1,0]);
 select cardinality([row(1,2,3), row(3,4,5)]);
+
+select array_contains_seq([1,2,3,4], [2,3]);


what if SELECT array_contains_seq([1, 2, NULL, 3, 4], ['a'])

SELECT array_contains_seq([1, 2, NULL, 3, 4], [2,3]);
SELECT array_contains_seq([1, 2, NULL, 3, 4], null);
SELECT array_contains_seq(null, [2,3])
SELECT array_contains_seq([1, 2, NULL, 3, 4], [null,null])
SELECT array_contains_seq([1, 2, NULL], [null,2])
SELECT array_contains_seq(null, null)

we prefer keep the same behavior with trino

what if SELECT array_contains_seq([1, 2, NULL, 3, 4], ['a'])

MySQL [(none)]> select array_contains_all([1, 2, NULL, 3, 4], ['a']);
+-----------------------------------------------+
| array_contains_all([1, 2, NULL, 3, 4], ['a']) |
+-----------------------------------------------+
| 0 |
+-----------------------------------------------+
1 row in set (0.18 sec)

SELECT array_contains_seq([1, 2, NULL, 3, 4], [2,3]); SELECT array_contains_seq([1, 2, NULL, 3, 4], null); SELECT array_contains_seq(null, [2,3]) SELECT array_contains_seq([1, 2, NULL, 3, 4], [null,null]) SELECT array_contains_seq([1, 2, NULL], [null,2]) SELECT array_contains_seq(null, null)

all add to sqltest

here are all cases that just consider single array, please add some cases for array columns from a table, which would get some bugs. like following cases

CREATE TABLE array_test ( pk bigint not null , i_0 Array<BigInt>, i_1 Array<BigInt>, ai_0 Array<Array<BigInt>>, ai_1 Array<Array<BigInt>> ) ENGINE=OLAP DUPLICATE KEY(`pk`) DISTRIBUTED BY HASH(`pk`) BUCKETS 3 PROPERTIES ( "replication_num" = "1", "in_memory" = "false" ); insert into array_test values (1,[null,1],[null],[null],[[]]), (2,[null],[1,3,4,5,1,2],[[null,1],null],[[1,null]]), (3,[],[],[[],null,[1,1]],[],[1,1]), (4,null,[],[[1,1]],[[1,1],null), (5,[4,4,4],[4,null],[null],[null]), (6,[1,1,2,1,1,2,3,3],[1,2,3],[[1]],[[1],[2],null,[null]]); select array_contains_seq(i_0,i_1),array_contains_seq(i_0,i_0),array_contains_seq(i_1,i_0), array_contains_seq(ai_0, ai_1),array_contains_seq(ai_1, ai_1),array_contains_seq(ai_1, ai_0),array_contains_seq(ai_0, i_1),array_contains_seq(ai_0,null),,array_contains_seq(ai_0, [1,2]) from array_test;

add this case to the Test file

fzhedu · 2023-11-01T08:38:13Z

docs/sql-reference/sql-functions/array-functions/array_contains_seq.md

+## Return value
+
+Returns a value of the BOOLEAN type.
+
+1 is returned if `arr2` is a subset of `arr1`. Otherwise, 0 is returned.
+
+## Examples


append more special msg like in https://clickhouse.com/docs/en/sql-reference/functions/array-functions#hassubstr, such as null, no supertype etc.

we prefer keeping the same behavior with trino

fzhedu · 2023-11-01T08:46:30Z

be/src/exprs/array_functions.cpp

+        bool found = false;
+        size_t i = target_start;
+        size_t j = element_start;
+        while (i < target_end && j < element_end) {
+            if constexpr (std::is_same_v<ArrayColumn, ElementColumn> || std::is_same_v<MapColumn, ElementColumn> ||
+                          std::is_same_v<StructColumn, ElementColumn> || std::is_same_v<JsonColumn, ElementColumn>) {
+                found = (elements.equals(j, targets, i) == 1);
+            } else {
+                auto elements_ptr = (const ValueType*)(elements.raw_data());
+                auto targets_ptr = (const ValueType*)(targets.raw_data());
+                found = (elements_ptr[j] == targets_ptr[i]);
+            }
+            if (found) {
+                i++;
+                j++;
+            } else {
+                i = 0;
+                j++;
+            }
+        }
+        return i == target_end;


seems not process nullable cases?

seems not process nullable cases?

done

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu · 2023-11-03T02:50:46Z

be/src/exprs/array_functions.cpp

+        if (element_end < target_end) {
+            return false;
+        }


this is not true, as your tests case just consider single array cases. Taking an array column as example,
elem: [null][null] (its offsets are [0,0,0]
target: [1,2,3,4],[null] (its offsets are [0,4,4]

array_contains_seq(elem, target) should return false, true. but your answer sould be wrong to false, false.

if(element_end-element_start > target_end-target_start)

fzhedu · 2023-11-03T02:51:19Z

be/src/exprs/array_functions.cpp

+                } else if (null_target || null_element) {
+                    found = false;
+                } else {
+                    found = (elements_ptr[j] == targets_ptr[i]);


if constexpr (std::is_same_v<ArrayColumn, ElementColumn> || std::is_same_v<MapColumn, ElementColumn> || std::is_same_v<StructColumn, ElementColumn> || std::is_same_v<JsonColumn, ElementColumn>) { found = (elements.equals(j, targets, i) == 1); } else

should move here

fzhedu

please refer to be/test/exprs/array_functions_test.cpp to write some unit tests.
array_contains_empty_array
array_contains_all

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu · 2023-11-06T02:09:53Z

be/src/exprs/array_functions.cpp

+            int k = j;
+            int l = 0;


remove k,l;
i = target_start;

fzhedu · 2023-11-06T02:10:00Z

be/src/exprs/array_functions.cpp

+            if (l == target_end) {
+                return true;
+            }
+            i = 0;


fzhedu · 2023-11-06T02:10:13Z

be/src/exprs/array_functions.cpp

+                    l++;
+                }


else break;

fzhedu · 2023-11-06T02:10:59Z

be/test/exprs/array_functions_test.cpp

+    // array_contains_seq(["a", "b", NULL], [NULL])       -> 1
+    // array_contains_seq(["a", "b", "c"], ["d"])         -> 0
+    // array_contains_seq(["a", "b", "c"], ["a", "d"])    -> 0
+    // array_contains_all(["a", "b", "c"], ["a", "c"])    -> 1


fzhedu · 2023-11-06T02:19:24Z

be/test/exprs/array_functions_test.cpp

@@ -5467,4 +5467,55 @@ TEST_F(ArrayFunctionsTest, array_match_only_null) {
        ASSERT_TRUE(dest_column->get(0).get_int8());
    }
 }
+// NOLINTNEXTLINE
+TEST_F(ArrayFunctionsTest, array_contains_seq) {


here is the case nullable array1 and nullable array2, pls refer to array_contains_no_null to add some cases from nullable array1, not-nullable array2, not-nullable array1, nullable array2, not_nullable array1, not_nullable array2.

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu · 2023-11-07T07:47:57Z

be/src/exprs/array_functions.cpp

+        bool found = false;
+        size_t i = target_start;
+        size_t j = element_start;
+        while (j < element_end) {


j <= element_end - (target_end - target_start), no need to loop if the left elements are less than target's elements

fzhedu · 2023-11-07T08:02:51Z

be/test/exprs/array_functions_test.cpp

+    {
+        auto array = ColumnHelper::create_column(TYPE_ARRAY_ARRAY_VARCHAR, true);
+        array->append_datum(DatumArray{Datum(DatumArray{"a"}), Datum(DatumArray{"b"}), Datum()});
+        array->append_datum(DatumArray{"a", "d", "c"});


should be [[xxx]]

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu · 2023-11-08T13:40:15Z

be/src/exprs/array_functions.cpp

+            if ((element_end - j) < (target_end - target_start)) {
+                return false;
+            }


if put here, it should be <=; prefer put this condition to the while() at L:780

fzhedu · 2023-11-08T13:40:56Z

test/sql/test_array_fn/T/test_array_fn

+select array_contains_seq(null, [2,3]);
+select array_contains_seq([1, 2, NULL, 3, 4], [null,null]);
+select array_contains_seq([1, 2, NULL], [null,2]);
+select array_contains_seq(null, null);


add these tests to this file

CREATE TABLE array_test ( pk bigint not null , i_0 Array<BigInt>, i_1 Array<BigInt>, ai_0 Array<Array<BigInt>>, ai_1 Array<Array<BigInt>> ) ENGINE=OLAP DUPLICATE KEY(`pk`) DISTRIBUTED BY HASH(`pk`) BUCKETS 3 PROPERTIES ( "replication_num" = "1", "in_memory" = "false" ); insert into array_test values (1,[null,1],[null],[null],[[]]), (2,[null],[1,3,4,5,1,2],[[null,1],null],[[1,null]]), (3,[],[],[[],null,[1,1]],[],[1,1]), (4,null,[],[[1,1]],[[1,1],null), (5,[4,4,4],[4,null],[null],[null]), (6,[1,1,2,1,1,2,3,3],[1,2,3],[[1]],[[1],[2],null,[null]]); select array_contains_seq(i_0,i_1),array_contains_seq(i_0,i_0),array_contains_seq(i_1,i_0), array_contains_seq(ai_0, ai_1),array_contains_seq(ai_1, ai_1),array_contains_seq(ai_1, ai_0),array_contains_seq(ai_0, i_1),array_contains_seq(ai_0,null),,array_contains_seq(ai_0, [1,2]) from array_test;

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

sonarqubecloud · 2023-11-09T02:52:19Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

The version of Java (11.0.21) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

github-actions · 2023-11-09T04:52:52Z

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2023-11-09T04:53:41Z

[BE Incremental Coverage Report]

✅ pass : 48 / 54 (88.89%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	src/exprs/array_functions.cpp	48	54	88.89%	[775, 849, 850, 851, 852, 1042]

support array_contains_seq functions like trino contains_sequence and…

aa015a8

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

leoyy0316 requested a review from a team as a code owner October 30, 2023 08:57

mergify bot assigned leoyy0316 Oct 30, 2023

leoyy0316 added 2 commits October 30, 2023 17:07

support array_contains_seq functions like trino contains_sequence and…

950bd8f

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

e894887

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

LiShuMing reviewed Oct 30, 2023

View reviewed changes

support array_contains_seq functions like trino contains_sequence and…

7bce46a

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

github-actions bot added the documentation Improvements or additions to documentation label Oct 31, 2023

leoyy0316 added 3 commits October 31, 2023 10:42

support array_contains_seq functions like trino contains_sequence and…

4e564b3

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

6fc6a75

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

972e8ed

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

LiShuMing previously approved these changes Nov 1, 2023

View reviewed changes

fzhedu reviewed Nov 1, 2023

View reviewed changes

support array_contains_seq functions like trino contains_sequence and…

c574a79

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

leoyy0316 dismissed LiShuMing’s stale review via 5046ff4 November 2, 2023 04:51

leoyy0316 added 4 commits November 2, 2023 12:52

support array_contains_seq functions like trino contains_sequence and…

5046ff4

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

37bcfa6

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

478c2de

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

1f73ee6

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

LiShuMing previously approved these changes Nov 3, 2023

View reviewed changes

leoyy0316 requested a review from fzhedu November 3, 2023 02:22

Seaven previously approved these changes Nov 3, 2023

View reviewed changes

Seaven enabled auto-merge (squash) November 3, 2023 02:27

fzhedu disabled auto-merge November 3, 2023 02:32

fzhedu reviewed Nov 3, 2023

View reviewed changes

fzhedu requested review from Seaven and LiShuMing November 3, 2023 03:09

fzhedu reviewed Nov 3, 2023

View reviewed changes

support array_contains_seq functions like trino contains_sequence and…

3bdd409

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

leoyy0316 dismissed stale reviews from Seaven and LiShuMing via 3bdd409 November 3, 2023 11:01

leoyy0316 added 2 commits November 3, 2023 19:11

support array_contains_seq functions like trino contains_sequence and…

16d3f0c

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

efead1a

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu reviewed Nov 6, 2023

View reviewed changes

be/src/exprs/array_functions.cpp Outdated

Comment on lines 781 to 782

int k = j;

int l = 0;

Copy link

Contributor

fzhedu Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove k,l;
i = target_start;

fzhedu reviewed Nov 6, 2023

View reviewed changes

be/src/exprs/array_functions.cpp Outdated

if (l == target_end) {

return true;

}

i = 0;

Copy link

Contributor

fzhedu Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

fzhedu reviewed Nov 6, 2023

View reviewed changes

be/src/exprs/array_functions.cpp Outdated

Comment on lines 811 to 812

l++;

}

Copy link

Contributor

fzhedu Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else break;

fzhedu reviewed Nov 6, 2023

View reviewed changes

support array_contains_seq functions like trino contains_sequence and…

d7b8ace

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu reviewed Nov 7, 2023

View reviewed changes

leoyy0316 added 2 commits November 8, 2023 10:56

support array_contains_seq functions like trino contains_sequence and…

a3724c8

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

support array_contains_seq functions like trino contains_sequence and…

6ed5253

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu reviewed Nov 8, 2023

View reviewed changes

support array_contains_seq functions like trino contains_sequence and…

2bbaa0d

… ck hasSubstr Signed-off-by: leoyy0316 <571684903@qq.com>

fzhedu approved these changes Nov 9, 2023

View reviewed changes

Seaven approved these changes Nov 9, 2023

View reviewed changes

LiShuMing enabled auto-merge (squash) November 9, 2023 06:24

packy92 approved these changes Nov 9, 2023

View reviewed changes

LiShuMing merged commit 6320dbc into StarRocks:main Nov 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature]Support array_contains_seq functions like trino contains_sequence and ck hasSubstr function #33929

[Feature]Support array_contains_seq functions like trino contains_sequence and ck hasSubstr function #33929

leoyy0316 commented Oct 30, 2023 •

edited by fzhedu

Loading

LiShuMing Oct 30, 2023

LiShuMing left a comment

fzhedu Nov 1, 2023

fzhedu Nov 1, 2023

fzhedu Nov 1, 2023

leoyy0316 Nov 2, 2023

leoyy0316 Nov 2, 2023

fzhedu Nov 3, 2023 •

edited

Loading

fzhedu Nov 6, 2023

fzhedu Nov 1, 2023

fzhedu Nov 1, 2023

fzhedu Nov 1, 2023

leoyy0316 Nov 2, 2023

fzhedu Nov 3, 2023

fzhedu Nov 3, 2023

fzhedu Nov 3, 2023

fzhedu left a comment

fzhedu Nov 6, 2023

fzhedu Nov 6, 2023

fzhedu Nov 6, 2023

fzhedu Nov 6, 2023

fzhedu Nov 6, 2023

fzhedu Nov 7, 2023

fzhedu Nov 7, 2023

fzhedu Nov 8, 2023

fzhedu Nov 8, 2023 •

edited

Loading

sonarqubecloud bot commented Nov 9, 2023

github-actions bot commented Nov 9, 2023

github-actions bot commented Nov 9, 2023

[Feature]Support array_contains_seq functions like trino contains_sequence and ck hasSubstr function #33929

[Feature]Support array_contains_seq functions like trino contains_sequence and ck hasSubstr function #33929

Conversation

leoyy0316 commented Oct 30, 2023 • edited by fzhedu Loading

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Choose a reason for hiding this comment

LiShuMing 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

fzhedu Nov 3, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzhedu 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fzhedu Nov 8, 2023 • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Nov 9, 2023

github-actions bot commented Nov 9, 2023

[FE Incremental Coverage Report]

github-actions bot commented Nov 9, 2023

[BE Incremental Coverage Report]

file detail

leoyy0316 commented Oct 30, 2023 •

edited by fzhedu

Loading

fzhedu Nov 3, 2023 •

edited

Loading

fzhedu Nov 8, 2023 •

edited

Loading