-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-16948][SQL] Use metastore schema instead of inferring schema for ORC in HiveMetastoreCatalog #14537
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
[SPARK-16948][SQL] Use metastore schema instead of inferring schema for ORC in HiveMetastoreCatalog #14537
Changes from 1 commit
5721b88
4ae92d8
75bca1b
97d21f6
4004c0a
9a8838a
eb8a955
70cf84d
c9d677b
7385a06
1746853
5f7e5ad
e8e2d70
0f901aa
0772a96
6ff7e5d
fc14e2d
9ecb2ed
fa71370
e39715e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
…tion
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -238,7 +238,13 @@ private[hive] class HiveMetastoreCatalog(sparkSession: SparkSession) extends Log | |
| partitionSpec) | ||
|
|
||
| val schema = fileType match { | ||
| case "orc" | "parquet" => | ||
| case "parquet" => | ||
| val inferredSchema = | ||
| defaultSource.inferSchema(sparkSession, options, fileCatalog.allFiles()) | ||
| inferredSchema.map { inferred => | ||
| ParquetFileFormat.mergeMetastoreParquetSchema(metastoreSchema, inferred) | ||
| }.getOrElse(metastoreSchema) | ||
| case "orc" => | ||
| metastoreSchema | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went through the code path again, seems we must infer the schema here. In metastore, we store the table schema and partition columns.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cloud-fan As we discussed offline yesterday, this is probably fine since ORC supports column pruning. Therefore, when reading an ORC file in a partitioned table, the reader always ignores partition columns stored inside the physical file and uses the value encoded in partition directory path.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have a test case for this case here. |
||
| case _ => | ||
| throw new RuntimeException(s"Cannot convert a $fileType to a data source table") | ||
|
|
||
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.
I'm a little worried here. If the table is partitioned,
metastoreSchemawill always contain partition columns, and thus the merged schema will contain partition columns too. This means, we always read parquet files with partition columns, I think we may have a hidden bug.