Alias and Doc support in Spark Schema #51

vasanthrj · 2015-05-30T19:59:27Z

Preserves avro doc and aliases while save
Include avro alias column in DataFrame schema

vasanthrj · 2015-06-10T10:52:08Z

Can someone review this? Please give some suggestion.

jendap · 2015-07-19T19:53:11Z

README.md

+
+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.


Is it stored as "aliases" or "_aliases"?

jendap · 2015-07-19T20:20:58Z

I love the idea! And it seems to be working just fine.

A few suggestions...

Let's ask @marmbrus if "_doc" and "_aliases" are a good naming convention. It would be nice to make it consistent with other formats supporting it.
Is the implicit addAvroAliasColumns a good choice? It would be on every DataFrame - even after modifying columns and wiping out their metadata. Would it make sense to make it a property on SQLContext the same way as parquet settings are done currently?
It would be nice to add an assert to the place where files are read after saving. Check that doc and aliases made the loop to disk and back.
Would it not be enough and more readable to add the doc and alias on one field in the test only? It does not have to be tested on all the different fields, right? It would save some 100 lines of code in tests ;-)

JoshRosen · 2015-07-20T19:41:57Z

src/main/scala/com/databricks/spark/avro/AvroSaver.scala

@@ -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) {


Style nit: space after if.

marmbrus · 2015-07-20T19:45:25Z

Thanks for working on this.

I'd consider making the addition of the alias columns just an option to the datasource, so that it is usable in languages other than scala.
Other users of metadata don't use _ so i'd just go with doc and aliases

vasanthrj · 2015-07-22T19:46:44Z

@jendap Implemented review comments
Suggestion reply:
2. I added to dataframe since dataframe know the schema and can store metadata.Where aliases/doc are belong to a column data. I think parquet properties are related to deserialization, data type handling.

@JoshRosen Implemented review comments
@marmbrus adding alias column is optional where alias columns added after invoking addAvroAliasColumns()

Build is failing due to some other reason. Kindly check

jendap · 2015-07-24T22:40:23Z

src/main/scala/com/databricks/spark/avro/SchemaConverters.scala

@@ -29,6 +29,9 @@ import org.apache.avro.Schema.Type._
 */
 private object SchemaConverters {

+  val METADATA_KEY_DOC = "doc";
+  val METADATA_KEY_ALIASES = "aliases";


this is good, you can put even the "_parent" here as a constant

vasanthrj · 2015-07-26T04:02:05Z

@jendap Implemented your review comments. Please review.

codecov-io · 2015-07-26T04:17:48Z

Current coverage is `93.79%`

Merging #51 into master will increase coverage by +0.39% as of ccc433e

@@            master     #51   diff @@
======================================
  Files            6       6       
  Stmts          273     306    +33
  Branches        45      57    +12
  Methods          0       0       
======================================
+ Hit            255     287    +32
  Partial          0       0       
- Missed          18      19     +1

Review entire Coverage Diff as of ccc433e

Powered by Codecov. Updated on successful CI builds.

marmbrus · 2015-07-27T19:25:12Z

@marmbrus adding alias column is optional where alias columns added after invoking addAvroAliasColumns()

I realize its optional, but implementing this feature as a scala function means that users in Python and R will not be able to use it. I think instead this should just be a parameter that is passed in the OPTIONS clause or as an option(...) so that it works across languages.

vasanthrj · 2015-08-16T14:19:50Z

@marmbrus Implemented review comments. Added a option. Please review

marmbrus · 2015-08-17T19:22:23Z

src/main/scala/com/databricks/spark/avro/package.scala

+   */
+  implicit class AvroDataFrame(dataFrame: DataFrame) {
+    def addAvroAliasColumns(): DataFrame = SchemaConverters.dataFrameWithAliasColumn(dataFrame)
+  }


Can we get rid of this entirely? I'd prefer to only have to support a single unified way to do this.

marmbrus · 2015-08-25T19:00:26Z

README.md

+
+Avro schema's field 'doc' will be preserved in DataFrame's schema metadata.
+
+To include aliases column in scheme invoke


"To include extra columns when aliases are specified in the schema, set withAlias to true, as follows:"

I would also call the property withAliases, since we handle more than one.

marmbrus · 2015-08-25T19:19:47Z

Thanks again for working on this! I did another review pass.

vasanthrj · 2015-09-09T20:43:49Z

@marmbrus implemented review comments. Except the usage of "withAliasesFields" of in buildScan(...) of AvroRelation.scala, I was getting:
object not serializable (class: org.apache.avro.Schema$RecordSchema,

gasparms · 2017-11-22T14:10:20Z

What happened with this PR? Are you planning include this in future releases? It would be useful support aliases.

Vasanth kumar added 4 commits May 26, 2015 00:24

support for doc and alias

ee40539

DataFrame schema with aliases

7c27654

Fixes

a72693d

Test case update

5ce1b3a

jendap reviewed Jul 19, 2015
View reviewed changes

JoshRosen reviewed Jul 20, 2015
View reviewed changes

Jagadeesh added 2 commits July 22, 2015 23:29

Merge remote-tracking branch 'upstream/master'

f16d13e

aliases/doc impl

d8e7759

jendap reviewed Jul 24, 2015
View reviewed changes

aliases/doc support

01cd427

aliases/doc support

c08017b

vasanth kumar added 4 commits August 15, 2015 21:07

support for doc and alias

a2085b6

support for doc and alias

04dfa9a

support for doc and alias

8cfe0e6

support for doc and alias

4e9e5b0

marmbrus reviewed Aug 17, 2015
View reviewed changes

support for doc and alias

34c2505

marmbrus reviewed Aug 25, 2015
View reviewed changes

vasanth kumar added 3 commits September 10, 2015 00:04

support for doc and alias

8cb303a

support for doc and alias

a89c385

support for doc and alias

702e9f7

JoshRosen added the enhancement label Nov 21, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alias and Doc support in Spark Schema #51

Alias and Doc support in Spark Schema #51

vasanthrj commented May 30, 2015

vasanthrj commented Jun 10, 2015

jendap Jul 19, 2015

jendap commented Jul 19, 2015

JoshRosen Jul 20, 2015

marmbrus commented Jul 20, 2015

vasanthrj commented Jul 22, 2015

jendap Jul 24, 2015

vasanthrj commented Jul 26, 2015

codecov-io commented Jul 26, 2015

marmbrus commented Jul 27, 2015

vasanthrj commented Aug 16, 2015

marmbrus Aug 17, 2015

marmbrus Aug 25, 2015

marmbrus commented Aug 25, 2015

vasanthrj commented Sep 9, 2015

gasparms commented Nov 22, 2017 •

edited

Loading


		Avro schema's field 'doc' will be preserved in DataFrame's schema metadata.

		To include aliases column in scheme invoke

Alias and Doc support in Spark Schema #51

Are you sure you want to change the base?

Alias and Doc support in Spark Schema #51

Conversation

vasanthrj commented May 30, 2015

vasanthrj commented Jun 10, 2015

jendap Jul 19, 2015

Choose a reason for hiding this comment

jendap commented Jul 19, 2015

JoshRosen Jul 20, 2015

Choose a reason for hiding this comment

marmbrus commented Jul 20, 2015

vasanthrj commented Jul 22, 2015

jendap Jul 24, 2015

Choose a reason for hiding this comment

vasanthrj commented Jul 26, 2015

codecov-io commented Jul 26, 2015

Current coverage is 93.79%

marmbrus commented Jul 27, 2015

vasanthrj commented Aug 16, 2015

marmbrus Aug 17, 2015

Choose a reason for hiding this comment

marmbrus Aug 25, 2015

Choose a reason for hiding this comment

marmbrus commented Aug 25, 2015

vasanthrj commented Sep 9, 2015

gasparms commented Nov 22, 2017 • edited Loading

Current coverage is `93.79%`

gasparms commented Nov 22, 2017 •

edited

Loading