[SPARK-21767] [TEST] [SQL] Add Decimal Test For Avro in VersionSuite #18977
Conversation
|
|
||
| if (isPartitioned) { | ||
| val insertStmt = s"INSERT OVERWRITE TABLE $tableName partition (ds='a') SELECT 1.3" | ||
| if (version == "0.12" || version == "0.13") { |
There was a problem hiding this comment.
0.13 or prior does not support the logical type Decimal. See https://issues.apache.org/jira/browse/HIVE-5823
There was a problem hiding this comment.
Quick question: I was trying to understand why this depends on the metastore version and realized that in case of schema mismatch (such as DecimalType and Binary here), HiveExternalCatalog always respects the table schema from hive over spark SQL. Is it worth having this limitation for all generalized cases?
There was a problem hiding this comment.
Yes. Our dear Hive metastore change the schema we inferred. We got the warning messages for 0.12 and 0.13
12:04:09.816 WARN org.apache.spark.sql.hive.test.TestHiveExternalCatalog: The table schema given by Hive metastore(structf0:binary,ds:string) is different from the schema when this table was created by Spark SQL(structf0:decimal(38,2),ds:string). We have to fall back to the table schema from Hive metastore which is not case preserving.
There was a problem hiding this comment.
Will submit a separate PR for adding a conf to ignore the schema overwritten by hive metastore.
| } | ||
| } | ||
|
|
||
| test(s"$version: Decimal support of Avro Hive serde") { |
There was a problem hiding this comment.
We should consider generalizing this test for all supported avro logical types: https://avro.apache.org/docs/1.8.1/spec.html#Logical+Types. Perhaps add a TODO?
There was a problem hiding this comment.
Yes. Let me post a todo there.
|
Test build #80800 has finished for PR 18977 at commit
|
|
Thanks, LGTM! |
|
Test build #80808 has finished for PR 18977 at commit
|
|
Thanks! Merging to master. |
3 participants
What changes were proposed in this pull request?
Decimal is a logical type of AVRO. We need to ensure the support of Hive's AVRO serde works well in Spark
How was this patch tested?
N/A