[SPARK-11044][SQL] Parquet writer version fixed as version1

[HyukjinKwon]

https://issues.apache.org/jira/browse/SPARK-11044

Spark writes a parquet file only with writer version1 ignoring the writer version given by user.

So, in this PR, it keeps the writer version if given or sets version1 as default.

[srowen]

Can you just use setIfUnset here?

[HyukjinKwon]

Yeap I just updated. Thanks.

[JoshRosen]

/cc @liancheng

[HyukjinKwon]

@liancheng I assume you missed this.

[liancheng]

@HyukjinKwon Oh yeah, sorry. Finally got sometime to clean my review queue :)

I wonder is there an easy way to add a test case for this? At first I thought WriterVersion corresponds to the the version field of the Thrift struct FileMetaData described in parquet-format, but it's not. I only found that when WriterVersion is set to v2, the Thrift field PageHeader.type is set to DATA_PAGE_V2.

[liancheng]

ok to test

[HyukjinKwon]

I will try to find and test them first tommorow before adding a commit!

[SparkQA]

Test build #45626 has finished for PR 9060 at commit 2eee7e3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[HyukjinKwon]

@liancheng I gave some tries to figure out the version but.. as you said, it is pretty tricky to check the writer version as it only changes the version of data page which we could know only within the internal of Parquet. This is also because the writer version changes encoding types of each data page but this encoding type is only recorded in datapage header which is not the part of footer.

Would this be too inappropriate if we write Parquet files with both version1 and version2 and then, check if the sizes of both are equal?

Since encoding types are different, both the sizes should be also different.

[liancheng]

I think we can check for column encoding information, which is accessible from Parquet footers. For example, PARQUET_2_0 uses RLE_DICTIONARY while PARQUET_1_0 uses PLAIN_DICTIONARY (see here).

The parquet-meta CLI tool can be a reference for how to inspect related metadata.

[HyukjinKwon]

Thank you very much. I will try in that way.

[liancheng]

You may construct a Parquet file consists of a single column with dictionary encoding using:

val path = "file:///tmp/parquet/dict"
sqlContext.range(1 << 16).selectExpr("(id % 4) AS i").coalesce(1).write.mode("overwrite").parquet(path)

And here are instructions of building and installing the parquet-tools CLI tool. Then you can inspect Parquet metadata using:

$ parquet-meta /tmp/parquet/dict

file:        file:/private/tmp/parquet/dict/part-r-00000-88498608-9eed-4728-b96a-b60bc5ebc2a8.gz.parquet
creator:     parquet-mr version 1.6.0
extra:       org.apache.spark.sql.parquet.row.metadata = {"type":"struct","fields":[{"name":"i","type":"long","nullable":true,"metadata":{}}]}

file schema: root
----------------------------------------------------------------------------------------------------------------------------------------------
i:           OPTIONAL INT64 R:0 D:1

row group 1: RC:65536 TS:16615 OFFSET:4
----------------------------------------------------------------------------------------------------------------------------------------------
i:            INT64 GZIP DO:0 FPO:4 SZ:198/16615/83.91 VC:65536 ENC:BIT_PACKED,RLE,PLAIN_DICTIONARY

The ENC:... part in the last line is column encoding information.

[HyukjinKwon]

Thanks! I will follow the way.

[HyukjinKwon]

Fortunately, I worked around parquet tools once and looked through Parquet codes several times before :).

Thank you very much for your help. This could be done much more easily than I though because of your help.

[SparkQA]

Test build #45810 has finished for PR 9060 at commit 2d1d343.

• This patch fails Spark unit tests.
• This patch does not merge cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #45811 has finished for PR 9060 at commit 7e80ad6.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #45831 has finished for PR 9060 at commit 78449ec.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):\n * class TypedColumn[-T, U](\n * class JavaTrackStateDStream[KeyType, ValueType, StateType, EmittedType](\n

[liancheng]

Nit: Remove this empty line.

[liancheng]

LGTM except for a few minor styling issues. I can merge it right after you fix them.

[HyukjinKwon]

I accidentally saw TODO Adds test case for reading dictionary encoded decimals written as 'FIXED_LEN_BYTE_ARRAY'.

I will also add this test in the following PR for using the overloaded writeMetaFile.

[SparkQA]

Test build #45964 has finished for PR 9060 at commit cea5034.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[liancheng]

@marmbrus Is this one OK for branch-1.6?

[liancheng]

@HyukjinKwon Thanks! I've merged this one to master. And yes, please feel free to add the decimal test case(s).

[marmbrus]

Sure

[liancheng]

Merging to branch-1.6.