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

CSV reader example for the cpp client #1455

Merged
merged 21 commits into from
Oct 18, 2021

Conversation

jcferretti
Copy link
Member

No description provided.

cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
cpp-examples/readcsv/main.cc Show resolved Hide resolved
@jcferretti
Copy link
Member Author

Fixes #892

@jcferretti jcferretti changed the title WIP CSV reader example for the cpp client CSV reader example for the cpp client Oct 15, 2021
@jcferretti jcferretti marked this pull request as ready for review October 15, 2021 21:52
@jcferretti jcferretti requested a review from kosak October 15, 2021 22:02
cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
nbauernfeind
nbauernfeind previously approved these changes Oct 18, 2021
cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
cpp-examples/readcsv/main.cc Show resolved Hide resolved
kosak
kosak previously approved these changes Oct 18, 2021
Copy link
Contributor

@kosak kosak left a comment

Choose a reason for hiding this comment

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

Approve, with some nits if you want to address them or not.

cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
cpp-examples/readcsv/main.cc Outdated Show resolved Hide resolved
@jcferretti jcferretti merged commit c646c80 into deephaven:main Oct 18, 2021
@jcferretti jcferretti deleted the cfs-cpp-readcsv-0 branch October 18, 2021 21:39
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2021
final ArrowType.Timestamp timestampType = (ArrowType.Timestamp) arrowType;
final String tz = timestampType.getTimezone();
final TimeUnit timestampUnit = timestampType.getUnit();
if (tz == null || "UTC".equals(tz)) {
Copy link
Member

Choose a reason for hiding this comment

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

This is what I was worried about on the call, and makes me happy enough to ignore the rest for now, a good error indicating that we are just punting on anything else interesting.

throw GrpcUtil.statusRuntimeException(Code.INVALID_ARGUMENT, exMsg +
" of floatingPointType(Precision=" + floatingPointType.getPrecision().toString() + ")");
}
case Utf8:
Copy link
Member

Choose a reason for hiding this comment

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

Also LargeUtf8 here.

And probably Null if we're adding to this again, which is easy, it sends nothing, all nulls.

And possibly binary/largebinary/fixedsizebinary would map easily to byte[] or DbByteArray.

Seems like if we're handling near matches, we should address most of the things - Interval and Time are mostly "shaped like" and used like Duration, for example, and any Decimal can be made to work in BigDecimal (but not vice versa - then again most of these others including TImestamp won't behave vice versa either).

return double.class;
case HALF:
default:
throw GrpcUtil.statusRuntimeException(Code.INVALID_ARGUMENT, exMsg +
Copy link
Member

Choose a reason for hiding this comment

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

or possibly just upconvert to float - i mean that's what we're doing with less precise timestamps that we don't support...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants