Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[SPARK-19611][SQL] Preserve metastore field order when merging inferr…
…ed schema

The ```HiveMetastoreCatalog.mergeWithMetastoreSchema()``` method added in #16944 may
not preserve the same field order as the metastore schema in some cases, which can cause
queries to fail. This change ensures that the metastore field order is preserved.

A test for ensuring that metastore order is preserved was added to ```HiveSchemaInferenceSuite.```
The particular failure usecase from #16944 was tested manually as well.
  • Loading branch information
Budde committed Mar 10, 2017
commit 99eb2dc9b07e953249b4ece6ea3a44ecebc9287b
Original file line number Diff line number Diff line change
Expand Up @@ -356,13 +356,10 @@ private[hive] object HiveMetastoreCatalog {
.filterKeys(!inferredSchema.map(_.name.toLowerCase).contains(_))
.values
.filter(_.nullable)

// Merge missing nullable fields to inferred schema and build a case-insensitive field map.
val inferredFields = StructType(inferredSchema ++ missingNullables)
.map(f => f.name.toLowerCase -> f).toMap
StructType(metastoreFields.map { case(name, field) =>
field.copy(name = inferredFields(name).name)
}.toSeq)
StructType(metastoreSchema.map(f => f.copy(name = inferredFields(f.name).name)))
Copy link
Member

Choose a reason for hiding this comment

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

I see. Now metastoreSchema is used instead of metastoreFields.

Copy link
Author

Choose a reason for hiding this comment

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

This should ensure the proper ordering is used. Iterating over the metastoreFields map isn't guaranteed to maintain the original ordering.

Copy link
Member

Choose a reason for hiding this comment

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

Yep. I'm still building this PR locally, but it looks good logically. :) I'll post my result soon.

} catch {
case NonFatal(_) =>
val msg = s"""Detected conflicting schemas when merging the schema obtained from the Hive
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,27 @@ class HiveSchemaInferenceSuite
StructField("firstField", StringType, nullable = true),
StructField("secondField", StringType, nullable = true))))
}.getMessage.contains("Detected conflicting schemas"))

// Schema merge should maintain metastore order.
assertResult(
StructType(Seq(
StructField("first_field", StringType, nullable = true),
StructField("second_field", StringType, nullable = true),
StructField("third_field", StringType, nullable = true),
StructField("fourth_field", StringType, nullable = true),
StructField("fifth_field", StringType, nullable = true)))) {
HiveMetastoreCatalog.mergeWithMetastoreSchema(
StructType(Seq(
StructField("first_field", StringType, nullable = true),
StructField("second_field", StringType, nullable = true),
StructField("third_field", StringType, nullable = true),
StructField("fourth_field", StringType, nullable = true),
StructField("fifth_field", StringType, nullable = true))),
StructType(Seq(
StructField("fifth_field", StringType, nullable = true),
StructField("third_field", StringType, nullable = true),
StructField("second_field", StringType, nullable = true))))
}
}
}

Expand Down