Skip to content

Conversation

@gengliangwang
Copy link
Member

What changes were proposed in this pull request?

Since https://github.com/apache/spark/pull/23383/files#diff-db4a140579c1ac4b1dbec7fe5057eecaR36, the exception message of schema inference failure in file source V2 is tableName, which is equivalent to shortName + path.

While in file source V1, the message is Unable to infer schema from ORC/CSV/JSON....
We should make the message in V2 consistent with V1, so that in the future migration the related test cases don't need to be modified. #24058 (review)

How was this patch tested?

Revert the modified unit test cases in https://github.com/apache/spark/pull/24005/files#diff-b9ddfbc9be8d83ecf100b3b8ff9610b9R431 and https://github.com/apache/spark/pull/23383/files#diff-9ab56940ee5a53f2bb81e3c008653362R577, and test with them.

@gengliangwang
Copy link
Member Author

@cloud-fan @felixcheung

testRead(spark.read.csv(), Seq.empty, schema)
}.getMessage
assert(message.toLowerCase(Locale.ROOT).contains("unable to infer schema for csv"))
assert(message.contains("Unable to infer schema for CSV. It must be specified manually."))
Copy link
Member

Choose a reason for hiding this comment

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

why remove toLowerCase(Locale.ROOT)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because both V1 and V2 show exactly the same message.
toLowerCase(Locale.ROOT) was added in the migration of CSV V2 https://github.com/apache/spark/pull/24005/files#diff-b9ddfbc9be8d83ecf100b3b8ff9610b9R431

@SparkQA
Copy link

SparkQA commented Apr 14, 2019

Test build #104576 has finished for PR 24369 at commit 7eaae34.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 27d625d Apr 15, 2019
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.

4 participants