{"action":"created","issue":{"url":"https://api.github.com/repos/apache/parquet-mr/issues/342","repository_url":"https://api.github.com/repos/apache/parquet-mr","labels_url":"https://api.github.com/repos/apache/parquet-mr/issues/342/labels{/name}","comments_url":"https://api.github.com/repos/apache/parquet-mr/issues/342/comments","events_url":"https://api.github.com/repos/apache/parquet-mr/issues/342/events","html_url":"https://github.com/apache/parquet-mr/pull/342","id":153329630,"number":342,"title":"PARQUET-601: Add support to configure the encoding used by ValueWriters","user":{"login":"piyushnarang","id":9885908,"avatar_url":"https://avatars.githubusercontent.com/u/9885908?v=3","gravatar_id":"","url":"https://api.github.com/users/piyushnarang","html_url":"https://github.com/piyushnarang","followers_url":"https://api.github.com/users/piyushnarang/followers","following_url":"https://api.github.com/users/piyushnarang/following{/other_user}","gists_url":"https://api.github.com/users/piyushnarang/gists{/gist_id}","starred_url":"https://api.github.com/users/piyushnarang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/piyushnarang/subscriptions","organizations_url":"https://api.github.com/users/piyushnarang/orgs","repos_url":"https://api.github.com/users/piyushnarang/repos","events_url":"https://api.github.com/users/piyushnarang/events{/privacy}","received_events_url":"https://api.github.com/users/piyushnarang/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":16,"created_at":"2016-05-05T21:24:25Z","updated_at":"2016-07-27T21:58:08Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/apache/parquet-mr/pulls/342","html_url":"https://github.com/apache/parquet-mr/pull/342","diff_url":"https://github.com/apache/parquet-mr/pull/342.diff","patch_url":"https://github.com/apache/parquet-mr/pull/342.patch"},"body":"### Context:\r\nParquet is currently structured to choose the appropriate value writer based on the type of the column as well as the Parquet version. As of now, the writer(s) (and hence encoding) for each data type is hard coded in the Parquet source code. \r\n\r\nThis PR adds support for being able to override the encodings per type via config. That allows users to experiment with various encoding strategies manually as well as enables them to override the hardcoded defaults if they don't suit their use case. \r\n\r\nWe can override encodings per data type (int32 / int64 / ...). \r\nSomething on the lines of:\r\n```\r\nparquet.writer.encoding-override.<type> = \"encoding1[,encoding2]\"\r\n```\r\n\r\nAs an example:\r\n```\r\n\"parquet.writer.encoding-override.int32\" = \"plain\"\r\n(Chooses Plain encoding and hence the PlainValuesWriter).\r\n```\r\n\r\nWhen a primary + fallback need to be specified, we can do the following:\r\n```\r\n\"parquet.writer.encoding-override.binary\" = \"rle_dictionary,delta_byte_array\"\r\n(Chooses RLE_DICTIONARY encoding as the initial encoding and DELTA_BYTE_ARRAY encoding as the fallback and hence creates a FallbackWriter(PlainBinaryDictionaryValuesWriter, DeltaByteArrayWriter). \r\n```\r\n\r\nIn such cases we can mandate that the first encoding listed must allow for Fallbacks by implementing [RequiresFallback](https://github.com/apache/parquet-mr/blob/master/parquet-column/src/main/java/org/apache/parquet/column/values/RequiresFallback.java#L31).\r\n\r\n### PR notes:\r\n\r\n- Restructured the ValuesWriter creation code. Pulled it out of ParquetProperties into a new class and refactored the flow based on type as it was getting hard to follow and I felt adding the overrides would make it harder. Added a bunch of unit tests to verify the ValuesWriter we create for combinations of type, parquet version and dictionary on / off. \r\n- Added unit tests to verify parsing of the encoding overrides + creation of ValuesWriters based on these overrides.\r\n- Manually tested some encoding overrides scenarios out on Hadoop (both parquet v1, v2). "},"comment":{"url":"https://api.github.com/repos/apache/parquet-mr/issues/comments/235734420","html_url":"https://github.com/apache/parquet-mr/pull/342#issuecomment-235734420","issue_url":"https://api.github.com/repos/apache/parquet-mr/issues/342","id":235734420,"user":{"login":"piyushnarang","id":9885908,"avatar_url":"https://avatars.githubusercontent.com/u/9885908?v=3","gravatar_id":"","url":"https://api.github.com/users/piyushnarang","html_url":"https://github.com/piyushnarang","followers_url":"https://api.github.com/users/piyushnarang/followers","following_url":"https://api.github.com/users/piyushnarang/following{/other_user}","gists_url":"https://api.github.com/users/piyushnarang/gists{/gist_id}","starred_url":"https://api.github.com/users/piyushnarang/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/piyushnarang/subscriptions","organizations_url":"https://api.github.com/users/piyushnarang/orgs","repos_url":"https://api.github.com/users/piyushnarang/repos","events_url":"https://api.github.com/users/piyushnarang/events{/privacy}","received_events_url":"https://api.github.com/users/piyushnarang/received_events","type":"User","site_admin":false},"created_at":"2016-07-27T21:58:08Z","updated_at":"2016-07-27T21:58:08Z","body":"@rdblue Thanks for taking a look. I can take a stab at refactoring things from the `ValuesWriterFactoryParams` to ParquetProperties. I also prefer the separation of ParquetProperties and the `ValuesWriterFactory` code. ParquetProperties was getting to hard to read and doing too many things at once. \r\n\r\nI wasn't aware of us requiring that `ValuesWriter` are not to be part of the public API. I do however see value in us being able to configure strategies to help choose these writers - it helps users test out various encoding strategies manually and in the future we could also plug in sophisticated strategies that could pick column encodings in an automated fashion. We could set up an SPI module but we'd still not be able to configure which factory to use at runtime right? We could have some annotations that we could look up by reflection to see which ValuesWriterFactories there are and which one is the chosen one but that would need to be specified in code. \r\n\r\nShall try and think of any other potential options."}}
{"actor":{"display_login":"piyushnarang"}}