Skip to content
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

Support filter for List #11091

Merged
merged 5 commits into from
Jun 27, 2024
Merged

Support filter for List #11091

merged 5 commits into from
Jun 27, 2024

Conversation

jayzhan211
Copy link
Contributor

@jayzhan211 jayzhan211 commented Jun 24, 2024

Which issue does this PR close?

Close #9857

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@jayzhan211
Copy link
Contributor Author

jayzhan211 commented Jun 24, 2024

It seems we need to rewrite array || element to array_append first to avoid the element to list coercion

The purpose is to remove these two lines, which tries to convert non-list to list, which is not expected. But it is incorrectly used for array || element

(List(_), _) => Some(lhs_type.clone()),
(_, List(_)) => Some(rhs_type.clone()),

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
@jayzhan211 jayzhan211 marked this pull request as ready for review June 25, 2024 02:28
@jayzhan211 jayzhan211 changed the title Basic comparison for List Support filter for List Jun 25, 2024
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.

Thank you @jayzhan211 -- this looks very good to me 👨‍🍳 👌

Really nice

return Ok(BooleanArray::new_null(len));
}

let cmp = make_comparator(l, r, SortOptions::default())?;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it worth a comment here explaining the choice of NULL comparison / sorting?

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
@jayzhan211 jayzhan211 merged commit ff116c3 into apache:main Jun 27, 2024
23 checks passed
@jayzhan211 jayzhan211 deleted the list-cmp branch June 27, 2024 08:40
@alamb
Copy link
Contributor

alamb commented Jun 27, 2024

🚀

findepi pushed a commit to findepi/datafusion that referenced this pull request Jul 16, 2024
* support basic list cmp

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* add more ops

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* add distinct

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* nested

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

* add comment

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>

---------

Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
physical-expr Physical Expressions sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support filter for ARRAY
2 participants