You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "siying (via GitHub)" <gi...@apache.org> on 2023/05/02 22:20:37 UTC
[GitHub] [spark] siying opened a new pull request, #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
siying opened a new pull request, #41022:
URL: https://github.com/apache/spark/pull/41022
### What changes were proposed in this pull request?
When FileStreamSource creates a DataSource for a file, disable globbing in the option passed to DataSource.
### Why are the changes needed?
It is to fix following bug.
For example, If a directory contains a following file:
/path/abc[123]
and users would load spark.readStream.format("text").load("/path") as stream input. It throws an exception, saying no matching path /path/abc[123]. Spark thinks abc[123] is a regex that only matches file named abc1, abc2 and abc3.
The bug is due to a second glob pattern match within DataSource, against files already glob matched by FileStreamSource. This match turns real file name into file path match pattern. It is unexpected and we would like to disable it.
### Does this PR introduce _any_ user-facing change?
No, except fixing the unexpected buggy behavior.
### How was this patch tested?
Added unit test scenarios which failed before the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #41022:
URL: https://github.com/apache/spark/pull/41022#issuecomment-1539357874
I see all tests passed; https://github.com/siying/spark/actions/runs/4920444751
Thanks! Merging to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] siying commented on pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on PR #41022:
URL: https://github.com/apache/spark/pull/41022#issuecomment-1532239242
@HeartSaVioR will you have time to take a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on a diff in pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on code in PR #41022:
URL: https://github.com/apache/spark/pull/41022#discussion_r1186982627
##########
sql/core/src/test/scala/org/apache/spark/sql/streaming/FileStreamSourceSuite.scala:
##########
@@ -474,47 +474,64 @@ class FileStreamSourceSuite extends FileStreamSourceTest {
}
}
- test("SPARK-21996 read from text files -- file name has space") {
- withTempDirs { case (src, tmp) =>
- val textStream = createFileStream("text", src.getCanonicalPath)
- val filtered = textStream.filter($"value" contains "keep")
+ test("SPARK-21996/SPARK-43343 read from text files -- file name has special chars") {
+ Seq(" ", "[", "[123]").foreach { special_str =>
+ withTempDirs { case (src, tmp) =>
+ val textStream = createFileStream("text", src.getCanonicalPath)
+ val filtered = textStream.filter($"value" contains "keep")
- testStream(filtered)(
- AddTextFileData("drop1\nkeep2\nkeep3", src, tmp, "text text"),
- CheckAnswer("keep2", "keep3")
- )
+ testStream(filtered)(
+ AddTextFileData("drop1\nkeep2\nkeep3", src, tmp, s"text${special_str}text"),
+ CheckAnswer("keep2", "keep3")
+ )
+ }
}
}
- test("SPARK-21996 read from text files generated by file sink -- file name has space") {
+ test(
+ "SPARK-21996/SPARK-43343 read from text files generated by file sink --" +
+ "file name has special chars") {
val testTableName = "FileStreamSourceTest"
- withTable(testTableName) {
- withTempDirs { case (src, checkpoint) =>
- val output = new File(src, "text text")
- val inputData = MemoryStream[String]
- val ds = inputData.toDS()
-
- val query = ds.writeStream
- .option("checkpointLocation", checkpoint.getCanonicalPath)
- .format("text")
- .start(output.getCanonicalPath)
+ Seq(" ", "[", "[123]").foreach{ special_str =>
+ withTable(testTableName) {
+ withTempDirs { case (src, checkpoint) =>
+ val output = new File(src, s"text${special_str}text")
+ val inputData = MemoryStream[String]
+ val ds = inputData.toDS()
+
+ val query = ds.writeStream
+ .option("checkpointLocation", checkpoint.getCanonicalPath)
+ .format("text")
+ .start(output.getCanonicalPath)
- try {
- inputData.addData("foo")
- failAfter(streamingTimeout) {
- query.processAllAvailable()
+ try {
+ inputData.addData("foo")
+ failAfter(streamingTimeout) {
+ query.processAllAvailable()
+ }
+ } finally {
+ query.stop()
}
- } finally {
- query.stop()
- }
- val df2 = spark.readStream.format("text").load(output.getCanonicalPath)
- val query2 = df2.writeStream.format("memory").queryName(testTableName).start()
- try {
- query2.processAllAvailable()
- checkDatasetUnorderly(spark.table(testTableName).as[String], "foo")
- } finally {
- query2.stop()
+ // We cannot put "[" as part of the input path as it will be intepreted as glob pattern,
+ // so use replace it with "*".
+ val input_path =
+ if (special_str == " ") {
+ output.getCanonicalPath
+ } else {
+ output
Review Comment:
Shall we give a try with `\` which escapes the next character? We can leave as it is if it does not work.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/FileStreamSource.scala:
##########
@@ -69,12 +69,15 @@ class FileStreamSource(
private val sourceCleaner: Option[FileStreamSourceCleaner] = FileStreamSourceCleaner(
fs, qualifiedBasePath, sourceOptions, hadoopConf)
- private val optionsWithPartitionBasePath = sourceOptions.optionMapWithoutPath ++ {
- if (!SparkHadoopUtil.get.isGlobPath(new Path(path)) && options.contains("path")) {
- Map("basePath" -> path)
- } else {
- Map()
- }}
+ private val optionsForInnerDataSource = sourceOptions.optionMapWithoutPath ++ {
+ val pathOption =
+ if (!SparkHadoopUtil.get.isGlobPath(new Path(path)) && options.contains("path")) {
+ Map("basePath" -> path)
+ } else {
+ Map()
+ }
+ pathOption ++ Map (DataSource.GLOB_PATHS_KEY -> "false")
Review Comment:
nit: not necessarily space between Map and `(` (super nit but consistent with other codes like above)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] siying commented on pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on PR #41022:
URL: https://github.com/apache/spark/pull/41022#issuecomment-1539163892
Not sure why one test ran for 4+ hours and it still didn't succeed or fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR commented on pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR commented on PR #41022:
URL: https://github.com/apache/spark/pull/41022#issuecomment-1539170299
It seems like the actual test run is only 2 hours ago - check out the full log via `view raw logs`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] siying commented on pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on PR #41022:
URL: https://github.com/apache/spark/pull/41022#issuecomment-1539068615
> Nice finding! Looks great in overall - just to see whether we can still read a single file containing special character via escaping it. I'd OK with it if it does not work in any way (Hadoop side bug or something) but better to confirm.
Thanks for your review and suggestion. I gave it a try ant it looks like "\[" works. I updated the PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] siying commented on pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "siying (via GitHub)" <gi...@apache.org>.
siying commented on PR #41022:
URL: https://github.com/apache/spark/pull/41022#issuecomment-1539284464
Failed tests don't seem to be related at all:
```
2023-05-08T23:33:54.8173143Z [0m[[0m[31merror[0m] [0m[0mFailed tests:[0m
2023-05-08T23:33:54.8173854Z [0m[[0m[31merror[0m] [0m[0m org.apache.spark.sql.connect.client.SparkConnectClientSuite[0m
2023-05-08T23:33:54.8805941Z [0m[[0m[31merror[0m] [0m[0m(connect-client-jvm / Test / [31mtest[0m) sbt.TestsFailedException: Tests unsuccessful[0m
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HeartSaVioR closed pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
Posted by "HeartSaVioR (via GitHub)" <gi...@apache.org>.
HeartSaVioR closed pull request #41022: [SPARK-43343][SS] FileStreamSource should disable an extra file glob check when creating DataSource
URL: https://github.com/apache/spark/pull/41022
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org