Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-1408: [C++] IPC public API cleanup, refactoring. Add SerializeSchema, ReadSchema public APIs #988

Closed
wants to merge 7 commits into from

Conversation

wesm
Copy link
Member

@wesm wesm commented Aug 24, 2017

This is mostly moving code around. In reviewing I recommend focusing on the public headers. There were a number of places where it is more consistent to use naked pointers versus shared_ptr. Also some constructors were returning shared_ptr to subclass, where it would be simpler for clients to return a pointer to base.

This includes ARROW-1376 and ARROW-1406

Change-Id: Ic152224d842b7c19079487d3ba1ca531e51e0687
… from metadata.h to metadata-internal.h and create message.h, dictionary.h

Change-Id: I3fbacc6a0f5d02b734a04b9717d426cb1b89ace2
…anywhere

Change-Id: I3ba12a8874426b33c17b9d5491ef54965cce1051
Change-Id: If8f66ebb6d3914d8aa1936be0410079ab735de89
Change-Id: I6be0f1d53df9661a158e80e5b1a901336a46fae6
Change-Id: I031b65db10357b04cfcfc0b85162b4a51406efc5
Change-Id: Icccce78b5fc73c66088dbafcdf8a098a009371aa
@wesm
Copy link
Member Author

wesm commented Aug 24, 2017

Summary of deprecations:

  • Deprecated in favor of an InputStream based version (since the only difference is a Seek)
ARROW_EXPORT
Status ReadRecordBatch(const std::shared_ptr<Schema>& schema, int64_t offset,
                       io::RandomAccessFile* stream, std::shared_ptr<RecordBatch>* out);
  • RecordBatchStreamWriter and file writer return std::shared_ptr<RecordBatchWriter> since they have identical APIs

@cpcloud
Copy link
Contributor

cpcloud commented Aug 24, 2017

+1 LGTM. The API looks much more unified this way.

@cpcloud
Copy link
Contributor

cpcloud commented Aug 24, 2017

Looks like a transient conda error on appveyor:

CondaHTTPError: HTTP 502 BAD GATEWAY for url <https://repo.continuum.io/pkgs/pro/win-64/repodata.json.bz2>

@wesm
Copy link
Member Author

wesm commented Aug 25, 2017

Alright, merging

@asfgit asfgit closed this in f50f2ea Aug 25, 2017
@wesm wesm deleted the ARROW-1408 branch August 25, 2017 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants