-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve array_prepend
signature for null and empty array
#8625
Conversation
Signed-off-by: jayzhan211 <[email protected]>
|
||
# DuckDB: [[]] | ||
# ClickHouse: [[]] | ||
# TODO: We may also return [[]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure should we implement this, return value with larger dimension seems weird to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree -- let's not implement this until someone asks for it explicitly
|
||
# DuckDB: [null] | ||
# ClickHouse: [null] | ||
# TODO: We may also return [null] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both cases are not considered in array_append
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks @jayzhan211 👍
And I could try to test the null for LargeList
after #8569 merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @jayzhan211 -- this is looking very nice. 👌
|
||
# DuckDB: [[]] | ||
# ClickHouse: [[]] | ||
# TODO: We may also return [[]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree -- let's not implement this until someone asks for it explicitly
Thank you for the review @Weijun-H |
Which issue does this PR close?
Closes #.
Part of #7142
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?