Add TableInfo and related model classes

[mziccard]

This is a very big PR (but most of it is javadocs and tests) that adds TableInfo model class and all related classes:

• TableSchema
• FieldSchema
• CsvOptions
• ExternalDataConfiguration
• UserDefinedFunction

Unit tests are also added.

Remarks
I added the TableSchema class to wrap schema information as it also appears elsewhere (Query results). Schema is just a list of FieldSchema objects so if you prefer we can drop the class and just use List<FieldSchema>. I have no strong preferences.

[mziccard]

Fixed the comments, main changes are:

• TableSchema renamed to Schema and FieldSchema renamed to Field
• Changed Type enum to class in Field with factory methods for all types
• Add setter and builder method that take fields as a vararg for Schema
• Fixed all javadocs comments and updated tests according to changes

[aozarov]

Looks good. Few more comments and suggestions.

[aozarov]

See my comment regarding CsvOptions (I like the change you made and suggested to take it slightly further), the possible bug in TableInfo when nulls are passed in the builder and later retrieved by the built instance and also my suggestion to use inheritance for TableInfo as well (if not inheritence maybe we can consider something similar to what you did with FormatOptions).

Also, I feel like an open comment regarding patch and ExternalDataConfiguration got lost somehow.

[mziccard]

I see where you are going with the hierarchies. The fact is that i really don't like that the user has to cast values to get to real data. It might be me but I really consider casting an unnecessary pain in the neck. The main point is that if we have:

@SuppressWarnings("unchecked")
public <F extends FormatOptions> F formatOptions() {
  return (F) formatOptions;
}

To use this method a user must not only read its javadoc but he needs also to be aware of the FormatOptions hierarchy. If he does't know the expected type, in fact, he has to do type-checking to figure out how to use the return type. Avoiding type-checking and casting is the reason why I provided both format and csvOptions getters.

The same reasoning (even worse maybe) applies to TableInfo type. We might want to have something like:

public abstract class TableType {

  static class Table extends TableType {
    // ...
  }
  static class View {
    // ...
  }
  static class External {
    // ...
  }
  public static Table of(Schema schema);
  public static View of (String query, List<UserDefinedFunction> functions);
  public static External of(ExternalDataConfiguration configuration);
}

public class TableInfo {
  // ...
  public static Builder builder(TableType type);
  public static ViewInfo of(TableType type);
}

This is ok but if you ask me I would not remove all schema(), query(), userDefinedFunctions() and externalConfiguration() getters from TableInfo if favor of public <T extends ViewType>type() for the same reasoning I did above for CsvOptions. Even worse, here the information wrapped in types is not disjoint, the table's schema in fact is returned by the BigQuery service for all 3 types of tables.

To sum things up: I am in favor of using hierarchies to make it easier for the user to build an object right, I am not in favor of having getters that require type-checking. I am open for discussion :)

Also, I feel like an open comment regarding patch and ExternalDataConfiguration got lost somehow.

It got lost in fact, my answer is here.

[aozarov]

I think javadoc, which shoes class hierarchies, and making the set of format type constants more public (maybe not enum, but there are other ways can do it) could help with knowing which one should be used.

Also, I would expect the user to know it as (similar to the schema) it probably set it.
I only mentioned that method as a "convenient" getter that avoid explicit casting (which
we also do in other places).

Avoiding type-checking and casting is the reason why I provided both format and csvOptions getters.

If someone was calling csvOptions it is because it already knew it was a csvOptions. Yes,
one could call all xxxOptions and check for not null but this does not look better to me than
instanceof.

If we assume that more and more formats are going to be supported (which, I think, is the reason we don't use enum for type), than in that case it would mean adding more and more xxxOptions get methods on ExternalDataConfiguration. Is that desired?

As for TableInfo I think here we should not expect proliferation of types (if any), so
having the, currently 3, specific getters is not a problem (but I don't see that as an improvement) however flattening all options (such as userDefinedFunctions) in TableInfo does not seem
right to me (and force the user to know what belongs to what). In my mind it is prefered to have common stuff in common class and specific stuff in its specific place. BTW, did you consider to make TableInfo abstract and have 3 concrete implementation for it?

[mziccard]

I obey :) Kidding, I don't completely agree but we could discuss this for days and there's no point in stalling this any longer.
I hope I caught all comments, let me recap:

• f84a3b1: Fixes the comment on unsetting Field mode and description
• bd58ee5 Removes redundant getters from ExternalDataConfiguration. Makes FormatOptions.csv return a CsvOptions object (comment here)
• 66972df: Adds the TableType class with 2 subclasses View and ExternalTable. This class is used in TableInfo to wrap table's fields that depends on the type.

[aozarov]

I like this one better! (but still have few suggestions).

Next time when you do not completely agree I suggest you create an issue (marked as question) and we can continue to discuss it there (and solicit more opinions) while keeping the code as is.

[mziccard]

Some more changes:

• ae2a484: Removes TableType and creates a hierarchy of TableInfo. If you are wondering why BaseTableInfo is not abstract it's because listTables return partial information on tables that does not allow to say whether they are normal tables, views or external tables. So I thought to use BaseTableInfo for them
• 338ede1: Fixes the mode issue and another minor comment

[aozarov]

I really like it!! Few nits.