PARQUET-583: Parquet to Thrift schema conversion

[xhochy]

Depends on #86

[wesm]

I suppose this should be made Visit(Node* node) to differentiate with ConstVisitor?

[wesm]

This looks good to me outside of the minor comments. The way we're handling the not-always-set SchemaElement fields bums me out a bit, but if they can be counted on to always be -1 if not set it does make testing easier. Pretty open-minded about improving this aspect of the schema code generally

[xhochy]

For the not-always-set SchemaElement fields, we probably need a way in SchemaElement to track if they were set (which we could also define as value == -1). Then we could decide consistently in the to-thrift-schema-conversion if we should write them out.

[wesm]

If the struct is zeroed out, you can use the element.__isset for this

[xhochy]

Yes, we have that in Thrift. Probably I got it wrong in my last comment ;) What I'm missing is that we haven't got something similar to .__isset in PrimitiveNode to track if we have set one of length, precision, scale.

But to get this PR to a close: Should I only call element->__set_type_length if length != -1 or keep the code for as it is? (this is the only remaining issue here or am I missing something?)

[wesm]

Keeping things as is works for me right now, we can iterate on the optional field stuff later.

[wesm]

+1, merging