Skip to content

Conversation

Jefffrey
Copy link
Contributor

Which issue does this PR close?

Part of #8249 and #8185

Rationale for this change

Some signature for nested functions are outdated and not using more robust/streamlined type signatures; also add more tests (specifically for LargeList and FixedSizeList array types) for nested functions.

What changes are included in this PR?

Refactor signatures of nested functions to be more streamlined and robust. Add more SLT tests for nested functions to ensure they work with LargeList and FixedSizeList types. No expected behaviour change for these functions in this PR, just refactoring.

Are these changes tested?

Existing SLT tests passed & added more tests.

Are there any user-facing changes?

No

@github-actions github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Sep 19, 2025
Comment on lines -107 to +97
List(field) | FixedSizeList(field, _) => match field.data_type() {
List(field) => match field.data_type() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed FixedSizeList since signature should coerce it to List

vec![
TypeSignature::ArraySignature(ArrayFunctionSignature::Array {
arguments: vec![ArrayFunctionArgument::Array],
array_coercion: None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No coercion here as the implementation natively supports FixedSizeList

fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
match &arg_types[0] {
List(field) | FixedSizeList(field, _) => Ok(List(Arc::clone(field))),
List(field) => Ok(List(Arc::clone(field))),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reason as flatten


# DuckDB: [4]
# ClickHouse: Null
# Since they dont have the same result, we just follow Postgres, return error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this since the query doesn't seem to error (am I misunderstanding what the comment means 🤔 )

@Jefffrey Jefffrey mentioned this pull request Sep 19, 2025
19 tasks
@Jefffrey Jefffrey marked this pull request as ready for review September 19, 2025 15:26
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks like a good change to me -- thanks @Jefffrey

@Jefffrey Jefffrey added this pull request to the merge queue Sep 20, 2025
Merged via the queue into apache:main with commit c2d839f Sep 20, 2025
28 checks passed
@Jefffrey Jefffrey deleted the refactor_arr_sigs branch September 20, 2025 23:00
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