PARQUET-212: Implement LIST read compatibility rules in Thrift

[rdblue]

This implements the read-side compatibility rules for 2-level and 3-level lists in Thrift.

Thrift doesn't allow null elements inside lists, but 3-level lists may have optional elements. This PR adds a property, parquet.thrift.ignore-null-elements, that allows thrift to read lists with optional elements by ignoring nulls. This is off by default, but is provided as an opt-in for compatibility with data written by Hive.

Thrift's schema conversion does not change because a Thrift class (or Scrooge etc.) must be set in a file's metadata or provided when constructing a reader.

This replaces and closes #144.

[rdblue]

@isnotinvain you may want to have a look at this since you guys do the most Thrift work. This adds support in Thrift to read data written by the other object models correctly.

[isnotinvain]

I'll try to take a look thanks

[rdblue]

Thanks, Alex!

@liancheng may have some comments as well since he is familiar with the compatibility rules.

[liancheng]

I don't quite get the purpose of this change. Is this mainly for more robust support for schema evolution? Take the following Avro schema and Parquet schema as an example:

// Avro
record Struct {
  int f0;
  int f1;
}

// Parquet
message StructArray {
  required struct_array (LIST) {
    repeated group array {
      required int32 f0;
    }
  }
}

According to the condition of this if branch, we should interpret struct_array as a LIST of Avro Struct, with all f1 field set as null. However, f1 defined in Struct isn't optional and can't be null.

Did I miss something here?

[liancheng]

My last comment is a little bit ambiguous. In the example I made, the Avro record Struct is the elementSchema, while the Parquet repeated group array is the repeatedType in the code.

[liancheng]

I guess what is missing here is that we should check all the missing fields are indeed optional, unless this constraint has already been guaranteed somewhere else (then let's document it in the comment).

[rdblue]

This is for the case where only one field of a nested record is projected. Avro has a projection schema and an expected schema so they can differ. Previously, the logic to match the requested schema (elementSchema) with the file's structure checked whether the name of the single field matched the name of the single field of the element schema. But that left out the case where the two do match (as in your example) but the requested schema has an optional field that should be defaulted.

Whether the other field is optional doesn't matter at this point in the code. Avro fills in default values later and throws an exception if something is missing and has no default at that point.

[liancheng]

Oh I see, this totally makes sense. Thanks for the explanation!

[liancheng]

Got some questions about isListElementType updates.

Also, it would be nice to add parquet-thrift test cases that cover PARQUET-364, e.g., something like this.

[rdblue]

@liancheng, you're right that I still need to add more test cases, thanks for pointing that out.

[rdblue]

@liancheng, I added all of the test cases that were added in PARQUET-364 to this commit. I think that addresses all of your concerns.

@isnotinvain, do you still want to have a look at this?

[liancheng]

+1

[rdblue]

Thanks @liancheng!