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

Add BaseVector::containsNullAt API #6515

Closed

Conversation

mbasmanova
Copy link
Contributor

BaseVector::containsNullAt(idx) returns true if value at the specified index
is null or contains null.

Primitive type values can be null, but cannot contain nulls. Arrays, maps
and structs can be null and can contains nulls. Non-null array may contain
one or more elements that are null or contain nulls themselves. Non-null
maps may contain one more entry with key or value that's null or contains
null. Non-null struct may contain a field that's null or contains null.

This functionality is need to implement IN predicate for complex types: #6513

@netlify
Copy link

netlify bot commented Sep 11, 2023

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 7e3e957
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/64ff820612fefd00089de34c

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 11, 2023
@facebook-github-bot
Copy link
Contributor

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@pedroerp pedroerp left a comment

Choose a reason for hiding this comment

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

Thanks for breaking this into smaller PRs to make it easier to review! A small question for my own understanding, but looks good.

@@ -80,6 +80,19 @@ class DictionaryVector : public SimpleVector<T> {

bool isNullAt(vector_size_t idx) const override;

bool containsNullAt(vector_size_t idx) const override {
if constexpr (std::is_same_v<T, ComplexType>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure I understand this part. In which cases we have dictionaries with ComplexType and when we don't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A dictionary vector can wrap a primitive type vector or a complex type vector. In the former case T will be primitive type. In the latter case T will be ComplexType.

EXPECT_TRUE(data->containsNullAt(1));
EXPECT_TRUE(data->containsNullAt(2));
EXPECT_TRUE(data->containsNullAt(3));
EXPECT_FALSE(data->containsNullAt(4));
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also have a test for nested containers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Let me add some.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pedroerp Added.

@facebook-github-bot
Copy link
Contributor

@mbasmanova has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mbasmanova merged this pull request in fb71ab2.

facebook-github-bot pushed a commit that referenced this pull request Sep 13, 2023
Summary:
In Presto, x IN (a, b, c) evaluated on complex types returns NULL if x doesn't match a, b, c, but
contains null or any of a, b, c contains null.

Depends on #6515

Pull Request resolved: #6513

Reviewed By: pedroerp

Differential Revision: D49155902

Pulled By: mbasmanova

fbshipit-source-id: 5149122a9e888d2831d9462167fd3ac33ce04db8
codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request Sep 27, 2023
Summary:
BaseVector::containsNullAt(idx) returns true if value at the specified index
is null or contains null.

Primitive type values can be null, but cannot contain nulls. Arrays, maps
and structs can be null and can contains nulls. Non-null array may contain
one or more elements that are null or contain nulls themselves. Non-null
maps may contain one more entry with key or value that's null or contains
null. Non-null struct may contain a field that's null or contains null.

This functionality is need to implement IN predicate for complex types: facebookincubator#6513

Pull Request resolved: facebookincubator#6515

Reviewed By: pedroerp

Differential Revision: D49155796

Pulled By: mbasmanova

fbshipit-source-id: b935744575bc67efad746fb1161673aa430606c6
codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request Sep 27, 2023
…ator#6513)

Summary:
In Presto, x IN (a, b, c) evaluated on complex types returns NULL if x doesn't match a, b, c, but
contains null or any of a, b, c contains null.

Depends on facebookincubator#6515

Pull Request resolved: facebookincubator#6513

Reviewed By: pedroerp

Differential Revision: D49155902

Pulled By: mbasmanova

fbshipit-source-id: 5149122a9e888d2831d9462167fd3ac33ce04db8
codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request Sep 27, 2023
Summary:
BaseVector::containsNullAt(idx) returns true if value at the specified index
is null or contains null.

Primitive type values can be null, but cannot contain nulls. Arrays, maps
and structs can be null and can contains nulls. Non-null array may contain
one or more elements that are null or contain nulls themselves. Non-null
maps may contain one more entry with key or value that's null or contains
null. Non-null struct may contain a field that's null or contains null.

This functionality is need to implement IN predicate for complex types: facebookincubator#6513

Pull Request resolved: facebookincubator#6515

Reviewed By: pedroerp

Differential Revision: D49155796

Pulled By: mbasmanova

fbshipit-source-id: b935744575bc67efad746fb1161673aa430606c6
codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request Sep 27, 2023
…ator#6513)

Summary:
In Presto, x IN (a, b, c) evaluated on complex types returns NULL if x doesn't match a, b, c, but
contains null or any of a, b, c contains null.

Depends on facebookincubator#6515

Pull Request resolved: facebookincubator#6513

Reviewed By: pedroerp

Differential Revision: D49155902

Pulled By: mbasmanova

fbshipit-source-id: 5149122a9e888d2831d9462167fd3ac33ce04db8
codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request Sep 27, 2023
Summary:
BaseVector::containsNullAt(idx) returns true if value at the specified index
is null or contains null.

Primitive type values can be null, but cannot contain nulls. Arrays, maps
and structs can be null and can contains nulls. Non-null array may contain
one or more elements that are null or contain nulls themselves. Non-null
maps may contain one more entry with key or value that's null or contains
null. Non-null struct may contain a field that's null or contains null.

This functionality is need to implement IN predicate for complex types: facebookincubator#6513

Pull Request resolved: facebookincubator#6515

Reviewed By: pedroerp

Differential Revision: D49155796

Pulled By: mbasmanova

fbshipit-source-id: b935744575bc67efad746fb1161673aa430606c6
codyschierbeck pushed a commit to codyschierbeck/velox that referenced this pull request Sep 27, 2023
…ator#6513)

Summary:
In Presto, x IN (a, b, c) evaluated on complex types returns NULL if x doesn't match a, b, c, but
contains null or any of a, b, c contains null.

Depends on facebookincubator#6515

Pull Request resolved: facebookincubator#6513

Reviewed By: pedroerp

Differential Revision: D49155902

Pulled By: mbasmanova

fbshipit-source-id: 5149122a9e888d2831d9462167fd3ac33ce04db8
ericyuliu pushed a commit to ericyuliu/velox that referenced this pull request Oct 12, 2023
Summary:
BaseVector::containsNullAt(idx) returns true if value at the specified index
is null or contains null.

Primitive type values can be null, but cannot contain nulls. Arrays, maps
and structs can be null and can contains nulls. Non-null array may contain
one or more elements that are null or contain nulls themselves. Non-null
maps may contain one more entry with key or value that's null or contains
null. Non-null struct may contain a field that's null or contains null.

This functionality is need to implement IN predicate for complex types: facebookincubator#6513

Pull Request resolved: facebookincubator#6515

Reviewed By: pedroerp

Differential Revision: D49155796

Pulled By: mbasmanova

fbshipit-source-id: b935744575bc67efad746fb1161673aa430606c6
ericyuliu pushed a commit to ericyuliu/velox that referenced this pull request Oct 12, 2023
…ator#6513)

Summary:
In Presto, x IN (a, b, c) evaluated on complex types returns NULL if x doesn't match a, b, c, but
contains null or any of a, b, c contains null.

Depends on facebookincubator#6515

Pull Request resolved: facebookincubator#6513

Reviewed By: pedroerp

Differential Revision: D49155902

Pulled By: mbasmanova

fbshipit-source-id: 5149122a9e888d2831d9462167fd3ac33ce04db8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants