Row-based read support #68

sadikovi · 2018-03-26T09:27:10Z

This PR adds new API for row-based reading of Parquet files.

This is one of the layers of API that will be finally provided (with a prospect of dropping record API in favour of Arrow, once it is integrated):

Low-level column readers API (already exists)
Record API (this PR)
Arrow API (TODO)

Most of the code is in src/record module:

api.rs contains all necessary API for mapping to Row, which is our internal representation of records, supports most of the primitive types (easy to add new ones) and all complex types, e.g. structs, lists, maps.
triplet.rs contains internal triplet iterator that provides access to (value, def level, rep level) tuple, performs necessary buffering and spacing of values.
reader.rs contains the code to assemble a tree of readers and to traverse the tree. Also contains RecordIter and RowIter iterators of records.

API for FileReader and RowGroupReader is also updated to return an iterator of Rows:

fn get_row_iter(&self, projection: Option<SchemaType>) -> Result<RowIter>;
fn get_row_iter(&self, projection: Option<SchemaType>) -> Result<RowIter>;

These methods (especially for FileReader) can take optional projected schema to read only certain columns from a file. When None is provided, full schema is implied.
RowIter will automatically load the next row group, as iterator progresses. For RowGroupReader row iterator will only load data from that row group.

I also added parquet-read.rs file in bin directory, to read Parquet files. This allows to quickly inspect files similar to parquet-mr/parquet-tools. Now we have two binaries - one is for inspecting schema parquet-schema and another one for reading data parquet-read.

There are also some minor updates, like making certain methods public so they can be used in reader and moving common test methods into test_common.rs.

Added the following test files:

nonnullable.impala.parquet (Impala test file)
nullable.impala.parquet (Impala test file)
nulls.snappy.parquet (file that contains nulls only)
nested_lists.snappy.parquet (file with nested lists)
nested_maps.snappy.parquet (file with nested maps)

I think it is a good idea to update main README with available APIs - will do it after this PR is merged.

Closes #41
Closes #42

sadikovi · 2018-03-26T09:29:07Z

@sunchao could you review the final version of record reader? It has not changed much, but I made some comment updates and minor tweaks.

Thanks!

coveralls · 2018-03-26T09:56:51Z

Coverage increased (+0.3%) to 92.773% when pulling c0b0d5b on sadikovi:read-support-final into d9710c8 on sunchao:master.

coveralls · 2018-03-26T09:56:51Z

Coverage increased (+0.2%) to 92.666% when pulling aefb3f4 on sadikovi:read-support-final into f2f0993 on sunchao:master.

sunchao

Thanks @sadikovi ! this is awesome work! I left a few comments.

sunchao · 2018-03-27T03:58:08Z

src/bin/read-file.rs

+fn main() {
+  let args: Vec<String> = env::args().collect();
+  if args.len() != 2 && args.len() != 3 {
+    println!("Usage: read-file <file-path> <num-records>");


Change <num-records> to [num-records]?

Yes, will change.

sunchao · 2018-03-27T03:59:36Z

src/bin/read-file.rs

@@ -0,0 +1,43 @@
+extern crate parquet;


I think the file name read-file is still not specific enough - after people do cargo install. Perhaps we should add prefix such as parquet or something to differentiate these executables (same for dump-schema)?

I can rename them to parquet-read and parquet-schema. Will that work?

Yes, that should be better 👍

sunchao · 2018-03-27T04:11:18Z

src/file/reader.rs

+  /// Get full iterator of `Row` from a file (over all row groups).
+  /// Projected schema can be a subset of or equal to the file schema, when it is None,
+  /// full file schema is assumed.
+  fn get_row_iter(&self, projection: Option<SchemaType>) -> Result<FileRowIter>;


Can we use a single type of iterator for both FileReader::get_row_iter and RowGroupReader::get_row_iter?

Yes, we should be able to, will change.

sunchao · 2018-03-27T04:42:15Z

src/record/api.rs

+      PhysicalType::BYTE_ARRAY => {
+        match logical_type {
+          LogicalType::UTF8 | LogicalType::ENUM | LogicalType::JSON => {
+            Row::Str(String::from_utf8(value.data().to_vec()).unwrap())


you can change from_utf8 to from_utf8_unchecked

It is marked as unsafe. I can add it like this:

let value = unsafe { String::from_utf8_unchecked(value.data().to_vec()) }; Row::Str(value)

Will it be okay?

Yes I think it should be OK.

sunchao · 2018-03-27T04:49:33Z

src/record/reader.rs

+
+  /// Returns true if repeated type is an element type for the list.
+  /// Used to determine legacy list types.
+  /// This method is copied from Spark Parquet reader.


nit: Perhaps good to also add the link for reference: https://github.com/apache/parquet-format/blob/master/LogicalTypes.md#backward-compatibility-rules

sadikovi · 2018-03-29T09:28:24Z

@sunchao I addressed your comments. Could you do another pass on this PR? Thanks!

sunchao

This LGTM!

sunchao · 2018-03-29T18:50:28Z

The latest build failed because Thrift failed to compile with the latest nightly. I've filed Thrift-4536 to fix it. In the meanwhile, we can update the travis.yml to use an earlier nightly version (e.g., nightly-2018-03-14).

sadikovi · 2018-03-29T21:35:30Z

Thanks for the following up on the build error!

As you mentioned on JIRA, it looks like we need to fix try_from crate. I suggest we do that, otherwise it would fail for other people that use the latest nightly Rust. Let me know if you are planning to open a PR in the try_from repository, or if you want I could try fixing it.

sunchao · 2018-03-29T21:51:12Z

NP @sadikovi . Yes I'm working on a PR for the try_from repo - will post soon :)
Meanwhile you can change the travis.yml to use nightly-2018-03-14 to unblock the merge.

sadikovi · 2018-03-29T21:59:57Z

@sunchao Thanks. I updated the version. We need to revert it once try_from is fixed!

sunchao · 2018-03-29T22:26:02Z

Actually the fix is more involved than I expected. I think it might be wiser to wait until there's a conclusion in the Rust thread. It is breaking multiple libraries.

sunchao · 2018-03-29T22:28:08Z

It failed again (not sure why since it succeeded on my machine) - can you try nightly-2018-03-26?

sadikovi · 2018-03-29T22:31:38Z

@sunchao I updated, thanks!

sunchao · 2018-03-29T23:01:52Z

Merged! Thanks @sadikovi for this awesome contribution!

sadikovi · 2018-03-29T23:03:50Z

@sunchao Thanks for merging! I really appreciate your review and help throughout working on read support.

I will open another pull request with README update, including how to read schema and/or files.

sadikovi added 9 commits March 25, 2018 10:14

make small changes for read support

561d1ce

add record api

266f231

add bin read-file

ca5de1e

update pattern match

6b5022d

add test files for reader

0644d13

move file methods to test_common

b9a63da

add tests for triplet

8ddebd7

add tests for reader

4294586

remove new line

c0b0d5b

sunchao reviewed Mar 27, 2018

View reviewed changes

sadikovi added 6 commits March 28, 2018 19:58

rename bin files

bc11250

update num-records messages

c28bdea

minor changes, add compatibility link

598d840

update file api

2476ae3

add test

e6ec884

Merge remote-tracking branch 'origin/master' into read-support-final

91254ed

sunchao approved these changes Mar 29, 2018

View reviewed changes

downgrade Rust nightly version for the build

8de8a5d

downgrade rust nightly to nightly-2018-03-26 in travis

aefb3f4

sunchao merged commit 67636b0 into sunchao:master Mar 29, 2018

sadikovi deleted the read-support-final branch March 29, 2018 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Row-based read support #68

Row-based read support #68

sadikovi commented Mar 26, 2018 •

edited

Loading

sadikovi commented Mar 26, 2018

coveralls commented Mar 26, 2018

coveralls commented Mar 26, 2018 •

edited

Loading

sunchao left a comment

sunchao Mar 27, 2018

sadikovi Mar 28, 2018

sunchao Mar 27, 2018

sadikovi Mar 28, 2018

sunchao Mar 28, 2018

sunchao Mar 27, 2018

sadikovi Mar 28, 2018

sunchao Mar 27, 2018

sadikovi Mar 28, 2018 •

edited

Loading

sunchao Mar 28, 2018

sunchao Mar 27, 2018

sadikovi commented Mar 29, 2018

sunchao left a comment

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sunchao commented Mar 29, 2018

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

Row-based read support #68

Row-based read support #68

Conversation

sadikovi commented Mar 26, 2018 • edited Loading

sadikovi commented Mar 26, 2018

coveralls commented Mar 26, 2018

coveralls commented Mar 26, 2018 • edited Loading

sunchao left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadikovi Mar 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadikovi commented Mar 29, 2018

sunchao left a comment

Choose a reason for hiding this comment

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sunchao commented Mar 29, 2018

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sunchao commented Mar 29, 2018

sadikovi commented Mar 29, 2018

sadikovi commented Mar 26, 2018 •

edited

Loading

coveralls commented Mar 26, 2018 •

edited

Loading

sadikovi Mar 28, 2018 •

edited

Loading