Skip to content

Conversation

@jonahgao
Copy link
Member

Which issue does this PR close?

Found in #9315 (comment)

unnest(make_array(1,2)) is supported, but unnest(range(1,3)) is not supported.

DataFusion CLI v36.0.0
❯ select unnest(make_array(1,2));
+-------------------------------+
| make_array(Int64(1),Int64(2)) |
+-------------------------------+
| 1                             |
| 2                             |
+-------------------------------+
2 rows in set. Query took 0.008 seconds.

❯ select unnest(range(1,3));
Error during planning: unnest() can only be applied to array and structs and null

These are all supported in DuckDB.

Rationale for this change

If I understand correctly, unnest should support any expression with a return type of Array, not just the make_array function and column expressions.

What changes are included in this PR?

The unnest function can accept any array expression, which is ensured by checking the return type of the argument.
Previously, it only supported the make_array function and column expressions.

Are these changes tested?

Yes. New tests and existing tests.

Are there any user-facing changes?

No

@github-actions github-actions bot added sql SQL Planner sqllogictest SQL Logic Tests (.slt) labels Feb 26, 2024

query error DataFusion error: Error during planning: unnest\(\) can only be applied to array and structs and null
query error DataFusion error: Error during planning: unnest\(\) can only be applied to array and struct
select unnest(null);
Copy link
Member Author

Choose a reason for hiding this comment

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

This test shows that null is not supported, which conflicts with the error message.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is an error because unnest with null is not supported, but it should, so the error message contains null and struct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also add null checking and return not yet implemented error

Copy link
Member Author

Choose a reason for hiding this comment

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

I misunderstood the intention of this error message. Fixed and thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you also add null checking and return not yet implemented error

Done.

}
};
// Check argument type, array types are supported
match arg.get_type(schema)? {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@jayzhan211
Copy link
Contributor

Thanks @jonahgao

Copy link
Contributor

@jayzhan211 jayzhan211 left a comment

Choose a reason for hiding this comment

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

LGTM

@jayzhan211 jayzhan211 merged commit ec86acb into apache:main Feb 26, 2024
@jonahgao jonahgao deleted the unnest_expand branch February 26, 2024 13:36
@alamb
Copy link
Contributor

alamb commented Feb 26, 2024

❤️

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

Labels

sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants