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

fix(joins): Strengthen nullptr check for null aware hash joins with filters #11620

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DanielHunte
Copy link

Summary: The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Differential Revision: D66333546

@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 Nov 21, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

Copy link

netlify bot commented Nov 21, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit aedecdc
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/675cdc7ec89aea000863d468

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 21, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Differential Revision: D66333546
velox/exec/HashProbe.cpp Outdated Show resolved Hide resolved
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 22, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

Copy link
Contributor

@Yuhta Yuhta left a comment

Choose a reason for hiding this comment

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

The join fix looks good. Let's make sure the build is not broken before merging (known failing individual tests are fine).

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 25, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 25, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 25, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 25, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 25, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 25, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 26, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 26, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 26, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Nov 26, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 6, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 6, 2024
…ilters (facebookincubator#11620)

Summary:
Pull Request resolved: facebookincubator#11620

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 9, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 10, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 10, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 14, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
Daniel Hunte added 3 commits December 13, 2024 17:16
…eferenceQueryRunners (facebookincubator#11566)

Summary:

This change updates both the DuckQueryRunner and PrestoQueryRunner to parse filters in their hasJoinNode toSql methods.

Reviewed By: kagamiori

Differential Revision: D66021799
…ery Runners (facebookincubator#11576)

Summary:

The select clause is completely missing in the produced query string.

Reviewed By: kagamiori

Differential Revision: D66132514
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D66333546

DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 14, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
DanielHunte pushed a commit to DanielHunte/velox that referenced this pull request Dec 14, 2024
…ilters (facebookincubator#11620)

Summary:

The mayHaveNulls() method sometimes returns true when there are in fact no nulls. This change will make sure that a nullptr isn't passed to memcpy.

Reviewed By: Yuhta

Differential Revision: D66333546
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. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants