You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/08 08:23:25 UTC
[GitHub] [spark] wangyum opened a new pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
wangyum opened a new pull request #32090:
URL: https://github.com/apache/spark/pull/32090
### What changes were proposed in this pull request?
This pr makes it support the data is written as integral type and read as decimal type with 0 scala. For example:
```scala
import org.apache.spark.sql.types.StructType
spark.sql("SELECT 1L AS a").write.parquet("/tmp/SPARK-34212")
spark.read.schema(StructType.fromDDL("a decimal(18, 0)")).parquet("/tmp/SPARK-34212").show
```
### Why are the changes needed?
It is supported before SPARK-34212.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
--
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.
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] wangyum edited a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum edited a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-817106123
I think we should not support this case. It has potential data issues. For example:
```scala
import org.apache.spark.sql.types.StructType
spark.sql("SELECT 999999999 AS a").write.mode("overwrite").parquet("/tmp/SPARK-34212")
val df = spark.read.schema(StructType.fromDDL("a decimal(2, 0)")).parquet("/tmp/SPARK-34212")
df.write.saveAsTable("t1")
spark.sql("select a, a * a as a2, a * a * a as a3, a * a * a * a as a4 from t1").write.saveAsTable("t2")
spark.sql("select * from t2").show(false)
spark.sql("desc t2").show(false)
```
```
+---------+----+----+----+
|a |a2 |a3 |a4 |
+---------+----+----+----+
|999999999|null|null|null|
+---------+----+----+----+
+--------+-------------+-------+
|col_name|data_type |comment|
+--------+-------------+-------+
|a |decimal(2,0) |null |
|a2 |decimal(5,0) |null |
|a3 |decimal(8,0) |null |
|a4 |decimal(11,0)|null |
+--------+-------------+-------+
```
--
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.
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] cloud-fan edited a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818576796
thanks, merging to master/3.1/3.0/2.4!
--
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.
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] AmplabJenkins removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815639626
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41646/
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818387454
**[Test build #137262 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137262/testReport)** for PR 32090 at commit [`d182f65`](https://github.com/apache/spark/commit/d182f659d2d0f82a2db7783f1dc2fbc3fcf6781f).
--
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.
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] cloud-fan commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609867914
##########
File path: sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedColumnReader.java
##########
@@ -121,17 +120,19 @@ private boolean isDecimalTypeMatched(DataType dt) {
private boolean canReadAsIntDecimal(DataType dt) {
if (!DecimalType.is32BitDecimalType(dt)) return false;
- return isDecimalTypeMatched(dt);
+ DecimalType d = (DecimalType) dt;
+ return isDecimalTypeMatched(d) || d.scale() == 0;
Review comment:
`!isDecimalTypeMatched(d)` doesn't mean `DecimalMetadata` is null. Shall we put the `|| d.scale() == 0` in `isDecimalTypeMatched`?
--
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.
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] AmplabJenkins commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815639626
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41646/
--
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.
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] viirya edited a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
viirya edited a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-819725770
Hi, as 2.4.8 RC2 is under voting now. I'd like to confirm that we don't want to support the case in 2.4.8, right? This PR just moves the test case. cc @wangyum @cloud-fan @dongjoon-hyun
--
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.
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] AmplabJenkins commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818522346
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137262/
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815831505
**[Test build #137068 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137068/testReport)** for PR 32090 at commit [`4fdb0aa`](https://github.com/apache/spark/commit/4fdb0aaa1ceba3dc637bd75f07d0807382b56ed3).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815636833
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41646/
--
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.
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] viirya commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815986411
> is this a regression of 2.4? cc @viirya
Yes. I can confirm that before SPARK-34212, the example query works in 2.4.
--
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.
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] viirya edited a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
viirya edited a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-819725770
Hi, as 2.4.8 RC2 is under voting now. I'd like to confirm that we don't want to support the case in 2.4.8, right? This PR just moves the test case. cc @wangyum @cloud-fan
--
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.
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] MaxGekk commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609551326
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
+ val df = sql("SELECT 11 a, 22L b")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ val schema = "a DECIMAL(9, 0), b DECIMAL(18, 0)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withAllParquetReaders {
+ val schema = "a DECIMAL(38, 0), b DECIMAL(38, 0)"
+ if (conf.parquetVectorizedReaderEnabled) {
Review comment:
Is it a fundamental issue or this can be fixed in the Vectorized reader? If so, could open a JIRA and add TODO here, please.
--
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.
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] AmplabJenkins removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818428810
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41841/
--
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.
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] cloud-fan edited a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-817504276
@wangyum I think we can still support some cases? For example, if the parquet field is int type, we can read it as `decimal(p, s)` where `p - s >= Decimal.MAX_INT_DIGITS`
--
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.
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] AmplabJenkins removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815596451
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41643/
--
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.
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] wangyum commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818378906
Many places need to be changed to support this case:
https://github.com/apache/spark/blob/a418548dad57775fbb10b4ea690610bad1a8bfb0/sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/WritableColumnVector.java#L364-L367
https://github.com/apache/spark/blob/faeb71b39d746afdc29f154e293e7c09871c1254/sql/core/src/main/java/org/apache/spark/sql/execution/vectorized/OnHeapColumnVector.java#L543-L549
https://github.com/apache/spark/blob/5013171fd36e6221a540c801cb7fd9e298a6b5ba/sql/core/src/main/java/org/apache/spark/sql/execution/datasources/parquet/VectorizedColumnReader.java#L119-L122
--
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.
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] cloud-fan edited a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
cloud-fan edited a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818576796
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.
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] AmplabJenkins removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818522346
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137262/
--
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.
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] cloud-fan commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818403148
Ah I see, the so-called `32BitDecimalType` can't hold Int.Max, so it's very difficult to support reading a plain int as decimal. The same to `64BitDecimalType`.
What we can support is reading plain int as `64BitDecimalType`, which may need more changes.
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818520781
**[Test build #137262 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137262/testReport)** for PR 32090 at commit [`d182f65`](https://github.com/apache/spark/commit/d182f659d2d0f82a2db7783f1dc2fbc3fcf6781f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] AmplabJenkins removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815813616
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137065/
--
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.
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] AmplabJenkins commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818428810
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41841/
--
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.
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] MaxGekk commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609458964
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
##########
@@ -3944,6 +3944,23 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
}
+
+ // scala is 0
Review comment:
scala -> scale
--
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.
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] wangyum commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609480878
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
##########
@@ -3944,6 +3944,23 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
}
+
+ // scala is 0
Review comment:
fixed
--
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.
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] wangyum closed pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum closed pull request #32090:
URL: https://github.com/apache/spark/pull/32090
--
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.
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] wangyum commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-817782173
It may be out of range?
```
scala> spark.sql(s"select cast(${Int.MaxValue} as decimal(9, 0)), cast(${Long.MaxValue} as decimal(18, 0))").show(false)
+--------------------------------+------------------------------------------+
|CAST(2147483647 AS DECIMAL(9,0))|CAST(9223372036854775807 AS DECIMAL(18,0))|
+--------------------------------+------------------------------------------+
|null |null |
+--------------------------------+------------------------------------------+
scala> spark.sql(s"select cast(${Int.MaxValue} as decimal(10, 0)), cast(${Long.MaxValue} as decimal(19, 0))").show(false)
+---------------------------------+------------------------------------------+
|CAST(2147483647 AS DECIMAL(10,0))|CAST(9223372036854775807 AS DECIMAL(19,0))|
+---------------------------------+------------------------------------------+
|2147483647 |9223372036854775807 |
+---------------------------------+------------------------------------------+
```
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815563278
**[Test build #137065 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137065/testReport)** for PR 32090 at commit [`fda77c7`](https://github.com/apache/spark/commit/fda77c7501f92410a630810b7d287076c4bb88e5).
--
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.
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] cloud-fan commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609870209
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
+ val df = sql("SELECT 11 a, 22L b")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ val schema = "a DECIMAL(9, 0), b DECIMAL(18, 0)"
Review comment:
does the decimal precision matter? what if we read Int.Max as decimal(1, 0)?
--
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.
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] SparkQA removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815563278
**[Test build #137065 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137065/testReport)** for PR 32090 at commit [`fda77c7`](https://github.com/apache/spark/commit/fda77c7501f92410a630810b7d287076c4bb88e5).
--
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.
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] AmplabJenkins removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815859381
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137068/
--
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.
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] MaxGekk commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609460743
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
##########
@@ -3944,6 +3944,23 @@ class SQLQuerySuite extends QueryTest with SharedSparkSession with AdaptiveSpark
}
}
}
+
+ // scala is 0
+ withTempPath { path =>
+ val df = sql("SELECT 11 a, 22L b")
+ df.write.parquet(path.toString)
+ val schema1 = "a DECIMAL(9, 0), b DECIMAL(18, 0)"
+ val schema2 = "a DECIMAL(38, 0), b DECIMAL(38, 0)"
+ Seq(true, false).foreach { vectorized =>
Review comment:
Please, use `withAllParquetReaders`.
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815592568
--
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.
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] AmplabJenkins commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815859381
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137068/
--
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.
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] dongjoon-hyun commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815927450
Thank you, @wangyum .
--
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.
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] AmplabJenkins commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815813616
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/137065/
--
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.
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] dongjoon-hyun commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609841651
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
Review comment:
Yes, it looks like that.
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815606196
**[Test build #137068 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137068/testReport)** for PR 32090 at commit [`4fdb0aa`](https://github.com/apache/spark/commit/4fdb0aaa1ceba3dc637bd75f07d0807382b56ed3).
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815639592
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41646/
--
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.
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] viirya commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
viirya commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-819725770
Hi, as 2.4.8 RC2 is under voting now. I'd like to confirm that we don't want to support the case, right? This PR just moves the test case. cc @wangyum @cloud-fan
--
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.
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] AmplabJenkins commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815596451
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/41643/
--
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.
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] wangyum commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-817106101
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818412040
--
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.
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] wangyum commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r611020598
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
+ val df = sql("SELECT 11 a, 22L b")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ val schema = "a DECIMAL(9, 0), b DECIMAL(18, 0)"
Review comment:
Supporting this scenario has potential data issues.
--
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.
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] SparkQA commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815802320
**[Test build #137065 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137065/testReport)** for PR 32090 at commit [`fda77c7`](https://github.com/apache/spark/commit/fda77c7501f92410a630810b7d287076c4bb88e5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
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] dongjoon-hyun commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609844974
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
Review comment:
If you don't mind, please make a separate test case, @wangyum . The existing test case is already a little too long.
--
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.
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] dongjoon-hyun commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609844974
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
Review comment:
If you don't mind, please make a separate test case, @wangyum . The test case is already a little too long.
--
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.
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] cloud-fan commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818576796
thanks, merging to master/3.1/3.0!
--
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.
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] cloud-fan commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-817504276
@wangyum I think we can still support some cases? For example, if the parquet field is int type, we can read it as `decimal(p, s)` where `p - s > Decimal.MAX_INT_DIGITS`
--
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.
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] SparkQA removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815606196
**[Test build #137068 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137068/testReport)** for PR 32090 at commit [`4fdb0aa`](https://github.com/apache/spark/commit/4fdb0aaa1ceba3dc637bd75f07d0807382b56ed3).
--
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.
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] SparkQA removed a comment on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-818387454
**[Test build #137262 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137262/testReport)** for PR 32090 at commit [`d182f65`](https://github.com/apache/spark/commit/d182f659d2d0f82a2db7783f1dc2fbc3fcf6781f).
--
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.
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] cloud-fan closed pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #32090:
URL: https://github.com/apache/spark/pull/32090
--
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.
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] cloud-fan commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-817848186
correction: `p - s > Decimal.MAX_INT_DIGITS`
--
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.
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] cloud-fan commented on pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #32090:
URL: https://github.com/apache/spark/pull/32090#issuecomment-815906581
is this a regression of 2.4? cc @viirya
--
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.
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] wangyum commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
wangyum commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r610181431
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
Review comment:
ok
--
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.
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] MaxGekk commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
MaxGekk commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609549838
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
+ withTempPath { path =>
+ val df = sql(s"SELECT 1 a, 123456 b, ${Int.MaxValue.toLong * 10} c, CAST('1.2' AS BINARY) d")
+ df.write.parquet(path.toString)
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ checkAnswer(readParquet("a DECIMAL(3, 2)", path), sql("SELECT 1.00"))
+ checkAnswer(readParquet("b DECIMAL(3, 2)", path), Row(null))
+ checkAnswer(readParquet("b DECIMAL(11, 1)", path), sql("SELECT 123456.0"))
+ checkAnswer(readParquet("c DECIMAL(11, 1)", path), Row(null))
+ checkAnswer(readParquet("c DECIMAL(13, 0)", path), df.select("c"))
+ val e = intercept[SparkException] {
+ readParquet("d DECIMAL(3, 2)", path).collect()
+ }.getCause
+ assert(e.getMessage.contains("Please read this column/field as Spark BINARY type"))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "c DECIMAL(18, 1)", "d DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // scale is 0
+ withTempPath { path =>
Review comment:
This is only new part, correct? You didn't change the code above, I guess or I am wrong?
--
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.
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] dongjoon-hyun commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Support reading data when DecimalMetadata is null
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r609840740
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,90 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
Review comment:
+1 for moving the test case to this suite.
--
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.
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] cloud-fan commented on a change in pull request #32090: [SPARK-34212][SQL][FOLLOWUP] Move the added test to ParquetQuerySuite
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #32090:
URL: https://github.com/apache/spark/pull/32090#discussion_r613738091
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetQuerySuite.scala
##########
@@ -840,6 +840,67 @@ abstract class ParquetQuerySuite extends QueryTest with ParquetTest with SharedS
testMigration(fromTsType = "INT96", toTsType = "TIMESTAMP_MICROS")
testMigration(fromTsType = "TIMESTAMP_MICROS", toTsType = "INT96")
}
+
+ test("SPARK-34212 Parquet should read decimals correctly") {
+ def readParquet(schema: String, path: File): DataFrame = {
+ spark.read.schema(schema).parquet(path.toString)
+ }
+
+ withTempPath { path =>
+ // a is int-decimal (4 bytes), b is long-decimal (8 bytes), c is binary-decimal (16 bytes)
+ val df = sql("SELECT 1.0 a, CAST(1.23 AS DECIMAL(17, 2)) b, CAST(1.23 AS DECIMAL(36, 2)) c")
+ df.write.parquet(path.toString)
+
+ withAllParquetReaders {
+ // We can read the decimal parquet field with a larger precision, if scale is the same.
+ val schema = "a DECIMAL(9, 1), b DECIMAL(18, 2), c DECIMAL(38, 2)"
+ checkAnswer(readParquet(schema, path), df)
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "false") {
+ val schema1 = "a DECIMAL(3, 2), b DECIMAL(18, 3), c DECIMAL(37, 3)"
+ checkAnswer(readParquet(schema1, path), df)
+ val schema2 = "a DECIMAL(3, 0), b DECIMAL(18, 1), c DECIMAL(37, 1)"
+ checkAnswer(readParquet(schema2, path), Row(1, 1.2, 1.2))
+ }
+
+ withSQLConf(SQLConf.PARQUET_VECTORIZED_READER_ENABLED.key -> "true") {
+ Seq("a DECIMAL(3, 2)", "b DECIMAL(18, 1)", "c DECIMAL(37, 1)").foreach { schema =>
+ val e = intercept[SparkException] {
+ readParquet(schema, path).collect()
+ }.getCause.getCause
+ assert(e.isInstanceOf[SchemaColumnConvertNotSupportedException])
+ }
+ }
+ }
+
+ // tests for parquet types without decimal metadata.
Review comment:
@viirya looking at the test, I think it was decided before that reading plain int/long as decimal is hard to implement in vectorized reader.
Basically we need to do 2 steps:
1. read the decimal from int/long as its actual precision/scale. Since it's a plain int/long, the precision should be max precision for int/long.
2. cast the decimal to the required precision/scale.
For vectorized reader, we can create a `Decimal` object with max precision for int/long, do the cast, and set the int/long to the vector if there is no overflow. This is super slow, but is still doable.
It's not a real regression, as @wangyum demonstrated before, the previous behavior in 2.4 was not reasonable when overflow happens.
--
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.
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