Skip to content

[SPARK-8868] SqlSerializer2 can go into infinite loop when row consists only of NullType columns #7262

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[SPARK-8868] SqlSerializer2 can go into infinite loop when row consists only of NullType columns #7262

wants to merge 3 commits into from

Conversation

@yhuai
Copy link
Contributor

@yhuai yhuai commented Jul 7, 2015

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build triggered.

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 7, 2015

Test build #36692 has started for PR 7262 at commit 5122e65.

JoshRosen
JoshRosen reviewed Jul 7, 2015
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlSerializer2.scala Outdated
Copy link
Contributor

@JoshRosen JoshRosen Jul 7, 2015

Choose a reason for hiding this comment

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

Typo: schemata?

@SparkQA
Copy link

SparkQA commented Jul 7, 2015

Test build #36692 has finished for PR 7262 at commit 5122e65.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build finished. Test FAILed.

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build triggered.

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 7, 2015

Test build #36696 has started for PR 7262 at commit e456857.

@SparkQA
Copy link

SparkQA commented Jul 7, 2015

Test build #36696 has finished for PR 7262 at commit e456857.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build finished. Test PASSed.

andrewor14
andrewor14 reviewed Jul 7, 2015
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlSerializer2.scala Outdated
Copy link
Contributor

@andrewor14 andrewor14 Jul 7, 2015

Choose a reason for hiding this comment

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

pretty minor, but this is the same as just setting allNullTypes = false

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build triggered.

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 7, 2015

Test build #36712 has started for PR 7262 at commit cb58780.

@SparkQA
Copy link

SparkQA commented Jul 7, 2015

Test build #36712 has finished for PR 7262 at commit cb58780.

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

@AmplabJenkins
Copy link

AmplabJenkins commented Jul 7, 2015

Merged build finished. Test PASSed.

JoshRosen
JoshRosen reviewed Jul 7, 2015
View reviewed changes
sql/core/src/test/scala/org/apache/spark/sql/execution/SparkSqlSerializer2Suite.scala
Copy link
Contributor

@JoshRosen JoshRosen Jul 7, 2015

Choose a reason for hiding this comment

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

For completeness, do we need a test to check that schemas that contain a NullType column and a column of some other supported type are supported?

Copy link
Contributor Author

@yhuai yhuai Jul 7, 2015

Choose a reason for hiding this comment

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

oh, we have that test. The table shuffle used in SparkSqlSerializer2Suite contains all supported types and NullType.

@JoshRosen
Copy link
Contributor

JoshRosen commented Jul 8, 2015

LGTM, so I'm going to merge this for master (1.5.0) and 1.4.2. Thanks @yhuai!

@asfgit asfgit closed this in 68a4a16 Jul 8, 2015
asfgit pushed a commit that referenced this pull request Jul 8, 2015
@yhuai @JoshRosen
[SPARK-8868] SqlSerializer2 can go into infinite loop when row consis…
d3d5f2a 
…ts only of NullType columns

https://issues.apache.org/jira/browse/SPARK-8868

Author: Yin Huai <[email protected]>

Closes #7262 from yhuai/SPARK-8868 and squashes the following commits:

cb58780 [Yin Huai] Andrew's comment.
e456857 [Yin Huai] Josh's comments.
5122e65 [Yin Huai] If types of all columns are NullTypes, do not use serializer2.

(cherry picked from commit 68a4a16)
Signed-off-by: Josh Rosen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yhuai @AmplabJenkins @SparkQA @JoshRosen @andrewor14