MaxGekk commented on a change in pull request #34057:
URL: https://github.com/apache/spark/pull/34057#discussion_r715060791
##########
File path:
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/CommonFileDataSourceSuite.scala
##########
@@ -36,22 +36,25 @@ trait CommonFileDataSourceSuite extends SQLHelper { self:
AnyFunSuite =>
protected def inputDataset: Dataset[_] =
spark.createDataset(Seq("abc"))(Encoders.STRING)
test(s"SPARK-36349: disallow saving of ANSI intervals to $dataSourceFormat")
{
- Seq("INTERVAL '1' DAY", "INTERVAL '1' YEAR").foreach { i =>
- withTempPath { dir =>
- val errMsg = intercept[AnalysisException] {
- spark.sql(s"SELECT
$i").write.format(dataSourceFormat).save(dir.getAbsolutePath)
- }.getMessage
- assert(errMsg.contains("Cannot save interval data type into external
storage"))
+ if (!Set("parquet").contains(dataSourceFormat.toLowerCase(Locale.ROOT))) {
+ Seq("INTERVAL '1' DAY", "INTERVAL '1' YEAR").foreach { i =>
+ withTempPath { dir =>
+ val errMsg = intercept[AnalysisException] {
+ spark.sql(s"SELECT
$i").write.format(dataSourceFormat).save(dir.getAbsolutePath)
+ }.getMessage
+ assert(errMsg.contains("Cannot save interval data type into external
storage"))
+ }
}
- }
- // Check all built-in file-based datasources except of libsvm which
requires particular schema.
- if (!Set("libsvm").contains(dataSourceFormat.toLowerCase(Locale.ROOT))) {
- Seq("INTERVAL DAY TO SECOND", "INTERVAL YEAR TO MONTH").foreach { it =>
- val errMsg = intercept[AnalysisException] {
- spark.sql(s"CREATE TABLE t (i $it) USING $dataSourceFormat")
- }.getMessage
- assert(errMsg.contains("data source does not support"))
+ // Check all built-in file-based datasources except of libsvm which
+ // requires particular schema.
+ if (!Set("libsvm").contains(dataSourceFormat.toLowerCase(Locale.ROOT))) {
+ Seq("INTERVAL DAY TO SECOND", "INTERVAL YEAR TO MONTH").foreach { it =>
+ val errMsg = intercept[AnalysisException] {
+ spark.sql(s"CREATE TABLE t (i $it) USING $dataSourceFormat")
Review comment:
> should we add tests for `CREATE TABLE ...`
Yes, we should but in this PR I focused on low-level support ANSI intervals
in parquet. We should ensure that ANSI interval types are stored properly to
catalogs, for instance. I especially highlighted that in the PR's description:
_The given PR focus on support of ANSI intervals in the Parquet datasource
via write or read as a column in `Dataset`_
After the PR be merged, I plan to open sub-tasks for follow-up work, so,
folks who are interested in this could do that in parallel.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
