Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Dec 1, 2015

Following up #10038.

We can use bitmasks to determine which grouping expressions need to be set as nullable.

cc @yhuai

@viirya
Copy link
Member Author

viirya commented Dec 1, 2015

I think this should not need a JIRA. If it is needed, please let me know. Thanks.

@SparkQA
Copy link

SparkQA commented Dec 1, 2015

Test build #46965 has finished for PR 10067 at commit 69be01d.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yhuai
Copy link
Contributor

yhuai commented Dec 1, 2015

How about we use the original jira number?

Copy link
Contributor

Choose a reason for hiding this comment

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

c87531b#commitcomment-14712726 looks like a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we did the same thing? I can't see the difference between this and @hvanhovell's suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is the same thing. I only suggested to aggregate the bitmap in advance: val nonNullBitmask = x.bitmasks.reduce(_ & _) and to use the nonNullBitmask to check for (non) nullability in the loop. It should be a tiny tiny tiny bit quicker (no re-iteration required , and less one closure).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. That's cool. I updated this according to your suggestion.

@viirya viirya changed the title [Minor][SQL] Check bitmasks to set nullable property [SPARK-11949][SQL] Check bitmasks to set nullable property Dec 1, 2015
@viirya
Copy link
Member Author

viirya commented Dec 1, 2015

retest this please.

@SparkQA
Copy link

SparkQA commented Dec 2, 2015

Test build #47006 has finished for PR 10067 at commit 69be01d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Dec 2, 2015

Test build #47016 has finished for PR 10067 at commit 723d24a.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member Author

viirya commented Dec 2, 2015

retest this please.

@SparkQA
Copy link

SparkQA commented Dec 2, 2015

Test build #47035 has finished for PR 10067 at commit 723d24a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yhuai
Copy link
Contributor

yhuai commented Dec 2, 2015

Thanks! Merging to master and branch 1.6.

asfgit pushed a commit that referenced this pull request Dec 2, 2015
Following up #10038.

We can use bitmasks to determine which grouping expressions need to be set as nullable.

cc yhuai

Author: Liang-Chi Hsieh <[email protected]>

Closes #10067 from viirya/fix-cube-following.

(cherry picked from commit 0f37d1d)
Signed-off-by: Yin Huai <[email protected]>
@asfgit asfgit closed this in 0f37d1d Dec 2, 2015
@viirya
Copy link
Member Author

viirya commented Dec 2, 2015

@yhuai Thank you.

@yhuai
Copy link
Contributor

yhuai commented Dec 2, 2015

no problem!

@viirya viirya deleted the fix-cube-following branch December 27, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants