-
Notifications
You must be signed in to change notification settings - Fork 300
Alias and Doc support in Spark Schema #51
base: master
Are you sure you want to change the base?
Changes from 4 commits
ee40539
7c27654
a72693d
5ce1b3a
f16d13e
d8e7759
01cd427
c08017b
a2085b6
04dfa9a
8cfe0e6
4e9e5b0
34c2505
8cb303a
a89c385
702e9f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,6 +127,14 @@ To save DataFrame as avro you should use the `save` method in `AvroSaver`. For e | |
| ```scala | ||
| scala> AvroSaver.save(myRDD, "my/output/dir") | ||
| ``` | ||
|
|
||
| To include aliases column in scheme invoke the method `addAvroAliasColumns()` of DataFrame. | ||
| With alias while saving `saveAsAvroFile`, alias columns in DataFrame schema will not be include in Avro file. | ||
| Alias will be available in `aliases` of avro schema. | ||
| ```scala | ||
| scala>val dfWithAlias = df.addAvroAliasColumns() | ||
| ``` | ||
|
|
||
|
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 should also include examples of how to set alias and docs.
Author
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. @marmbrus user can set alias and docs to a avro using avro tools. Not using spark-avro |
||
| You can also specifiy the the record name and namespace with optional parameters: | ||
| ```scala | ||
| scala> AvroSaver.save(myRDD, "my/output/dir", Map("recordName" -> "MyRecord", "recordNamespace" -> "com.mycompany.mystuff")) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,7 +158,12 @@ object AvroSaver { | |
|
|
||
| while (convertersIterator.hasNext) { | ||
| val converter = convertersIterator.next() | ||
| record.put(fieldNamesIterator.next(), converter(rowIterator.next())) | ||
| val fieldName = fieldNamesIterator.next() | ||
| if(schema.getField(fieldName) != null) { | ||
|
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. Style nit: space after |
||
| record.put(fieldName, converter(rowIterator.next())) | ||
| } else { | ||
| rowIterator.next() | ||
| } | ||
| } | ||
| record | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,12 @@ | |
| package com.databricks.spark.avro | ||
|
|
||
| import scala.collection.JavaConversions._ | ||
|
|
||
| import org.apache.avro.{Schema, SchemaBuilder} | ||
| import org.apache.avro.SchemaBuilder._ | ||
|
|
||
| import org.apache.spark.sql.types._ | ||
| import org.apache.avro.Schema.Type._ | ||
| import org.apache.spark.sql.DataFrame | ||
| import util.control.Breaks._ | ||
|
|
||
| /** | ||
| * This object contains method that are used to convert sparkSQL schemas to avro schemas and vice | ||
|
|
@@ -49,7 +49,15 @@ private object SchemaConverters { | |
| case RECORD => | ||
| val fields = avroSchema.getFields.map { f => | ||
| val schemaType = toSqlType(f.schema()) | ||
| StructField(f.name, schemaType.dataType, schemaType.nullable) | ||
| var meta = new MetadataBuilder() | ||
| if (f.doc != null) meta.putString("_doc", f.doc) | ||
| if(f.aliases() != null && f.aliases().size() > 0) { | ||
|
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. Style nit: space after |
||
| val aliasArray = new Array[String](f.aliases().size()) | ||
| meta.putString("_parent", f.name) | ||
| f.aliases copyToArray(aliasArray) | ||
| meta.putStringArray("_aliases", aliasArray); | ||
| } | ||
| StructField(f.name, schemaType.dataType, schemaType.nullable, meta.build()) | ||
| } | ||
|
|
||
| SchemaType(StructType(fields), nullable = false) | ||
|
|
@@ -88,6 +96,19 @@ private object SchemaConverters { | |
| } | ||
| } | ||
|
|
||
| def dataFrameWithAliasColumn(df : DataFrame) : DataFrame = { | ||
|
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. Is this function still needed? or is this redundant with the above listing of field names ++ aliases? |
||
| var newDf = df | ||
| for(field <- df.schema.fields) { | ||
|
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. Style nit: space after |
||
| if (field.metadata.contains("_aliases")) { | ||
| val aliasArray = field.metadata.getStringArray("_aliases") | ||
| for (alias <- aliasArray) { | ||
| newDf = newDf.withColumn(alias, df.col(field.name)) | ||
| } | ||
| } | ||
| } | ||
| newDf | ||
| } | ||
|
|
||
| /** | ||
| * This function converts sparkSQL StructType into avro schema. This method uses two other | ||
| * converter methods in order to do the conversion. | ||
|
|
@@ -98,14 +119,26 @@ private object SchemaConverters { | |
| recordNamespace: String): T = { | ||
| val fieldsAssembler: FieldAssembler[T] = schemaBuilder.fields() | ||
| structType.fields.foreach { field => | ||
| val newField = fieldsAssembler.name(field.name).`type`() | ||
|
|
||
| if (field.nullable) { | ||
| convertFieldTypeToAvro(field.dataType, newField.nullable(), field.name, recordNamespace) | ||
| .noDefault | ||
| } else { | ||
| convertFieldTypeToAvro(field.dataType, newField, field.name, recordNamespace) | ||
| .noDefault | ||
| breakable { | ||
|
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. Would it be more readable to just make it "if ... else" statement instead of "breakable" and "break"?
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. +1
Author
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. used breakable for moving next element. primary checks before creating the fields. I feel this is simple than writing complex if condition check. I can modify if required.
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. Yes, it is nice to check them before. But as far as simplicity and readability goes... breakable {
if (predicate) break
body
}is equivalent to if (!predicate) {
body
}Hence is is shorter, cleaner and other folks do not need to know what breakable is. Right? BTW: the code here can be written as... val nonAliasStructFields = structType.fields.filterNot(field =>
field.metadata.contains(METADATA_KEY_PARENT) && !field.metadata.getString(METADATA_KEY_PARENT).equals(field.name))
nonAliasStructFields.foreach { field =>
var newFieldBuilder = fieldsAssembler.name(field.name)
... |
||
| if (field.metadata.contains("_aliases") && field.metadata.contains("_parent") | ||
| && !field.metadata.getString("_parent").equals(field.name)) { | ||
| break | ||
| } | ||
| var newFieldBuilder = fieldsAssembler.name(field.name) | ||
| if (field.metadata contains ("_doc")) { | ||
| newFieldBuilder = newFieldBuilder.doc(field.metadata.getString("_doc")) | ||
| } | ||
| if (field.metadata.contains("_aliases")){ | ||
| newFieldBuilder = newFieldBuilder.aliases(field.metadata.getStringArray("_aliases"): _*) | ||
| } | ||
| val newField = newFieldBuilder.`type`() | ||
| if (field.nullable) { | ||
| convertFieldTypeToAvro(field.dataType, newField.nullable(), field.name, recordNamespace) | ||
| .noDefault | ||
| } else { | ||
| convertFieldTypeToAvro(field.dataType, newField, field.name, recordNamespace) | ||
| .noDefault | ||
| } | ||
| } | ||
| } | ||
| fieldsAssembler.endRecord() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ package object avro { | |
| implicit class AvroContext(sqlContext: SQLContext) { | ||
| def avroFile(filePath: String, minPartitions: Int = 0) = | ||
| sqlContext.baseRelationToDataFrame(AvroRelation(filePath, None, minPartitions)(sqlContext)) | ||
|
|
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -35,5 +36,8 @@ package object avro { | |
| path: String, | ||
| parameters: Map[String, String] = AvroSaver.defaultParameters): Unit = | ||
| AvroSaver.save(dataFrame, path, parameters) | ||
|
|
||
| def addAvroAliasColumns() : DataFrame = | ||
| SchemaConverters.dataFrameWithAliasColumn(dataFrame) | ||
| } | ||
|
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. Can we get rid of this entirely? I'd prefer to only have to support a single unified way to do this. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -443,4 +443,92 @@ class AvroSuite extends FunSuite { | |
| assert(newDf.count == 8) | ||
| } | ||
|
|
||
| test("test doc in meta") { | ||
| val df = TestSQLContext.load(episodesFile, "com.databricks.spark.avro") | ||
| df.schema.fields(0).metadata.getString("_doc") | ||
|
|
||
| for(x <- df.schema.fields) { | ||
|
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. More style nits: always add a space after |
||
| if(x.name == "title") { | ||
| assert("episode title" == x.metadata.getString("_doc")) | ||
| } else if(x.name == "doctor") { | ||
| assert("main actor playing the Doctor in episode" == x.metadata.getString("_doc")) | ||
| } else if(x.name == "air_date") { | ||
| assert("initial date" == x.metadata.getString("_doc")) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| test("test aliases in meta") { | ||
| val df = TestSQLContext.load(testFile, "com.databricks.spark.avro") | ||
|
|
||
| for (x <- df.schema.fields) { | ||
|
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 guess people will believe the metadata are not type dependent. Hence you can put them only on one field in test file - let's say "string" and then this whole test can become val df = TestSQLContext.load(testFile, "com.databricks.spark.avro")
assert(df.schema("string").getStringArray(SchemaConverters.METADATA_KEY_ALIASES) === Array("string_alias1", "string_alias1")Would that make it easier? |
||
| if (x.name == "string") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 2) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "string_alias1") | ||
| assert(x.metadata.getStringArray("_aliases")(1) == "string_alias2") | ||
| } else if (x.name == "simple_map") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "map_alias") | ||
| } else if (x.name == "complex_map") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "complex_map_alias") | ||
| } else if (x.name == "union_string_null") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "union_string_alias") | ||
| } else if (x.name == "union_int_long_null") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "union_int_alias") | ||
| } else if (x.name == "union_float_double") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 2) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "union_float_alias1") | ||
| assert(x.metadata.getStringArray("_aliases")(1) == "union_float_alias2") | ||
| } else if (x.name == "fixed3") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "fixed3_alias") | ||
| } else if (x.name == "enum") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "enum_alias") | ||
| } else if (x.name == "value_field") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "value_field_alias") | ||
| } else if (x.name == "array_of_boolean") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "array_of_boolean_alias") | ||
| } else if (x.name == "bytes") { | ||
| assert(x.metadata.contains("_aliases")) | ||
| assert(x.metadata.getStringArray("_aliases").size == 1) | ||
| assert(x.metadata.getStringArray("_aliases")(0) == "bytes_alias") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| test("test aliases columns in data frame") { | ||
| var df = TestSQLContext.load(testFile, "com.databricks.spark.avro") | ||
| var fieldArray = df.schema.fieldNames; | ||
|
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. break these into two tests instead of using a |
||
| assert(fieldArray contains("string")) | ||
| assert(!(fieldArray contains("string_alias1"))) | ||
|
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. Either use infix notation or leave the |
||
| assert(!(fieldArray contains("string_alias2"))) | ||
| assert(!(fieldArray contains("map_alias"))) | ||
| assert(!(fieldArray contains("enum_alias"))) | ||
| assert(!(fieldArray contains("union_int_alias"))) | ||
|
|
||
| fieldArray = SchemaConverters.dataFrameWithAliasColumn(df).schema.fieldNames | ||
|
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. this test should probably use the user visible option instead of diving into internals. |
||
| assert(fieldArray contains("string")) | ||
| assert(fieldArray contains("string_alias1")) | ||
| assert(fieldArray contains("string_alias2")) | ||
| assert(fieldArray contains("map_alias")) | ||
| assert(fieldArray contains("enum_alias")) | ||
| assert(fieldArray contains("union_int_alias")) | ||
| } | ||
|
|
||
| } | ||
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.
Is it stored as "aliases" or "_aliases"?