Skip to content

Conversation

@gawain-bolton
Copy link

Add methods to the StreamReader class to allow for columns and rows of
data to be skipped.

Also add helper methods for finding out:

  • total number of columns
  • total number of rows
  • current row
  • current column

@github-actions
Copy link

github-actions bot commented Nov 9, 2019

@gawain-bolton gawain-bolton changed the title PARQUET-1689: Stream API: Allow for columns/rows to be skipped when reading PARQUET-1689: [C++] Stream API: Allow for columns/rows to be skipped when reading Nov 9, 2019
@gawain-bolton gawain-bolton force-pushed the PARQUET-1689_stream_reader_column_row_skipping branch from 6f55838 to 87c11ac Compare November 10, 2019 16:47
…eading

Add methods to the StreamReader class to allow for columns and rows of
data to be skipped.

Also add helper methods for finding out:
- total number of columns
- total number of rows
- current row
- current column
@gawain-bolton gawain-bolton force-pushed the PARQUET-1689_stream_reader_column_row_skipping branch from 87c11ac to 5ec4cca Compare November 12, 2019 22:34
@emkornfield
Copy link
Contributor

One more small comment on tests, otherwise I think this is reasonable to merge. Also, as a note, it is generally easier to review changes if you don't squash commits on every revision (when merging the merge tool will do this, and it can sometimes be easier to review individual updates).

Copy link
Contributor

@emkornfield emkornfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me.

@emkornfield
Copy link
Contributor

LGTM, @xhochy could you take a look over and see if you can merge it?

Copy link
Member

@xhochy xhochy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants