Skip to content

Conversation

@dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Dec 12, 2017

What changes were proposed in this pull request?

Until 2.2.1, Spark raises NullPointerException on zero-size ORC files. Usually, these zero-size ORC files are generated by 3rd-party apps like Flume.

scala> sql("create table empty_orc(a int) stored as orc location '/tmp/empty_orc'")

$ touch /tmp/empty_orc/zero.orc

scala> sql("select * from empty_orc").show
java.lang.RuntimeException: serious problem at
org.apache.hadoop.hive.ql.io.orc.OrcInputFormat.generateSplitsInfo(OrcInputFormat.java:1021)
...
Caused by: java.lang.NullPointerException at
org.apache.hadoop.hive.ql.io.orc.OrcInputFormat$BISplitStrategy.getSplits(OrcInputFormat.java:560)

After SPARK-22279, Apache Spark with the default configuration doesn't have this bug. Although Hive 1.2.1 library code path still has the problem, we had better have a test coverage on what we have now in order to prevent future regression on it.

How was this patch tested?

Pass a newly added test case.

@SparkQA
Copy link

SparkQA commented Dec 12, 2017

Test build #84747 has finished for PR 19948 at commit 492a30f.

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

@viirya
Copy link
Member

viirya commented Dec 12, 2017

So if SQLConf.ORC_IMPLEMENTATION is hive, we still hit this bug?

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Dec 12, 2017

Test build #84755 has finished for PR 19948 at commit 492a30f.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Dec 12, 2017

Test build #84758 has finished for PR 19948 at commit 492a30f.

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

@dongjoon-hyun
Copy link
Member Author

dongjoon-hyun commented Dec 12, 2017

Thank you, @HyukjinKwon and @viirya .

@dongjoon-hyun
Copy link
Member Author

@viirya .
The user will hit this issue only when HiveUtils.CONVERT_METASTORE_ORC.key = false.
This test case covers HiveUtils.CONVERT_METASTORE_ORC.key -> true for both native and hive ORC impl.

@HyukjinKwon
Copy link
Member

Hey @dongjoon-hyun, I am going to merge this but let's leave a comment about ^ in the JIRA too for clarification.

@HyukjinKwon
Copy link
Member

Merged to master.

}
}

test("SPARK-19809 NullPointerException on zero-size ORC file") {
Copy link
Member

Choose a reason for hiding this comment

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

This test case should not be put in SQLQuerySuite.scala

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW, @gatorsmile . Which suite do you prefer? So far, this test case covers

  • Both native and hive
  • Also, STORED AS with CONVERT_METASTORE_ORC=true

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'll move this into HiveOrcQuerySuite.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. That sounds fine to me.

@asfgit asfgit closed this in 17cdabb Dec 12, 2017
@gatorsmile
Copy link
Member

@dongjoon-hyun Could you submit a follow-up PR to move the test case?

Files.touch(new File(s"${dir.getCanonicalPath}", "zero.orc"))

withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> "true") { // default since 2.3.0
checkAnswer(sql("SELECT * FROM spark_19809"), Seq.empty)
Copy link
Member

Choose a reason for hiding this comment

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

Use table("spark_19809")

Copy link
Member Author

Choose a reason for hiding this comment

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

It's done.

sql(s"CREATE TABLE spark_19809(a int) STORED AS ORC LOCATION '$dir'")
Files.touch(new File(s"${dir.getCanonicalPath}", "zero.orc"))

withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> "true") { // default since 2.3.0
Copy link
Member

Choose a reason for hiding this comment

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

Use both true and false

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Dec 13, 2017

Choose a reason for hiding this comment

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

Ur, this test case is for convertMetastoreOrc=true which bacame default in Spark 2.3.0.

false still has the issue of Hive 1.2.1 OrcInputFormat.getSplits, so I wrote like the following in PR description.

After SPARK-22279, Apache Spark with the default configuration doesn't have this bug. Although Hive 1.2.1 library code path still has the problem, we had better have a test coverage on what we have now in order to prevent future regression on it.

Copy link
Member

Choose a reason for hiding this comment

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

Add a comment in the test case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure!

@dongjoon-hyun
Copy link
Member Author

Thank you, @HyukjinKwon and @gatorsmile . I'll.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-19809-EMPTY-FILE branch December 12, 2017 23:55
ghost pushed a commit to dbtsai/spark that referenced this pull request Dec 13, 2017
…Suite

## What changes were proposed in this pull request?

As a follow-up of apache#19948 , this PR moves the test case and adds comments.

## How was this patch tested?

Pass the Jenkins.

Author: Dongjoon Hyun <[email protected]>

Closes apache#19960 from dongjoon-hyun/SPARK-19809-2.
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.

5 participants