You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "Jefffrey (via GitHub)" <gi...@apache.org> on 2026/07/08 05:29:44 UTC
[PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Jefffrey opened a new pull request, #23382:
URL: https://github.com/apache/datafusion/pull/23382
Cleaning some outdated TODO items or updating the queries so they at least run (even if result is incorrect)
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #23382:
URL: https://github.com/apache/datafusion/pull/23382#discussion_r3551724409
##########
datafusion/sqllogictest/test_files/array/array_has.slt:
##########
@@ -41,13 +41,11 @@ select array_has([1, null, 2], 3),
false false
#TODO: array_has_all and array_has_any cannot handle NULL
-#query BBBB
-#select array_has_any([], null),
-# array_has_any([1, 2, 3], null),
-# array_has_all([], null),
-# array_has_all([1, 2, 3], null);
-#----
-#false false false false
+query error array_has does not support type 'Null'
+select array_has_any([], null),
+ array_has_any([1, 2, 3], null),
+ array_has_all([], null),
+ array_has_all([1, 2, 3], null);
Review Comment:
good point, ive split them up
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #23382:
URL: https://github.com/apache/datafusion/pull/23382#discussion_r3551727638
##########
datafusion/sqllogictest/test_files/array/array_index.slt:
##########
Review Comment:
thanks, fixed them. i think the way you have the SQL is the correct way, since that is what sqlparser-rs accepts
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on code in PR #23382:
URL: https://github.com/apache/datafusion/pull/23382#discussion_r3541434111
##########
datafusion/sqllogictest/test_files/array/array_prepend.slt:
##########
@@ -57,7 +57,6 @@ select array_prepend(null, [[1,2,3]]);
# DuckDB: [[]]
# ClickHouse: [[]]
-# TODO: We may also return [[]]
Review Comment:
seems we already return this
##########
datafusion/sqllogictest/test_files/ddl.slt:
##########
@@ -200,10 +200,6 @@ SELECT foo_schema.bar.a FROM foo_schema.bar;
----
1
-# TODO: Drop schema for cleanup, see #6027
-# statement ok
-# DROP SCHEMA foo_schema;
Review Comment:
tested later in the file now
https://github.com/apache/datafusion/blob/f34a676302e2320526172705503a5ac8222804ea/datafusion/sqllogictest/test_files/ddl.slt#L772-L776
##########
datafusion/sqllogictest/test_files/array/array_empty.slt:
##########
@@ -69,10 +69,8 @@ false
#TODO: https://github.com/apache/datafusion/issues/7142
# empty scalar function #4
-#query B
-#select empty(NULL);
-#----
-#NULL
+query error array_empty does not support type Null
+select empty(NULL);
Review Comment:
its better for it to run to an error so when we fix it later we'll know to also fix this here
##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -1978,15 +1977,15 @@ query B
select column1 <=> column2 from (VALUES (1, 1), (2, 3), (NULL, NULL)) as t;
----
true
-false
+false
true
# Sanity test - comparing <=> with equivalent expression
query B
-SELECT
- (column1 <=> column2) =
Review Comment:
my editor was removing some trailing whitespace so decided to leave that little cleanup in as well
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #23382:
URL: https://github.com/apache/datafusion/pull/23382#issuecomment-4929604605
This is greeat
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Posted by "nuno-faria (via GitHub)" <gi...@apache.org>.
nuno-faria commented on code in PR #23382:
URL: https://github.com/apache/datafusion/pull/23382#discussion_r3546514323
##########
datafusion/sqllogictest/test_files/array/array_has.slt:
##########
@@ -41,13 +41,11 @@ select array_has([1, null, 2], 3),
false false
#TODO: array_has_all and array_has_any cannot handle NULL
-#query BBBB
-#select array_has_any([], null),
-# array_has_any([1, 2, 3], null),
-# array_has_all([], null),
-# array_has_all([1, 2, 3], null);
-#----
-#false false false false
+query error array_has does not support type 'Null'
+select array_has_any([], null),
+ array_has_any([1, 2, 3], null),
+ array_has_all([], null),
+ array_has_all([1, 2, 3], null);
Review Comment:
Should these return null, like the `select array_distinct(null);` above or like in DuckDB? If so, we could split this into two tests, since `array_has_any` already does not return an error.
##########
datafusion/sqllogictest/test_files/array/array_index.slt:
##########
Review Comment:
This test works if we convert `[2 | 2:10]` to `[(2 | 2):10]`:
```sql
> select make_array(1, 2, 3)[2 + 1 - 1:10], make_array(1.0, 2.0, 3.0)[(2 | 2):10], make_array('h', 'e', 'l', 'l', 'o')[6 ^ 6:10];
+-------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------+
| make_array(Int64(1),Int64(2),Int64(3))[Int64(2) + Int64(1) - Int64(1):Int64(10):Int64(1)] | make_array(Float64(1),Float64(2),Float64(3))[Int64(2) | Int64(2):Int64(10):Int64(1)] | make_array(Utf8("h"),Utf8("e"),Utf8("l"),Utf8("l"),Utf8("o"))[Int64(6) BIT_XOR Int64(6):Int64(10):Int64(1)] |
+-------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------+
| [2, 3] | [2.0, 3.0] | [h, e, l, l, o] |
+-------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------+
```
I don't know if the idea is for it to work without them.
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: cleanup some TODO items in sqllogictests [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #23382:
URL: https://github.com/apache/datafusion/pull/23382
--
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-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org