Skip to content

[SPARK-17153][SQL] Should read partition data when reading new files in filestream without globbing #14803

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

Closed
viirya wants to merge 10 commits into from
Closed

[SPARK-17153][SQL] Should read partition data when reading new files in filestream without globbing #14803

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2771d71
Pass path as basePath for partitionSpec creation if path is not globb…
viirya Aug 25, 2016
0d841e2
Merge remote-tracking branch 'upstream/master' into filestreamsource-…
viirya Aug 29, 2016
6adf2e2
Address comment.
viirya Aug 30, 2016
f7c9067
Merge remote-tracking branch 'upstream/master' into filestreamsource-…
viirya Sep 19, 2016
04b61c7
Fix a test.
viirya Sep 19, 2016
23ba9a2
Address reviewer comments.
viirya Sep 20, 2016
5b101ab
Fix wrong comment, slightly tailor the new test.
viirya Sep 20, 2016
541dfdc
Update programming guide.
viirya Sep 20, 2016
9d16631
Merge remote-tracking branch 'upstream/master' into filestreamsource-…
viirya Sep 22, 2016
e21536e
Update doc and enhance the added test.
viirya Sep 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamSource.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,20 @@ class FileStreamSource(
val files = metadataLog.get(Some(startId + 1), Some(endId)).flatMap(_._2)
logInfo(s"Processing ${files.length} files from ${startId + 1}:$endId")
logTrace(s"Files are:\n\t" + files.mkString("\n\t"))
val newOptions = if (!SparkHadoopUtil.get.isGlobPath(new Path(path))) {
Copy link
Contributor

@frreiss frreiss Aug 29, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I recommend putting this check for globs into the initialization code at the top of this file that sets qualifiedBasePath (currently lines 47-50). That way all the code that interprets the meaning of the path parameter will be in one place.

Copy link
Member Author

@viirya viirya Aug 29, 2016

Choose a reason for hiding this comment

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

ok.

options.get("path").map { path =>
sourceOptions.optionMapWithoutPath + (("basePath", path))
}.getOrElse(sourceOptions.optionMapWithoutPath)
} else {
sourceOptions.optionMapWithoutPath
}
val newDataSource =
DataSource(
sparkSession,
paths = files.map(_.path),
userSpecifiedSchema = Some(schema),
className = fileFormatClassName,
options = sourceOptions.optionMapWithoutPath)
options = newOptions)
Dataset.ofRows(sparkSession, LogicalRelation(newDataSource.resolveRelation()))
}

Expand Down
28 changes: 28 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/streaming/FileStreamSourceSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,34 @@ class FileStreamSourceSuite extends FileStreamSourceTest {

// =============== other tests ================

test("read new files in partitioned table without globbing, should read partition data") {
Copy link
Contributor

@marmbrus marmbrus Sep 19, 2016

Choose a reason for hiding this comment

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

We should probably also have an explicit test for the case where schema inference is turned on (you implicitly test it some with the code changed below)

Copy link
Member Author

@viirya viirya Sep 20, 2016

Choose a reason for hiding this comment

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

Added a test for it.

withTempDirs { case (dir, tmp) =>
val partitionFooSubDir = new File(dir, "partition=foo")
val partitionBarSubDir = new File(dir, "partition=bar")

val schema = new StructType().add("value", StringType).add("partition", StringType)
val fileStream = createFileStream("json", s"${dir.getCanonicalPath}", Some(schema))
val filtered = fileStream.filter($"value" contains "keep")
testStream(filtered)(
// Create new partition=foo sub dir and write to it
AddTextFileData("{'value': 'drop1'}\n{'value': 'keep2'}", partitionFooSubDir, tmp),
CheckAnswer(("keep2", "foo")),

// Append to same partition=1 sub dir
AddTextFileData("{'value': 'keep3'}", partitionFooSubDir, tmp),
CheckAnswer(("keep2", "foo"), ("keep3", "foo")),

// Create new partition sub dir and write to it
AddTextFileData("{'value': 'keep4'}", partitionBarSubDir, tmp),
CheckAnswer(("keep2", "foo"), ("keep3", "foo"), ("keep4", "bar")),

// Append to same partition=2 sub dir
AddTextFileData("{'value': 'keep5'}", partitionBarSubDir, tmp),
CheckAnswer(("keep2", "foo"), ("keep3", "foo"), ("keep4", "bar"), ("keep5", "bar"))
)
}
}

test("fault tolerance") {
withTempDirs { case (src, tmp) =>
val fileStream = createFileStream("text", src.getCanonicalPath)
Expand Down