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.

Sign up for GitHub

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

Jump to bottom

Update CPP client #2981

Merged
merged 4 commits into from
Oct 10, 2022
Merged

Update CPP client #2981

merged 4 commits into from
Oct 10, 2022

Conversation

kosak
Copy link
Contributor

@kosak kosak commented Oct 10, 2022 •
edited
Loading

There are three commits in this PR:

  1. Address C++ Client should get schema via Arrow GetSchema call, rather than Get #2980
  2. Fix a bug in update_processor.cc which was converting key space into position space (good), but then converting again to position space (very bad)
  3. Reorganize some Arrow utility methods into separate files
  4. Commit the demo code for chapter 3 of the video demos

Fixes #2980

@kosak kosak requested a review from jcferretti October 10, 2022 00:02
jcferretti
jcferretti reviewed Oct 10, 2022
View reviewed changes
cpp-client/deephaven/client/include/private/deephaven/client/impl/table_handle_impl.h Outdated
@@ -37,11 +37,8 @@ class TableHandleManagerImpl;
namespace internal {
class GetColumnDefsCallback;

class LazyState final
class EtcCallback final
Copy link
Member

Choose a reason for hiding this comment

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

Is Etc == ExportedTableCreation? :-)
I am all in for terseness, but it sounds a bit funny to me to abbrev to "etc" and then spell out "Callback" in full :-P.
No action item here, not asking you to change anything, just thinking out loud. Maybe put a comment on top.

jcferretti
jcferretti previously approved these changes Oct 10, 2022
View reviewed changes
@kosak kosak merged commit c599c78 into deephaven:main Oct 10, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 10, 2022
@kosak kosak deleted the kosak_update-cpp-client branch February 5, 2023 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jcferretti jcferretti jcferretti approved these changes

Assignees
No one assigned
Labels
NoDocumentationNeeded
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

C++ Client should get schema via Arrow GetSchema call, rather than Get
2 participants
@kosak @jcferretti