alamb commented on code in PR #14489:
URL: https://github.com/apache/datafusion/pull/14489#discussion_r1942981686
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -1993,6 +1993,28 @@ select array_slice(arrow_cast(make_array(1, 2, 3, 4, 5),
'LargeList(Int64)'), 2,
----
[2, 3, 4] [h, e]
+# array_slice scalar function #24 (with first negative index larger than len)
+query ??
+select array_slice(make_array(1, 2, 3, 4, 5), -2147483648, 1),
list_slice(make_array('h', 'e', 'l', 'l', 'o'), -2147483648, 1);
+----
+[1] [h]
+
+query ??
+select array_slice(arrow_cast(make_array(1, 2, 3, 4, 5), 'LargeList(Int64)'),
-9223372036854775808, 1), list_slice(arrow_cast(make_array('h', 'e', 'l', 'l',
'o'), 'LargeList(Utf8)'), -9223372036854775808, 1);
Review Comment:
```sql
D select array_slice([1, 2, 3, 4, 5], -2147483648, 1)
;
┌─────────────────────────────────────────────────────────────┐
│ array_slice(main.list_value(1, 2, 3, 4, 5), -2147483648, 1) │
│ int32[] │
├─────────────────────────────────────────────────────────────┤
│ [1] │
└─────────────────────────────────────────────────────────────┘
```
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -1941,12 +1941,12 @@ select array_slice(arrow_cast(make_array(1, 2, 3, 4,
5), 'LargeList(Int64)'), -4
query ??
select array_slice(make_array(1, 2, 3, 4, 5), -7, -2),
array_slice(make_array('h', 'e', 'l', 'l', 'o'), -7, -3);
Review Comment:
Verified this is consisent with DuckDB
```sql
D select array_slice([1, 2, 3, 4, 5], -7, -2)
;
┌─────────────────────────────────────────────────────┐
│ array_slice(main.list_value(1, 2, 3, 4, 5), -7, -2) │
│ int32[] │
├─────────────────────────────────────────────────────┤
│ [1, 2, 3, 4] │
└─────────────────────────────────────────────────────┘
```
D select array_slice(['h', 'e', 'l', 'l', 'o'], -7, -3);
┌───────────────────────────────────────────────────────────────┐
│ array_slice(main.list_value('h', 'e', 'l', 'l', 'o'), -7, -3) │
│ varchar[] │
├───────────────────────────────────────────────────────────────┤
│ [h, e, l] │
└───────────────────────────────────────────────────────────────┘
##########
datafusion/functions-nested/src/extract.rs:
##########
@@ -487,7 +487,17 @@ where
// 0 ~ len - 1
let adjusted_zero_index = if index < 0 {
if let Ok(index) = index.try_into() {
- index + len
+ // When index < 0 and -index > length, index is clamped to the
beginning of the list.
+ // Otherwise, when index < 0, the index is counted from the
end of the list.
+ //
+ // Note, we actually test the contrapositive, index < -length,
because negating a
+ // negative will panic if the negative is equal to the
smallest representable value
+ // while negating a positive is always safe.
+ if index < (O::zero() - O::one()) * len {
Review Comment:
I played around with it and I could not find anything better than this
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -1993,6 +1993,28 @@ select array_slice(arrow_cast(make_array(1, 2, 3, 4, 5),
'LargeList(Int64)'), 2,
----
[2, 3, 4] [h, e]
+# array_slice scalar function #24 (with first negative index larger than len)
+query ??
+select array_slice(make_array(1, 2, 3, 4, 5), -2147483648, 1),
list_slice(make_array('h', 'e', 'l', 'l', 'o'), -2147483648, 1);
+----
+[1] [h]
+
+query ??
+select array_slice(arrow_cast(make_array(1, 2, 3, 4, 5), 'LargeList(Int64)'),
-9223372036854775808, 1), list_slice(arrow_cast(make_array('h', 'e', 'l', 'l',
'o'), 'LargeList(Utf8)'), -9223372036854775808, 1);
+----
+[1] [h]
+
+# array_slice scalar function #25 (with negative step and equal indexes)
+query ??
+select array_slice(make_array(1, 2, 3, 4, 5), 2, 2, -1),
list_slice(make_array('h', 'e', 'l', 'l', 'o'), 2, 2, -1);
Review Comment:
```sql
D select array_slice([1, 2, 3, 4, 5], 2, 2, -1);
┌───────────────────────────────────────────────────────┐
│ array_slice(main.list_value(1, 2, 3, 4, 5), 2, 2, -1) │
│ int32[] │
├───────────────────────────────────────────────────────┤
│ [2] │
└───────────────────────────────────────────────────────
```
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -1993,6 +1993,28 @@ select array_slice(arrow_cast(make_array(1, 2, 3, 4, 5),
'LargeList(Int64)'), 2,
----
[2, 3, 4] [h, e]
+# array_slice scalar function #24 (with first negative index larger than len)
+query ??
+select array_slice(make_array(1, 2, 3, 4, 5), -2147483648, 1),
list_slice(make_array('h', 'e', 'l', 'l', 'o'), -2147483648, 1);
Review Comment:
BTW I know this is consistent with the other tests in this file (so we
shouldn't change it in this PR), but DataFusion now supports the `[]` array
literal syntax too:
```sql
(venv) andrewlamb@Andrews-MacBook-Pro-2:~/Software/datafusion$ datafusion-cli
DataFusion CLI v45.0.0
> select array_slice([1, 2, 3, 4, 5], -7, -2);
+-------------------------------------------------------------------------------+
|
make_array(Int64(1),Int64(2),Int64(3),Int64(4),Int64(5))[Int64(-7):Int64(-2)] |
+-------------------------------------------------------------------------------+
| []
|
+-------------------------------------------------------------------------------+
1 row(s) fetched.
Elapsed 0.049 seconds.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
