-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-41708][SQL][FOLLOWUP] Override toString method of FileIndex
#39610
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.
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
Conversation
|
|
||
| == Analyzed Logical Plan == | ||
| InsertIntoHadoopFsRelationCommand Location [not included in comparison]/{warehouse_dir}/explain_temp5], Append, `spark_catalog`.`default`.`explain_temp5`, org.apache.spark.sql.execution.datasources.CatalogFileIndex, [key, val] | ||
| InsertIntoHadoopFsRelationCommand Location [not included in comparison]/{warehouse_dir}/explain_temp5, false, [val#x], Parquet, [path=Location [not included in comparison]/{warehouse_dir}/explain_temp5], Append, `spark_catalog`.`default`.`explain_temp5`, org.apache.spark.sql.execution.datasources.CatalogFileIndex(Location [not included in comparison]/{warehouse_dir}/explain_temp5), [key, val] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cloud-fan Is this OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also cc @dongjoon-hyun
sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestHelper.scala
Outdated
Show resolved
Hide resolved
cloud-fan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the quick fix!
…r.scala Location -> file: Co-authored-by: Wenchen Fan <[email protected]>
dongjoon-hyun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM, too.
|
Merged to master. |
|
Thanks @dongjoon-hyun @cloud-fan |
### What changes were proposed in this pull request? This is a followup of #39610 . The non-greedy mode does not work well if the ending class name appears more than once, like `file://myPath/.../clsName/.../clsName`. We should still use greedy mode, but to match any chars that are not space or comma (comma is used to combine multiple paths in `FileIndex.toString`). ### Why are the changes needed? make the test framework more stable ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? N/A Closes #39924 from cloud-fan/minor. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>
### What changes were proposed in this pull request? This is a followup of #39610 . The non-greedy mode does not work well if the ending class name appears more than once, like `file://myPath/.../clsName/.../clsName`. We should still use greedy mode, but to match any chars that are not space or comma (comma is used to combine multiple paths in `FileIndex.toString`). ### Why are the changes needed? make the test framework more stable ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? N/A Closes #39924 from cloud-fan/minor. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]> (cherry picked from commit 04550ed) Signed-off-by: Hyukjin Kwon <[email protected]>
### What changes were proposed in this pull request? This is a followup of apache#39610 . The non-greedy mode does not work well if the ending class name appears more than once, like `file://myPath/.../clsName/.../clsName`. We should still use greedy mode, but to match any chars that are not space or comma (comma is used to combine multiple paths in `FileIndex.toString`). ### Why are the changes needed? make the test framework more stable ### Does this PR introduce _any_ user-facing change? no ### How was this patch tested? N/A Closes apache#39924 from cloud-fan/minor. Authored-by: Wenchen Fan <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]> (cherry picked from commit 04550ed) Signed-off-by: Hyukjin Kwon <[email protected]>
What changes were proposed in this pull request?
The main change of this pr is to fix suggestions of #39598 (comment):
replaceAlltoSQLQueryTestHelper#replaceNotIncludedMsgto remove@hashCode#39598toStringmethod ofFileIndexto print className withrootPathsSQLQueryTestHelper#replaceNotIncludedMsgmethod forfile://xxx/clsName/path to replace one by one instead of replacing the longest matchWhy are the changes needed?
Fix suggestions of #39598 (comment)
Does this PR introduce any user-facing change?
No
How was this patch tested?
build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite" -Pscala-2.13, run successful