From b26bfc40d4a85d0a685d7a985e1541828f613027 Mon Sep 17 00:00:00 2001 From: Rafael Telles Date: Tue, 2 Nov 2021 14:54:53 -0300 Subject: [PATCH] [C++] Other fixes for ratification (#190) * Rename sqlClient to sql_client on test_app_cli.cc * Add missing parameters on PreparedStatement constructor * Add NOTE to PreparedStatement destructor * Parse PreparedStatement's dataset and parameters schema when constructing * Rename Flight SQL Actions constants * Remove unnecessary 'using' keyword on server.h * Clean up header files and includes * Rename getters for schemas on PreparedStatement and make them const * Handle possible protobuf parsing errors on server.cc * Move CreateStatementQueryTicket implementation to sql namespace --- cpp/src/arrow/flight/flight_sql/api.h | 2 +- cpp/src/arrow/flight/flight_sql/client.cc | 75 ++++++++++--------- cpp/src/arrow/flight/flight_sql/client.h | 37 +++++---- .../arrow/flight/flight_sql/client_internal.h | 2 + .../arrow/flight/flight_sql/client_test.cc | 9 +-- .../flight_sql/example/sqlite_server.cc | 14 ++-- .../flight/flight_sql/example/sqlite_server.h | 11 +-- .../flight_sql/example/sqlite_statement.cc | 7 +- .../flight_sql/example/sqlite_statement.h | 4 +- .../example/sqlite_statement_batch_reader.cc | 8 +- .../example/sqlite_statement_batch_reader.h | 5 +- .../sqlite_tables_schema_batch_reader.cpp | 6 +- .../sqlite_tables_schema_batch_reader.h | 9 +-- cpp/src/arrow/flight/flight_sql/server.cc | 55 +++++++------- cpp/src/arrow/flight/flight_sql/server.h | 49 ++++++------ .../arrow/flight/flight_sql/server_test.cc | 6 +- .../arrow/flight/flight_sql/test_app_cli.cc | 42 +++++------ .../flight/flight_sql/test_server_cli.cc | 15 ++-- 18 files changed, 179 insertions(+), 177 deletions(-) diff --git a/cpp/src/arrow/flight/flight_sql/api.h b/cpp/src/arrow/flight/flight_sql/api.h index 8d5124ee86d1d..ddb83b25b1c22 100644 --- a/cpp/src/arrow/flight/flight_sql/api.h +++ b/cpp/src/arrow/flight/flight_sql/api.h @@ -17,4 +17,4 @@ #pragma once -#include "arrow/flight/flight_sql/client.h" +#include diff --git a/cpp/src/arrow/flight/flight_sql/client.cc b/cpp/src/arrow/flight/flight_sql/client.cc index e960833112939..8e0fc34f55d39 100644 --- a/cpp/src/arrow/flight/flight_sql/client.cc +++ b/cpp/src/arrow/flight/flight_sql/client.cc @@ -27,8 +27,7 @@ #include #include -#include -#include +namespace pb = arrow::flight::protocol; namespace arrow { namespace flight { @@ -41,17 +40,21 @@ FlightSqlClient::FlightSqlClient(std::unique_ptr client) : impl_(internal::FlightClientImpl_Create(std::move(client))) {} FlightSqlClient::PreparedStatement::PreparedStatement( - std::shared_ptr client, const std::string& query, - pb::sql::ActionCreatePreparedStatementResult& prepared_statement_result, + std::shared_ptr client, std::string handle, + std::shared_ptr dataset_schema, std::shared_ptr parameter_schema, FlightCallOptions options) - : client_(client), + : client_(std::move(client)), options_(std::move(options)), - prepared_statement_result_(std::move(prepared_statement_result)), + handle_(std::move(handle)), + dataset_schema_(std::move(dataset_schema)), + parameter_schema_(std::move(parameter_schema)), is_closed_(false) {} FlightSqlClient::~FlightSqlClient() = default; FlightSqlClient::PreparedStatement::~PreparedStatement() { + if (IsClosed()) return; + const Status status = Close(); if (!status.ok()) { ARROW_LOG(ERROR) << "Failed to delete PreparedStatement: " << status.ToString(); @@ -290,8 +293,28 @@ FlightSqlClient::Prepare(const FlightCallOptions& options, const std::string& qu return Status::Invalid("Unable to unpack ActionCreatePreparedStatementResult"); } - return std::make_shared(impl_, query, prepared_statement_result, - options); + const std::string& serialized_dataset_schema = + prepared_statement_result.dataset_schema(); + const std::string& serialized_parameter_schema = + prepared_statement_result.parameter_schema(); + + std::shared_ptr dataset_schema; + if (!serialized_dataset_schema.empty()) { + io::BufferReader dataset_schema_reader(serialized_dataset_schema); + ipc::DictionaryMemo in_memo; + ARROW_ASSIGN_OR_RAISE(dataset_schema, ReadSchema(&dataset_schema_reader, &in_memo)); + } + std::shared_ptr parameter_schema; + if (!serialized_parameter_schema.empty()) { + io::BufferReader parameter_schema_reader(serialized_parameter_schema); + ipc::DictionaryMemo in_memo; + ARROW_ASSIGN_OR_RAISE(parameter_schema, + ReadSchema(¶meter_schema_reader, &in_memo)); + } + auto handle = prepared_statement_result.prepared_statement_handle(); + + return std::make_shared(impl_, handle, dataset_schema, + parameter_schema, options); } arrow::Result> FlightSqlClient::PreparedStatement::Execute() { @@ -301,8 +324,7 @@ arrow::Result> FlightSqlClient::PreparedStatement::E pb::sql::CommandPreparedStatementQuery execute_query_command; - execute_query_command.set_prepared_statement_handle( - prepared_statement_result_.prepared_statement_handle()); + execute_query_command.set_prepared_statement_handle(handle_); google::protobuf::Any any; any.PackFrom(execute_query_command); @@ -336,8 +358,7 @@ arrow::Result FlightSqlClient::PreparedStatement::ExecuteUpdate() { } pb::sql::CommandPreparedStatementUpdate command; - command.set_prepared_statement_handle( - prepared_statement_result_.prepared_statement_handle()); + command.set_prepared_statement_handle(handle_); const FlightDescriptor& descriptor = GetFlightDescriptorForCommand(command); std::unique_ptr writer; std::unique_ptr reader; @@ -377,26 +398,12 @@ Status FlightSqlClient::PreparedStatement::SetParameters( bool FlightSqlClient::PreparedStatement::IsClosed() { return is_closed_; } -arrow::Result> -FlightSqlClient::PreparedStatement::GetResultSetSchema() { - auto& args = prepared_statement_result_.dataset_schema(); - std::shared_ptr schema_buffer = std::make_shared(args); - - io::BufferReader reader(schema_buffer); - - ipc::DictionaryMemo in_memo; - return ReadSchema(&reader, &in_memo); +std::shared_ptr FlightSqlClient::PreparedStatement::dataset_schema() const { + return dataset_schema_; } -arrow::Result> -FlightSqlClient::PreparedStatement::GetParameterSchema() { - auto& args = prepared_statement_result_.parameter_schema(); - std::shared_ptr schema_buffer = std::make_shared(args); - - io::BufferReader reader(schema_buffer); - - ipc::DictionaryMemo in_memo; - return ReadSchema(&reader, &in_memo); +std::shared_ptr FlightSqlClient::PreparedStatement::parameter_schema() const { + return parameter_schema_; } Status FlightSqlClient::PreparedStatement::Close() { @@ -405,8 +412,7 @@ Status FlightSqlClient::PreparedStatement::Close() { } google::protobuf::Any command; pb::sql::ActionClosePreparedStatementRequest request; - request.set_prepared_statement_handle( - prepared_statement_result_.prepared_statement_handle()); + request.set_prepared_statement_handle(handle_); command.PackFrom(request); @@ -431,11 +437,6 @@ arrow::Result> FlightSqlClient::GetSqlInfo( return GetFlightInfoForCommand(*impl_, options, command); } -arrow::Result> FlightSqlClient::GetSqlInfo( - const FlightCallOptions& options, const std::vector& sql_info) { - return GetSqlInfo(options, reinterpret_cast&>(sql_info)); -} - } // namespace sql } // namespace flight } // namespace arrow diff --git a/cpp/src/arrow/flight/flight_sql/client.h b/cpp/src/arrow/flight/flight_sql/client.h index b1ddf5ddef894..218e949ed89dd 100644 --- a/cpp/src/arrow/flight/flight_sql/client.h +++ b/cpp/src/arrow/flight/flight_sql/client.h @@ -17,15 +17,13 @@ #pragma once -#include #include -#include #include #include #include -#include -namespace pb = arrow::flight::protocol; +#include +#include namespace arrow { namespace flight { @@ -159,13 +157,6 @@ class ARROW_EXPORT FlightSqlClient { arrow::Result> GetSqlInfo(const FlightCallOptions& options, const std::vector& sql_info); - /// \brief Request a list of SQL information. - /// \param[in] options RPC-layer hints for this call. - /// \param[in] sql_info the SQL info required. - /// \return The FlightInfo describing where to access the dataset. - arrow::Result> GetSqlInfo( - const FlightCallOptions& options, const std::vector& sql_info); - /// \brief Create a prepared statement object. /// \param[in] options RPC-layer hints for this call. /// \param[in] query The query that will be executed. @@ -178,21 +169,29 @@ class ARROW_EXPORT FlightSqlClient { class PreparedStatement { std::shared_ptr client_; FlightCallOptions options_; - pb::sql::ActionCreatePreparedStatementResult prepared_statement_result_; + std::string handle_; + std::shared_ptr dataset_schema_; + std::shared_ptr parameter_schema_; std::shared_ptr parameter_binding_; bool is_closed_; public: /// \brief Constructor for the PreparedStatement class. - /// \param[in] query The query that will be executed. - PreparedStatement( - std::shared_ptr client_, const std::string& query, - pb::sql::ActionCreatePreparedStatementResult& prepared_statement_result, - FlightCallOptions options); + /// \param[in] client Client object used to make the RPC requests. + /// \param[in] handle Handle for this prepared statement. + /// \param[in] dataset_schema Schema of the resulting dataset. + /// \param[in] parameter_schema Schema of the parameters (if any). + /// \param[in] options RPC-layer hints for this call. + PreparedStatement(std::shared_ptr client, + std::string handle, std::shared_ptr dataset_schema, + std::shared_ptr parameter_schema, + FlightCallOptions options); /// \brief Default destructor for the PreparedStatement class. /// The destructor will call the Close method from the class in order, /// to send a request to close the PreparedStatement. + /// NOTE: It is best to explicitly close the PreparedStatement, otherwise + /// errors can't be caught. ~PreparedStatement(); /// \brief Executes the prepared statement query on the server. @@ -205,11 +204,11 @@ class ARROW_EXPORT FlightSqlClient { /// \brief Retrieve the parameter schema from the query. /// \return The parameter schema from the query. - arrow::Result> GetParameterSchema(); + std::shared_ptr parameter_schema() const; /// \brief Retrieve the ResultSet schema from the query. /// \return The ResultSet schema from the query. - arrow::Result> GetResultSetSchema(); + std::shared_ptr dataset_schema() const; /// \brief Set a RecordBatch that contains the parameters that will be bind. /// \param parameter_binding_ The parameters that will be bind. diff --git a/cpp/src/arrow/flight/flight_sql/client_internal.h b/cpp/src/arrow/flight/flight_sql/client_internal.h index 12ac4cd6e1da1..23eba399b9dd3 100644 --- a/cpp/src/arrow/flight/flight_sql/client_internal.h +++ b/cpp/src/arrow/flight/flight_sql/client_internal.h @@ -17,6 +17,8 @@ #include +#include + namespace arrow { namespace flight { diff --git a/cpp/src/arrow/flight/flight_sql/client_test.cc b/cpp/src/arrow/flight/flight_sql/client_test.cc index 2e9886b291feb..13851aae461f6 100644 --- a/cpp/src/arrow/flight/flight_sql/client_test.cc +++ b/cpp/src/arrow/flight/flight_sql/client_test.cc @@ -396,7 +396,7 @@ TEST(TestFlightSqlClient, TestPreparedStatementExecuteParameterBinding) { ASSERT_OK_AND_ASSIGN(auto prepared_statement, sql_client.Prepare(call_options, query)); - ASSERT_OK_AND_ASSIGN(auto parameter_schema, prepared_statement->GetParameterSchema()); + auto parameter_schema = prepared_statement->parameter_schema(); arrow::Int64Builder int_builder; ASSERT_OK(int_builder.Append(1)); @@ -459,10 +459,9 @@ TEST(TestFlightSqlClient, TestGetSqlInfo) { auto client_mock = std::make_shared(); FlightSqlClient sql_client(client_mock); - std::vector sql_info{ - pb::sql::SqlInfo::FLIGHT_SQL_SERVER_NAME, - pb::sql::SqlInfo::FLIGHT_SQL_SERVER_VERSION, - pb::sql::SqlInfo::FLIGHT_SQL_SERVER_ARROW_VERSION}; + std::vector sql_info{pb::sql::SqlInfo::FLIGHT_SQL_SERVER_NAME, + pb::sql::SqlInfo::FLIGHT_SQL_SERVER_VERSION, + pb::sql::SqlInfo::FLIGHT_SQL_SERVER_ARROW_VERSION}; pb::sql::CommandGetSqlInfo command; for (const auto& info : sql_info) command.add_info(info); diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_server.cc b/cpp/src/arrow/flight/flight_sql/example/sqlite_server.cc index 43697ae6f4480..20b04a050ed3c 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_server.cc +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_server.cc @@ -15,8 +15,12 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/flight_sql/example/sqlite_server.h" - +#include +#include +#include +#include +#include +#include #include #include @@ -25,12 +29,6 @@ #include #include -#include "arrow/api.h" -#include "arrow/flight/flight_sql/example/sqlite_statement.h" -#include "arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h" -#include "arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.h" -#include "arrow/flight/flight_sql/server.h" - namespace arrow { namespace flight { namespace sql { diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_server.h b/cpp/src/arrow/flight/flight_sql/example/sqlite_server.h index 8fdcec87d8d9e..d7c86a4682eb8 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_server.h +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_server.h @@ -17,16 +17,17 @@ #pragma once +#include +#include +#include +#include #include #include #include #include - -#include "arrow/api.h" -#include "arrow/flight/flight_sql/example/sqlite_statement.h" -#include "arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h" -#include "arrow/flight/flight_sql/server.h" +#include +#include namespace arrow { namespace flight { diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.cc b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.cc index 723f4323efc1c..76ef4e4ce2a72 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.cc +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.cc @@ -15,15 +15,12 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/flight_sql/example/sqlite_statement.h" - +#include +#include #include #include -#include "arrow/api.h" -#include "arrow/flight/flight_sql/example/sqlite_server.h" - namespace arrow { namespace flight { namespace sql { diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.h b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.h index ee9191e42b3c8..d0285dbd96ef9 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.h +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement.h @@ -17,9 +17,11 @@ #pragma once +#include #include -#include "arrow/api.h" +#include +#include namespace arrow { namespace flight { diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.cc b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.cc index 27c3d01816fb9..f510b31904838 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.cc +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.cc @@ -15,13 +15,11 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h" - +#include +#include +#include #include -#include "arrow/api.h" -#include "arrow/flight/flight_sql/example/sqlite_statement.h" - #define STRING_BUILDER_CASE(TYPE_CLASS, STMT, COLUMN) \ case TYPE_CLASS##Type::type_id: { \ int bytes = sqlite3_column_bytes(STMT, COLUMN); \ diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h index 2ae8db6e8e6a4..336592807dd50 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h @@ -17,10 +17,11 @@ #pragma once +#include +#include #include -#include "arrow/api.h" -#include "arrow/flight/flight_sql/example/sqlite_statement.h" +#include namespace arrow { namespace flight { diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.cpp b/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.cpp index d78684836e470..229057aa23015 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.cpp +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.cpp @@ -15,7 +15,7 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.h" +#include #include #include @@ -24,8 +24,8 @@ #include -#include "arrow/flight/flight_sql/example/sqlite_server.h" -#include "arrow/flight/flight_sql/example/sqlite_statement.h" +#include +#include namespace arrow { namespace flight { diff --git a/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.h b/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.h index 0fe7cef7efb91..e8f157e287aba 100644 --- a/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.h +++ b/cpp/src/arrow/flight/flight_sql/example/sqlite_tables_schema_batch_reader.h @@ -17,14 +17,13 @@ #pragma once +#include +#include #include #include -#include -#include - -#include "arrow/flight/flight_sql/example/sqlite_statement.h" -#include "arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h" +#include +#include namespace arrow { namespace flight { diff --git a/cpp/src/arrow/flight/flight_sql/server.cc b/cpp/src/arrow/flight/flight_sql/server.cc index e0bbaeafdfdf5..90834df1f3c04 100644 --- a/cpp/src/arrow/flight/flight_sql/server.cc +++ b/cpp/src/arrow/flight/flight_sql/server.cc @@ -18,30 +18,16 @@ // Interfaces to use for defining Flight RPC servers. API should be considered // experimental for now -#include "arrow/flight/flight_sql/server.h" - +#include +#include #include +#include #include #include #include #include -#include "arrow/api.h" -#include "arrow/buffer.h" - -std::string arrow::flight::sql::FlightSqlServerBase::CreateStatementQueryTicket( - const std::string& statement_handle) { - protocol::sql::TicketStatementQuery ticket_statement_query; - ticket_statement_query.set_statement_handle(statement_handle); - - google::protobuf::Any ticket; - ticket.PackFrom(ticket_statement_query); - - const std::string& ticket_string = ticket.SerializeAsString(); - return ticket_string; -} - namespace arrow { namespace flight { namespace sql { @@ -248,6 +234,18 @@ ParseActionClosePreparedStatementRequest(const google::protobuf::Any& any) { return result; } +std::string FlightSqlServerBase::CreateStatementQueryTicket( + const std::string& statement_handle) { + protocol::sql::TicketStatementQuery ticket_statement_query; + ticket_statement_query.set_statement_handle(statement_handle); + + google::protobuf::Any ticket; + ticket.PackFrom(ticket_statement_query); + + const std::string& ticket_string = ticket.SerializeAsString(); + return ticket_string; +} + Status FlightSqlServerBase::GetFlightInfo(const ServerCallContext& context, const FlightDescriptor& request, std::unique_ptr* info) { @@ -354,7 +352,9 @@ Status FlightSqlServerBase::DoPut(const ServerCallContext& context, const FlightDescriptor& request = reader->descriptor(); google::protobuf::Any any; - any.ParseFromArray(request.cmd.data(), static_cast(request.cmd.size())); + if (!any.ParseFromArray(request.cmd.data(), static_cast(request.cmd.size()))) { + return Status::Invalid("Unable to parse command."); + } if (any.Is()) { ARROW_ASSIGN_OR_RAISE(StatementUpdate internal_command, @@ -395,18 +395,20 @@ Status FlightSqlServerBase::DoPut(const ServerCallContext& context, Status FlightSqlServerBase::ListActions(const ServerCallContext& context, std::vector* actions) { - *actions = {FlightSqlServerBase::FLIGHT_SQL_CREATE_PREPARED_STATEMENT, - FlightSqlServerBase::FLIGHT_SQL_CLOSE_PREPARED_STATEMENT}; + *actions = {FlightSqlServerBase::kCreatePreparedStatementActionType, + FlightSqlServerBase::kClosePreparedStatementActionType}; return Status::OK(); } Status FlightSqlServerBase::DoAction(const ServerCallContext& context, const Action& action, std::unique_ptr* result_stream) { - if (action.type == FlightSqlServerBase::FLIGHT_SQL_CREATE_PREPARED_STATEMENT.type) { + if (action.type == FlightSqlServerBase::kCreatePreparedStatementActionType.type) { google::protobuf::Any any_command; - any_command.ParseFromArray(action.body->data(), - static_cast(action.body->size())); + if (!any_command.ParseFromArray(action.body->data(), + static_cast(action.body->size()))) { + return Status::Invalid("Unable to parse action."); + } ARROW_ASSIGN_OR_RAISE(ActionCreatePreparedStatementRequest internal_command, ParseActionCreatePreparedStatementRequest(any_command)); @@ -432,10 +434,11 @@ Status FlightSqlServerBase::DoAction(const ServerCallContext& context, *result_stream = std::unique_ptr(new SimpleResultStream({Result{buf}})); return Status::OK(); - } else if (action.type == - FlightSqlServerBase::FLIGHT_SQL_CLOSE_PREPARED_STATEMENT.type) { + } else if (action.type == FlightSqlServerBase::kClosePreparedStatementActionType.type) { google::protobuf::Any any; - any.ParseFromArray(action.body->data(), static_cast(action.body->size())); + if (!any.ParseFromArray(action.body->data(), static_cast(action.body->size()))) { + return Status::Invalid("Unable to parse action."); + } ARROW_ASSIGN_OR_RAISE(ActionClosePreparedStatementRequest internal_command, ParseActionClosePreparedStatementRequest(any)); diff --git a/cpp/src/arrow/flight/flight_sql/server.h b/cpp/src/arrow/flight/flight_sql/server.h index 79bc293a7e05d..c13ec5f12561c 100644 --- a/cpp/src/arrow/flight/flight_sql/server.h +++ b/cpp/src/arrow/flight/flight_sql/server.h @@ -20,45 +20,50 @@ #pragma once +#include +#include +#include #include -#include "arrow/api.h" -#include "arrow/flight/flight_sql/example/sqlite_statement.h" -#include "arrow/flight/flight_sql/example/sqlite_statement_batch_reader.h" -#include "arrow/flight/flight_sql/server.h" +#include +#include namespace arrow { namespace flight { namespace sql { -using StatementQuery = struct StatementQuery { std::string query; }; +struct StatementQuery { + std::string query; +}; -using StatementUpdate = struct StatementUpdate { std::string query; }; +struct StatementUpdate { + std::string query; +}; -using StatementQueryTicket = struct StatementQueryTicket { +struct StatementQueryTicket { std::string statement_handle; }; -using PreparedStatementQuery = struct PreparedStatementQuery { +struct PreparedStatementQuery { std::string prepared_statement_handle; }; -using PreparedStatementUpdate = struct PreparedStatementUpdate { +struct PreparedStatementUpdate { std::string prepared_statement_handle; }; -using GetSqlInfo = struct GetSqlInfo { +struct GetSqlInfo { // TODO: To be implemented. }; -using GetSchemas = struct GetSchemas { +struct GetSchemas { bool has_catalog; std::string catalog; bool has_schema_filter_pattern; std::string schema_filter_pattern; }; -using GetTables = struct GetTables { +struct GetTables { bool has_catalog; std::string catalog; bool has_schema_filter_pattern; @@ -69,7 +74,7 @@ using GetTables = struct GetTables { bool include_schema; }; -using GetPrimaryKeys = struct GetPrimaryKeys { +struct GetPrimaryKeys { bool has_catalog; std::string catalog; bool has_schema; @@ -77,7 +82,7 @@ using GetPrimaryKeys = struct GetPrimaryKeys { std::string table; }; -using GetExportedKeys = struct GetExportedKeys { +struct GetExportedKeys { bool has_catalog; std::string catalog; bool has_schema; @@ -85,7 +90,7 @@ using GetExportedKeys = struct GetExportedKeys { std::string table; }; -using GetImportedKeys = struct GetImportedKeys { +struct GetImportedKeys { bool has_catalog; std::string catalog; bool has_schema; @@ -93,7 +98,7 @@ using GetImportedKeys = struct GetImportedKeys { std::string table; }; -using GetCrossReference = struct GetCrossReference { +struct GetCrossReference { bool has_pk_catalog; std::string pk_catalog; bool has_pk_schema; @@ -106,21 +111,21 @@ using GetCrossReference = struct GetCrossReference { std::string fk_table; }; -using ActionCreatePreparedStatementRequest = struct ActionCreatePreparedStatementRequest { +struct ActionCreatePreparedStatementRequest { std::string query; }; -using ActionClosePreparedStatementRequest = struct ActionClosePreparedStatementRequest { +struct ActionClosePreparedStatementRequest { std::string prepared_statement_handle; }; -using ActionCreatePreparedStatementResult = struct ActionCreatePreparedStatementResult { +struct ActionCreatePreparedStatementResult { std::shared_ptr dataset_schema; std::shared_ptr parameter_schema; std::string prepared_statement_handle; }; -class FlightSqlServerBase : public FlightServerBase { +class ARROW_EXPORT FlightSqlServerBase : public FlightServerBase { public: Status GetFlightInfo(const ServerCallContext& context, const FlightDescriptor& request, std::unique_ptr* info) override; @@ -132,12 +137,12 @@ class FlightSqlServerBase : public FlightServerBase { std::unique_ptr reader, std::unique_ptr writer) override; - const ActionType FLIGHT_SQL_CREATE_PREPARED_STATEMENT = + const ActionType kCreatePreparedStatementActionType = ActionType{"CreatePreparedStatement", "Creates a reusable prepared statement resource on the server.\n" "Request Message: ActionCreatePreparedStatementRequest\n" "Response Message: ActionCreatePreparedStatementResult"}; - const ActionType FLIGHT_SQL_CLOSE_PREPARED_STATEMENT = + const ActionType kClosePreparedStatementActionType = ActionType{"ClosePreparedStatement", "Closes a reusable prepared statement resource on the server.\n" "Request Message: ActionClosePreparedStatementRequest\n" diff --git a/cpp/src/arrow/flight/flight_sql/server_test.cc b/cpp/src/arrow/flight/flight_sql/server_test.cc index f466b32058477..df1c0d971ee5b 100644 --- a/cpp/src/arrow/flight/flight_sql/server_test.cc +++ b/cpp/src/arrow/flight/flight_sql/server_test.cc @@ -17,8 +17,8 @@ #include #include +#include #include -#include #include #include #include @@ -348,7 +348,7 @@ TEST(TestFlightSqlServer, TestCommandPreparedStatementQueryWithParameterBinding) auto prepared_statement, sql_client->Prepare({}, "SELECT * FROM intTable WHERE keyName LIKE ?")); - ASSERT_OK_AND_ASSIGN(auto parameter_schema, prepared_statement->GetParameterSchema()); + auto parameter_schema = prepared_statement->parameter_schema(); const std::shared_ptr& expected_parameter_schema = arrow::schema({arrow::field("parameter_1", example::GetUnknownColumnDataType())}); @@ -428,7 +428,7 @@ TEST(TestFlightSqlServer, TestCommandPreparedStatementUpdateWithParameterBinding sql_client->Prepare( {}, "INSERT INTO INTTABLE (keyName, value) VALUES ('new_value', ?)")); - ASSERT_OK_AND_ASSIGN(auto parameter_schema, prepared_statement->GetParameterSchema()); + auto parameter_schema = prepared_statement->parameter_schema(); const std::shared_ptr& expected_parameter_schema = arrow::schema({arrow::field("parameter_1", example::GetUnknownColumnDataType())}); diff --git a/cpp/src/arrow/flight/flight_sql/test_app_cli.cc b/cpp/src/arrow/flight/flight_sql/test_app_cli.cc index c8e3de9fae56f..fb2a8890bfb62 100644 --- a/cpp/src/arrow/flight/flight_sql/test_app_cli.cc +++ b/cpp/src/arrow/flight/flight_sql/test_app_cli.cc @@ -114,10 +114,10 @@ Status RunMain() { call_options.headers.push_back(bearer_result.ValueOrDie()); } - FlightSqlClient sqlClient(std::move(client)); + FlightSqlClient sql_client(std::move(client)); if (FLAGS_command == "ExecuteUpdate") { - ARROW_ASSIGN_OR_RAISE(auto rows, sqlClient.ExecuteUpdate(call_options, FLAGS_query)); + ARROW_ASSIGN_OR_RAISE(auto rows, sql_client.ExecuteUpdate(call_options, FLAGS_query)); std::cout << "Result: " << rows << std::endl; @@ -127,19 +127,17 @@ Status RunMain() { std::unique_ptr info; if (FLAGS_command == "Execute") { - ARROW_ASSIGN_OR_RAISE(info, sqlClient.Execute(call_options, FLAGS_query)); + ARROW_ASSIGN_OR_RAISE(info, sql_client.Execute(call_options, FLAGS_query)); } else if (FLAGS_command == "GetCatalogs") { - ARROW_ASSIGN_OR_RAISE(info, sqlClient.GetCatalogs(call_options)); + ARROW_ASSIGN_OR_RAISE(info, sql_client.GetCatalogs(call_options)); } else if (FLAGS_command == "PreparedStatementExecute") { ARROW_ASSIGN_OR_RAISE(auto prepared_statement, - sqlClient.Prepare(call_options, FLAGS_query)); + sql_client.Prepare(call_options, FLAGS_query)); ARROW_ASSIGN_OR_RAISE(info, prepared_statement->Execute()); } else if (FLAGS_command == "PreparedStatementExecuteParameterBinding") { - ARROW_ASSIGN_OR_RAISE(auto prepared_statement, sqlClient.Prepare({}, FLAGS_query)); - ARROW_ASSIGN_OR_RAISE(auto parameter_schema, - prepared_statement->GetParameterSchema()); - ARROW_ASSIGN_OR_RAISE(auto result_set_schema, - prepared_statement->GetResultSetSchema()); + ARROW_ASSIGN_OR_RAISE(auto prepared_statement, sql_client.Prepare({}, FLAGS_query)); + auto parameter_schema = prepared_statement->parameter_schema(); + auto result_set_schema = prepared_statement->dataset_schema(); std::cout << result_set_schema->ToString(false) << std::endl; arrow::Int64Builder int_builder; @@ -153,30 +151,30 @@ Status RunMain() { ARROW_ASSIGN_OR_RAISE(info, prepared_statement->Execute()); } else if (FLAGS_command == "GetSchemas") { ARROW_ASSIGN_OR_RAISE( - info, sqlClient.GetSchemas(call_options, &FLAGS_catalog, &FLAGS_schema)); + info, sql_client.GetSchemas(call_options, &FLAGS_catalog, &FLAGS_schema)); } else if (FLAGS_command == "GetTableTypes") { - ARROW_ASSIGN_OR_RAISE(info, sqlClient.GetTableTypes(call_options)); + ARROW_ASSIGN_OR_RAISE(info, sql_client.GetTableTypes(call_options)); } else if (FLAGS_command == "GetTables") { std::vector table_types = {}; bool include_schema = false; - ARROW_ASSIGN_OR_RAISE(info, - sqlClient.GetTables(call_options, &FLAGS_catalog, &FLAGS_schema, - &FLAGS_table, include_schema, table_types)); + ARROW_ASSIGN_OR_RAISE( + info, sql_client.GetTables(call_options, &FLAGS_catalog, &FLAGS_schema, + &FLAGS_table, include_schema, table_types)); } else if (FLAGS_command == "GetExportedKeys") { - ARROW_ASSIGN_OR_RAISE(info, sqlClient.GetExportedKeys(call_options, &FLAGS_catalog, - &FLAGS_schema, FLAGS_table)); + ARROW_ASSIGN_OR_RAISE(info, sql_client.GetExportedKeys(call_options, &FLAGS_catalog, + &FLAGS_schema, FLAGS_table)); } else if (FLAGS_command == "GetImportedKeys") { - ARROW_ASSIGN_OR_RAISE(info, sqlClient.GetImportedKeys(call_options, &FLAGS_catalog, - &FLAGS_schema, FLAGS_table)); + ARROW_ASSIGN_OR_RAISE(info, sql_client.GetImportedKeys(call_options, &FLAGS_catalog, + &FLAGS_schema, FLAGS_table)); } else if (FLAGS_command == "GetPrimaryKeys") { - ARROW_ASSIGN_OR_RAISE(info, sqlClient.GetPrimaryKeys(call_options, &FLAGS_catalog, - &FLAGS_schema, FLAGS_table)); + ARROW_ASSIGN_OR_RAISE(info, sql_client.GetPrimaryKeys(call_options, &FLAGS_catalog, + &FLAGS_schema, FLAGS_table)); } if (info != NULLPTR && !boost::istarts_with(FLAGS_command, "PreparedStatementExecute")) { - return PrintResults(sqlClient, call_options, info); + return PrintResults(sql_client, call_options, info); } return Status::OK(); diff --git a/cpp/src/arrow/flight/flight_sql/test_server_cli.cc b/cpp/src/arrow/flight/flight_sql/test_server_cli.cc index 0e0d63a272d73..4ace087776558 100644 --- a/cpp/src/arrow/flight/flight_sql/test_server_cli.cc +++ b/cpp/src/arrow/flight/flight_sql/test_server_cli.cc @@ -15,20 +15,19 @@ // specific language governing permissions and limitations // under the License. +#include +#include +#include +#include +#include +#include +#include #include #include #include #include -#include "arrow/flight/flight_sql/example/sqlite_server.h" -#include "arrow/flight/server.h" -#include "arrow/flight/test_integration.h" -#include "arrow/flight/test_util.h" -#include "arrow/io/test_common.h" -#include "arrow/testing/json_integration.h" -#include "arrow/util/logging.h" - DEFINE_int32(port, 31337, "Server port to listen on"); std::unique_ptr g_server;