fix length error with array_has
#12459
Conversation
There was a problem hiding this comment.
@alamb I would love to get this merged, it's causing many queries that use ANY() to fail right now.
cc @jayzhan211 who I think wrote this.
| @@ -200,7 +201,10 @@ fn array_has_dispatch_for_scalar<O: OffsetSizeTrait>( | |||
| // If first argument is empty list (second argument is non-null), return false | |||
| // i.e. array_has([], non-null element) -> false | |||
| if values.len() == 0 { | |||
| return Ok(Arc::new(BooleanArray::from(vec![Some(false)]))); | |||
| return Ok(Arc::new(BooleanArray::new( | |||
| BooleanBuffer::new_unset(haystack.len()), | |||
There was a problem hiding this comment.
the point is that in this case the array returned should be the same length as the haystack, not 1.
There was a problem hiding this comment.
Thank you for this fix @samuelcolvin -- I have one small nit pick about where the test should live, but we can also fix that as a follow on PR.
samuelcolvin
force-pushed
the
fix-array_has
branch
from
7123862
to
b59daed
Compare
samuelcolvin
force-pushed
the
fix-array_has
branch
from
b59daed
to
9c4b6c4
Compare
samuelcolvin
force-pushed
the
fix-array_has
branch
from
80906e8
to
16a6006
Compare
|
slt test updated and dedicated test removed. It would be great to get this merged. We'll run datafusion main until there's a another release since this is is affecting a lot of users as we migrate querying from timescale to datafusion. |
| # test empty arrays return length | ||
| # issue: https://github.com/apache/datafusion/pull/12459 | ||
| statement ok | ||
| create table values_all_empty (a int[]) as values ([]), ([]); |
|
Thanks @alamb for merging so quickly. 🙏 |
I try hard to prioritize bug fixes. Thank you for fixing the bug! |
Rationale for this change
Fixes an error we've been experiencing in prod where some queries that use
array_has(including viaANY()) result int:What changes are included in this PR?
Fix a bug in
array_haswhere it returned 1 row, instead of the size of the group.Are these changes tested?
Yes.
Are there any user-facing changes?
No.