Introduce array_except function
#8135
Conversation
github-actions
bot
added
logical-expr
| @@ -1342,6 +1347,10 @@ pub fn parse_expr( | |||
| parse_expr(&args[0], registry)?, | |||
| parse_expr(&args[1], registry)?, | |||
| )), | |||
| ScalarFunction::ArrayIntersect => Ok(array_intersect( | |||
There was a problem hiding this comment.
Do we need to enhance the check during CI? maybe this is a little bug?
| @@ -2454,6 +2454,113 @@ select list_has_all(make_array(1,2,3), make_array(4,5,6)), | |||
| ---- | |||
| false true false true | |||
|
|
|||
| ## array_except | |||
|
|
|||
| statement ok | |||
There was a problem hiding this comment.
Unlike other tests, I try a new style of test, keep creating table and drop table close together, hope this is easier for review.
There was a problem hiding this comment.
this is choice of style, imho no-table tests are easier to read.
query ?
select array_expect([], []), array_except(....)
----
[], ...
There was a problem hiding this comment.
Well, we still need table to test multi-rows cases. The new style I mean is not create table in the beginning of this large file and drop table at the end of the file, but when we are done we drop it.
There was a problem hiding this comment.
the multirow still possible to do without creating a table
select ... from (select 1 a, 'asdf' b union all select 2 b, 'zxcv' b)
thats the matter of style of course.
There was a problem hiding this comment.
We cant differentiate empty array and null currently. Both of them are null type So we can fix them in another PR
There was a problem hiding this comment.
I also prefer the new style (create/query/drop table statements are organized more tightly). This will make tests more clear.
jayzhan211
force-pushed
the
array-except
branch
from
3846b70
to
6a291c2
Compare
There was a problem hiding this comment.
Thanks @jayzhan211 please also add tests with empty arrays
And since we have introduced the new built in function we need to get it documented in
scalar_functions.md and expressions.md
jayzhan211
force-pushed
the
array-except
branch
from
49108ae
to
edf8618
Compare
jayzhan211
force-pushed
the
array-except
branch
from
cd0b846
to
b990b47
Compare
|
LGTM! |
There was a problem hiding this comment.
lgtm thanks @jayzhan211
Please create a followup ticket to differentiate empty array and null, that sounds important
|
There appears to be a non trivial number of conflicts in this PR now |
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
jayzhan211
force-pushed
the
array-except
branch
from
b990b47
to
218a299
Compare
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
|
I took the liberty of merging up from main to resolve a merge conflict |
Which issue does this PR close?
Closes #6979
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?