Skip to content

chore: cleanup some TODO items in sqllogictests#23382

Open
Jefffrey wants to merge 1 commit into
apache:mainfrom
Jefffrey:cleanup-slt-todos
Open

chore: cleanup some TODO items in sqllogictests#23382
Jefffrey wants to merge 1 commit into
apache:mainfrom
Jefffrey:cleanup-slt-todos

Conversation

@Jefffrey

@Jefffrey Jefffrey commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Cleaning some outdated TODO items or updating the queries so they at least run (even if result is incorrect)

@github-actions github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Jul 8, 2026
Comment on lines +72 to +73
query error array_empty does not support type Null
select empty(NULL);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

its better for it to run to an error so when we fix it later we'll know to also fix this here


# DuckDB: [[]]
# ClickHouse: [[]]
# TODO: We may also return [[]]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

seems we already return this


# TODO: Drop schema for cleanup, see #6027
# statement ok
# DROP SCHEMA foo_schema;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

tested later in the file now

statement error DataFusion error: Execution error: Cannot drop schema foo_schema because other tables depend on it: bar
DROP SCHEMA foo_schema;
statement ok
DROP SCHEMA foo_schema CASCADE;

# Sanity test - comparing <=> with equivalent expression
query B
SELECT
(column1 <=> column2) =

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

my editor was removing some trailing whitespace so decided to leave that little cleanup in as well

@nuno-faria nuno-faria left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @Jefffrey, LGTM. I double checked some of these with DuckDB and they appear ok. I leave some suggestions below.

Comment on lines +44 to +48
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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines 156 to 160
# TODO: support complex index
# multiple index with columns #4 (complex index)
# query III
# 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];
# ----

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test works if we convert [2 | 2:10] to [(2 | 2):10]:

> 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants