From ddc08ac6d05478a739d71afdd7aae68f3e84a378 Mon Sep 17 00:00:00 2001 From: Corey Kosak Date: Sun, 19 Nov 2023 17:13:46 -0500 Subject: [PATCH] C++ Client: avoid deprecated Arrow methods, plus conform to coding style --- .../include/public/deephaven/client/client.h | 8 +- .../include/public/deephaven/client/flight.h | 2 +- .../deephaven/client/utility/arrow_util.h | 14 +- cpp-client/deephaven/dhclient/src/client.cc | 162 +++++++++--------- cpp-client/deephaven/dhclient/src/flight.cc | 8 +- .../dhclient/src/utility/arrow_util.cc | 2 - 6 files changed, 98 insertions(+), 98 deletions(-) diff --git a/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h b/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h index a99ca9c46e9..d40d0bed805 100644 --- a/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h +++ b/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h @@ -1407,7 +1407,7 @@ class TableHandle { * @return A TableHandle referencing the new table */ [[nodiscard]] - TableHandle WAvgBy(std::string weight_column, std::vector columnSpecs) const; + TableHandle WAvgBy(std::string weight_column, std::vector column_specs) const; /** * A variadic form of WAvgBy(std::string, std::vector) const that takes a combination of * argument types. @@ -1538,7 +1538,7 @@ class TableHandle { * @return A TableHandle referencing the new table */ [[nodiscard]] - TableHandle Merge(std::string key_column, std::vector sources) const; + TableHandle Merge(std::string key_columns, std::vector sources) const; /** * A variadic form of Merge(std::string, std::vector) const that takes a combination of * argument types. @@ -1848,8 +1848,8 @@ class TableHandle { * Subscribe to a ticking table (C-style). */ [[nodiscard]] - std::shared_ptr Subscribe(onTickCallback_t onTick, void *onTickUserData, - onErrorCallback_t on_error, void *onErrorUserData); + std::shared_ptr Subscribe(onTickCallback_t on_tick, void *on_tick_user_data, + onErrorCallback_t on_error, void *on_error_user_data); /** * Unsubscribe from the table. */ diff --git a/cpp-client/deephaven/dhclient/include/public/deephaven/client/flight.h b/cpp-client/deephaven/dhclient/include/public/deephaven/client/flight.h index 7d8c5351658..2c14fa528b6 100644 --- a/cpp-client/deephaven/dhclient/include/public/deephaven/client/flight.h +++ b/cpp-client/deephaven/dhclient/include/public/deephaven/client/flight.h @@ -30,7 +30,7 @@ class FlightWrapper { * @return An Arrow FlightStreamReader */ [[nodiscard]] - std::shared_ptr GetFlightStreamReader( + std::unique_ptr GetFlightStreamReader( const TableHandle &table) const; /** diff --git a/cpp-client/deephaven/dhclient/include/public/deephaven/client/utility/arrow_util.h b/cpp-client/deephaven/dhclient/include/public/deephaven/client/utility/arrow_util.h index 2c42c5d43f7..7b77f0fef15 100644 --- a/cpp-client/deephaven/dhclient/include/public/deephaven/client/utility/arrow_util.h +++ b/cpp-client/deephaven/dhclient/include/public/deephaven/client/utility/arrow_util.h @@ -15,6 +15,13 @@ namespace deephaven::client::utility { arrow::flight::FlightDescriptor ConvertTicketToFlightDescriptor(const std::string &ticket); +/** + * If status is OK, do nothing. Otherwise throw a runtime error with an informative message. + * @param debug_info A DebugInfo object, typically as provided by DEEPHAVEN_LOCATION_EXPR. + * @param status the arrow::Status + */ +void OkOrThrow(const deephaven::dhcore::utility::DebugInfo &debug_info, const arrow::Status &status); + /** * If result's status is OK, do nothing. Otherwise throw a runtime error with an informative message. * @param debug_info A DebugInfo object, typically as provided by DEEPHAVEN_LOCATION_EXPR. @@ -25,13 +32,6 @@ void OkOrThrow(const deephaven::dhcore::utility::DebugInfo &debug_info, const ar OkOrThrow(debug_info, result.status()); } -/** - * If status is OK, do nothing. Otherwise throw a runtime error with an informative message. - * @param debug_info A DebugInfo object, typically as provided by DEEPHAVEN_LOCATION_EXPR. - * @param status the arrow::Status - */ -void OkOrThrow(const deephaven::dhcore::utility::DebugInfo &debug_info, const arrow::Status &status); - /** * If result's internal status is OK, return result's contained value. * Otherwise throw a runtime error with an informative message. diff --git a/cpp-client/deephaven/dhclient/src/client.cc b/cpp-client/deephaven/dhclient/src/client.cc index d6ed48dd0be..b31b86b8aa9 100644 --- a/cpp-client/deephaven/dhclient/src/client.cc +++ b/cpp-client/deephaven/dhclient/src/client.cc @@ -115,8 +115,8 @@ TableHandle TableHandleManager::EmptyTable(int64_t size) const { return TableHandle(std::move(qs_impl)); } -TableHandle TableHandleManager::FetchTable(std::string tableName) const { - auto qs_impl = impl_->FetchTable(std::move(tableName)); +TableHandle TableHandleManager::FetchTable(std::string table_name) const { + auto qs_impl = impl_->FetchTable(std::move(table_name)); return TableHandle(std::move(qs_impl)); } @@ -185,8 +185,8 @@ Aggregate::~Aggregate() = default; Aggregate::Aggregate(std::shared_ptr impl) : impl_(std::move(impl)) { } -Aggregate Aggregate::AbsSum(std::vector columnSpecs) { - return createAggForMatchPairs(ComboAggregateRequest::ABS_SUM, std::move(columnSpecs)); +Aggregate Aggregate::AbsSum(std::vector column_specs) { + return createAggForMatchPairs(ComboAggregateRequest::ABS_SUM, std::move(column_specs)); } Aggregate Aggregate::Avg(std::vector column_specs) { @@ -211,8 +211,8 @@ Aggregate Aggregate::Last(std::vector column_specs) { return createAggForMatchPairs(ComboAggregateRequest::LAST, std::move(column_specs)); } -Aggregate Aggregate::Max(std::vector columnSpecs) { - return createAggForMatchPairs(ComboAggregateRequest::MAX, std::move(columnSpecs)); +Aggregate Aggregate::Max(std::vector column_specs) { + return createAggForMatchPairs(ComboAggregateRequest::MAX, std::move(column_specs)); } Aggregate Aggregate::Med(std::vector column_specs) { @@ -306,124 +306,126 @@ TableHandle TableHandle::Sort(std::vector sortPairs) const { return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::Select(std::vector columnSpecs) const { - auto qt_impl = impl_->Select(std::move(columnSpecs)); +TableHandle TableHandle::Select(std::vector column_specs) const { + auto qt_impl = impl_->Select(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::Update(std::vector columnSpecs) const { - auto qt_impl = impl_->Update(std::move(columnSpecs)); +TableHandle TableHandle::Update(std::vector column_specs) const { + auto qt_impl = impl_->Update(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::LazyUpdate(std::vector columnSpecs) const { - auto qt_impl = impl_->LazyUpdate(std::move(columnSpecs)); +TableHandle TableHandle::LazyUpdate(std::vector column_specs) const { + auto qt_impl = impl_->LazyUpdate(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::View(std::vector columnSpecs) const { - auto qt_impl = impl_->View(std::move(columnSpecs)); +TableHandle TableHandle::View(std::vector column_specs) const { + auto qt_impl = impl_->View(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::DropColumns(std::vector columnSpecs) const { - auto qt_impl = impl_->DropColumns(std::move(columnSpecs)); +TableHandle TableHandle::DropColumns(std::vector column_specs) const { + auto qt_impl = impl_->DropColumns(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::UpdateView(std::vector columnSpecs) const { - auto qt_impl = impl_->UpdateView(std::move(columnSpecs)); +TableHandle TableHandle::UpdateView(std::vector column_specs) const { + auto qt_impl = impl_->UpdateView(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::By(std::vector columnSpecs) const { - auto qt_impl = impl_->By(std::move(columnSpecs)); +TableHandle TableHandle::By(std::vector column_specs) const { + auto qt_impl = impl_->By(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::By(AggregateCombo combo, std::vector groupByColumns) const { - auto qt_impl = impl_->By(combo.Impl()->Aggregates(), std::move(groupByColumns)); +TableHandle TableHandle::By(AggregateCombo combo, std::vector group_by_columns) const { + auto qt_impl = impl_->By(combo.Impl()->Aggregates(), std::move(group_by_columns)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::MinBy(std::vector columnSpecs) const { - auto qt_impl = impl_->MinBy(std::move(columnSpecs)); +TableHandle TableHandle::MinBy(std::vector column_specs) const { + auto qt_impl = impl_->MinBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::MaxBy(std::vector columnSpecs) const { - auto qt_impl = impl_->MaxBy(std::move(columnSpecs)); +TableHandle TableHandle::MaxBy(std::vector column_specs) const { + auto qt_impl = impl_->MaxBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::SumBy(std::vector columnSpecs) const { - auto qt_impl = impl_->SumBy(std::move(columnSpecs)); +TableHandle TableHandle::SumBy(std::vector column_specs) const { + auto qt_impl = impl_->SumBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::AbsSumBy(std::vector columnSpecs) const { - auto qt_impl = impl_->AbsSumBy(std::move(columnSpecs)); +TableHandle TableHandle::AbsSumBy(std::vector column_specs) const { + auto qt_impl = impl_->AbsSumBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::VarBy(std::vector columnSpecs) const { - auto qt_impl = impl_->VarBy(std::move(columnSpecs)); +TableHandle TableHandle::VarBy(std::vector column_specs) const { + auto qt_impl = impl_->VarBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::StdBy(std::vector columnSpecs) const { - auto qt_impl = impl_->StdBy(std::move(columnSpecs)); +TableHandle TableHandle::StdBy(std::vector column_specs) const { + auto qt_impl = impl_->StdBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::AvgBy(std::vector columnSpecs) const { - auto qt_impl = impl_->AvgBy(std::move(columnSpecs)); +TableHandle TableHandle::AvgBy(std::vector column_specs) const { + auto qt_impl = impl_->AvgBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::LastBy(std::vector columnSpecs) const { - auto qt_impl = impl_->LastBy(std::move(columnSpecs)); +TableHandle TableHandle::LastBy(std::vector column_specs) const { + auto qt_impl = impl_->LastBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::FirstBy(std::vector columnSpecs) const { - auto qt_impl = impl_->FirstBy(std::move(columnSpecs)); +TableHandle TableHandle::FirstBy(std::vector column_specs) const { + auto qt_impl = impl_->FirstBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::MedianBy(std::vector columnSpecs) const { - auto qt_impl = impl_->MedianBy(std::move(columnSpecs)); +TableHandle TableHandle::MedianBy(std::vector column_specs) const { + auto qt_impl = impl_->MedianBy(std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::PercentileBy(double percentile, bool avgMedian, - std::vector columnSpecs) const { - auto qt_impl = impl_->PercentileBy(percentile, avgMedian, std::move(columnSpecs)); +TableHandle TableHandle::PercentileBy(double percentile, bool avg_median, + std::vector column_specs) const { + auto qt_impl = impl_->PercentileBy(percentile, avg_median, std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::PercentileBy(double percentile, std::vector columnSpecs) const { - auto qt_impl = impl_->PercentileBy(percentile, std::move(columnSpecs)); +TableHandle TableHandle::PercentileBy(double percentile, std::vector column_specs) const { + auto qt_impl = impl_->PercentileBy(percentile, std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::CountBy(std::string countByColumn, std::vector columnSpecs) const { - auto qt_impl = impl_->CountBy(std::move(countByColumn), std::move(columnSpecs)); +TableHandle TableHandle::CountBy(std::string count_by_column, + std::vector column_specs) const { + auto qt_impl = impl_->CountBy(std::move(count_by_column), std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::WAvgBy(std::string weightColumn, std::vector columnSpecs) const { - auto qt_impl = impl_->WavgBy(std::move(weightColumn), std::move(columnSpecs)); +TableHandle TableHandle::WAvgBy(std::string weight_column, + std::vector column_specs) const { + auto qt_impl = impl_->WavgBy(std::move(weight_column), std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::TailBy(int64_t n, std::vector columnSpecs) const { - auto qt_impl = impl_->TailBy(n, std::move(columnSpecs)); +TableHandle TableHandle::TailBy(int64_t n, std::vector column_specs) const { + auto qt_impl = impl_->TailBy(n, std::move(column_specs)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::HeadBy(int64_t n, std::vector columnSpecs) const { - auto qt_impl = impl_->HeadBy(n, std::move(columnSpecs)); +TableHandle TableHandle::HeadBy(int64_t n, std::vector column_specs) const { + auto qt_impl = impl_->HeadBy(n, std::move(column_specs)); return TableHandle(std::move(qt_impl)); } @@ -437,40 +439,40 @@ TableHandle TableHandle::Tail(int64_t n) const { return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::Ungroup(bool nullFill, std::vector groupByColumns) const { - auto qt_impl = impl_->Ungroup(nullFill, std::move(groupByColumns)); +TableHandle TableHandle::Ungroup(bool null_fill, std::vector group_by_columns) const { + auto qt_impl = impl_->Ungroup(null_fill, std::move(group_by_columns)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::Merge(std::string keyColumn, std::vector sources) const { +TableHandle TableHandle::Merge(std::string key_columns, std::vector sources) const { std::vector source_handles; source_handles.reserve(sources.size() + 1); source_handles.push_back(impl_->Ticket()); for (const auto &s : sources) { source_handles.push_back(s.Impl()->Ticket()); } - auto qt_impl = impl_->Merge(std::move(keyColumn), std::move(source_handles)); + auto qt_impl = impl_->Merge(std::move(key_columns), std::move(source_handles)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::CrossJoin(const TableHandle &rightSide, - std::vector columnsToMatch, std::vector columnsToAdd) const { - auto qt_impl = impl_->CrossJoin(*rightSide.impl_, std::move(columnsToMatch), - std::move(columnsToAdd)); +TableHandle TableHandle::CrossJoin(const TableHandle &right_side, + std::vector columns_to_match, std::vector columns_to_add) const { + auto qt_impl = impl_->CrossJoin(*right_side.impl_, std::move(columns_to_match), + std::move(columns_to_add)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::NaturalJoin(const TableHandle &rightSide, - std::vector columnsToMatch, std::vector columnsToAdd) const { - auto qt_impl = impl_->NaturalJoin(*rightSide.impl_, std::move(columnsToMatch), - std::move(columnsToAdd)); +TableHandle TableHandle::NaturalJoin(const TableHandle &right_side, + std::vector columns_to_match, std::vector columns_to_add) const { + auto qt_impl = impl_->NaturalJoin(*right_side.impl_, std::move(columns_to_match), + std::move(columns_to_add)); return TableHandle(std::move(qt_impl)); } -TableHandle TableHandle::ExactJoin(const TableHandle &rightSide, - std::vector columnsToMatch, std::vector columnsToAdd) const { - auto qt_impl = impl_->ExactJoin(*rightSide.impl_, std::move(columnsToMatch), - std::move(columnsToAdd)); +TableHandle TableHandle::ExactJoin(const TableHandle &right_side, + std::vector columns_to_match, std::vector columns_to_add) const { + auto qt_impl = impl_->ExactJoin(*right_side.impl_, std::move(columns_to_match), + std::move(columns_to_add)); return TableHandle(std::move(qt_impl)); } @@ -550,9 +552,9 @@ std::shared_ptr TableHandle::Subscribe( } std::shared_ptr -TableHandle::Subscribe(onTickCallback_t onTick, void *onTickUserData, - onErrorCallback_t onError, void *onErrorUserData) { - return impl_->Subscribe(onTick, onTickUserData, onError, onErrorUserData); +TableHandle::Subscribe(onTickCallback_t on_tick, void *on_tick_user_data, + onErrorCallback_t on_error, void *on_error_user_data) { + return impl_->Subscribe(on_tick, on_tick_user_data, on_error, on_error_user_data); } void TableHandle::Unsubscribe(std::shared_ptr callback) { @@ -563,9 +565,9 @@ const std::string &TableHandle::GetTicketAsString() const { return impl_->Ticket().ticket(); } -std::string TableHandle::ToString(bool wantHeaders) const { +std::string TableHandle::ToString(bool want_headers) const { SimpleOstringstream oss; - oss << Stream(wantHeaders); + oss << Stream(want_headers); return std::move(oss.str()); } @@ -590,12 +592,12 @@ void PrintTableData(std::ostream &s, const TableHandle &table_handle, bool want_ } while (true) { - arrow::flight::FlightStreamChunk chunk; - OkOrThrow(DEEPHAVEN_LOCATION_EXPR(fsr->Next(&chunk))); - if (chunk.data == nullptr) { + auto chunk = fsr->Next(); + OkOrThrow(DEEPHAVEN_LOCATION_EXPR(chunk)); + if (chunk->data == nullptr) { break; } - const auto *data = chunk.data.get(); + const auto *data = chunk->data.get(); const auto &columns = data->columns(); for (int64_t row_num = 0; row_num < data->num_rows(); ++row_num) { if (row_num != 0) { diff --git a/cpp-client/deephaven/dhclient/src/flight.cc b/cpp-client/deephaven/dhclient/src/flight.cc index 3f9257da04d..165377785f6 100644 --- a/cpp-client/deephaven/dhclient/src/flight.cc +++ b/cpp-client/deephaven/dhclient/src/flight.cc @@ -16,17 +16,17 @@ FlightWrapper TableHandleManager::CreateFlightWrapper() const { FlightWrapper::FlightWrapper(std::shared_ptr impl) : impl_(std::move(impl)) {} FlightWrapper::~FlightWrapper() = default; -std::shared_ptr FlightWrapper::GetFlightStreamReader( +std::unique_ptr FlightWrapper::GetFlightStreamReader( const TableHandle &table) const { arrow::flight::FlightCallOptions options; AddHeaders(&options); - std::unique_ptr fsr; arrow::flight::Ticket tkt; tkt.ticket = table.Impl()->Ticket().ticket(); - OkOrThrow(DEEPHAVEN_LOCATION_EXPR(impl_->Server()->FlightClient()->DoGet(options, tkt, &fsr))); - return fsr; + auto fsr_result = impl_->Server()->FlightClient()->DoGet(options, tkt); + OkOrThrow(DEEPHAVEN_LOCATION_EXPR(fsr_result)); + return std::move(*fsr_result); } void FlightWrapper::AddHeaders(arrow::flight::FlightCallOptions *options) const { diff --git a/cpp-client/deephaven/dhclient/src/utility/arrow_util.cc b/cpp-client/deephaven/dhclient/src/utility/arrow_util.cc index e7c6ceeec30..70ed283164b 100644 --- a/cpp-client/deephaven/dhclient/src/utility/arrow_util.cc +++ b/cpp-client/deephaven/dhclient/src/utility/arrow_util.cc @@ -10,8 +10,6 @@ #include #include "deephaven/dhcore/utility/utility.h" -using namespace std; - namespace deephaven::client::utility { void OkOrThrow(const deephaven::dhcore::utility::DebugInfo &debug_info, const arrow::Status &status) {