Skip to content

Commit

Permalink
[C++] Address ratification comments (round 4 - part 3) (apache#214)
Browse files Browse the repository at this point in the history 
* Make other methods from SQLite server example to return arrow::Result instead of Status

* Fix bug for null values in numeric columns on SQLite server example

* Add comment regarding to performance on sqlite_statement_batch_reader

* Separate GetSqlInfoResultMap from sqlite_server.h

* Remove unused parameter on DoPutCommandStatementUpdate
  • Loading branch information
@rafael-telles
rafael-telles committed Dec 3, 2021
1 parent c36b817 commit e8d8a13
Show file tree
Hide file tree
Showing 11 changed files with 349 additions and 294 deletions.
2 changes: 2 additions & 0 deletions cpp/src/arrow/flight/sql/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ add_arrow_test(flight_sql_test
SOURCES
client_test.cc
server_test.cc
example/sqlite_sql_info.cc
STATIC_LINK_LIBS
${ARROW_FLIGHT_SQL_TEST_LINK_LIBS}
LABELS
Expand All @@ -81,6 +82,7 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES)

add_executable(flight_sql_test_server
test_server_cli.cc
example/sqlite_sql_info.cc
example/sqlite_statement.cc
example/sqlite_statement_batch_reader.cc
example/sqlite_server.cc
Expand Down
33 changes: 11 additions & 22 deletions cpp/src/arrow/flight/sql/example/sqlite_server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <boost/uuid/uuid_io.hpp>

#include "arrow/api.h"
#include "arrow/flight/sql/example/sqlite_sql_info.h"
#include "arrow/flight/sql/example/sqlite_statement.h"
#include "arrow/flight/sql/example/sqlite_statement_batch_reader.h"
#include "arrow/flight/sql/example/sqlite_tables_schema_batch_reader.h"
Expand Down Expand Up @@ -204,6 +205,7 @@ arrow::Result<std::shared_ptr<SQLiteFlightSqlServer>> SQLiteFlightSqlServer::Cre
INSERT INTO intTable (keyName, value, foreignId) VALUES ('one', 1, 1);
INSERT INTO intTable (keyName, value, foreignId) VALUES ('zero', 0, 1);
INSERT INTO intTable (keyName, value, foreignId) VALUES ('negative one', -1, 1);
INSERT INTO intTable (keyName, value, foreignId) VALUES (NULL, NULL, NULL);
)"));

return result;
Expand Down Expand Up @@ -251,14 +253,11 @@ arrow::Result<std::unique_ptr<FlightInfo>> SQLiteFlightSqlServer::GetFlightInfoS
const FlightDescriptor& descriptor) {
const std::string& query = command.query;

std::shared_ptr<SqliteStatement> statement;
ARROW_ASSIGN_OR_RAISE(statement, SqliteStatement::Create(db_, query));
ARROW_ASSIGN_OR_RAISE(auto statement, SqliteStatement::Create(db_, query));

std::shared_ptr<Schema> schema;
ARROW_RETURN_NOT_OK(statement->GetSchema(&schema));
ARROW_ASSIGN_OR_RAISE(auto schema, statement->GetSchema());

std::string ticket_string;
ARROW_ASSIGN_OR_RAISE(ticket_string, CreateStatementQueryTicket(query));
ARROW_ASSIGN_OR_RAISE(auto ticket_string, CreateStatementQueryTicket(query));
std::vector<FlightEndpoint> endpoints{FlightEndpoint{{ticket_string}, {}}};
ARROW_ASSIGN_OR_RAISE(auto result,
FlightInfo::Make(*schema, descriptor, endpoints, -1, -1))
Expand Down Expand Up @@ -363,17 +362,12 @@ arrow::Result<std::unique_ptr<FlightDataStream>> SQLiteFlightSqlServer::DoGetTab
}

arrow::Result<int64_t> SQLiteFlightSqlServer::DoPutCommandStatementUpdate(
const ServerCallContext& context, const StatementUpdate& command,
std::unique_ptr<FlightMessageReader>& reader) {
const ServerCallContext& context, const StatementUpdate& command) {
const std::string& sql = command.query;

std::shared_ptr<SqliteStatement> statement;
ARROW_ASSIGN_OR_RAISE(statement, SqliteStatement::Create(db_, sql));

int64_t record_count;
ARROW_RETURN_NOT_OK(statement->ExecuteUpdate(&record_count));
ARROW_ASSIGN_OR_RAISE(auto statement, SqliteStatement::Create(db_, sql));

return record_count;
return statement->ExecuteUpdate();
}

arrow::Result<ActionCreatePreparedStatementResult>
Expand All @@ -385,8 +379,7 @@ SQLiteFlightSqlServer::CreatePreparedStatement(
boost::uuids::uuid uuid = uuid_generator_();
prepared_statements_[uuid] = statement;

std::shared_ptr<Schema> dataset_schema;
ARROW_RETURN_NOT_OK(statement->GetSchema(&dataset_schema));
ARROW_ASSIGN_OR_RAISE(auto dataset_schema, statement->GetSchema());

sqlite3_stmt* stmt = statement->GetSqlite3Stmt();
const int parameter_count = sqlite3_bind_parameter_count(stmt);
Expand Down Expand Up @@ -449,8 +442,7 @@ SQLiteFlightSqlServer::GetFlightInfoPreparedStatement(

std::shared_ptr<SqliteStatement> statement = search->second;

std::shared_ptr<Schema> schema;
ARROW_RETURN_NOT_OK(statement->GetSchema(&schema));
ARROW_ASSIGN_OR_RAISE(auto schema, statement->GetSchema());

return GetFlightInfoForCommand(descriptor, schema);
}
Expand Down Expand Up @@ -497,10 +489,7 @@ arrow::Result<int64_t> SQLiteFlightSqlServer::DoPutPreparedStatementUpdate(
sqlite3_stmt* stmt = statement->GetSqlite3Stmt();
ARROW_RETURN_NOT_OK(SetParametersOnSQLiteStatement(stmt, reader));

int64_t record_count;
ARROW_RETURN_NOT_OK(statement->ExecuteUpdate(&record_count));

return record_count;
return statement->ExecuteUpdate();
}

arrow::Result<std::unique_ptr<FlightInfo>> SQLiteFlightSqlServer::GetFlightInfoTableTypes(
Expand Down
198 changes: 1 addition & 197 deletions cpp/src/arrow/flight/sql/example/sqlite_server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
#include <string>

#include "arrow/api.h"
#include "arrow/flight/sql/FlightSql.pb.h"
#include "arrow/flight/sql/example/sqlite_statement.h"
#include "arrow/flight/sql/example/sqlite_statement_batch_reader.h"
#include "arrow/flight/sql/server.h"
Expand All @@ -36,200 +35,6 @@ namespace flight {
namespace sql {
namespace example {

namespace flight_sql_pb = arrow::flight::protocol::sql;

/// \brief Gets the mapping from SQL info ids to SqlInfoResult instances.
/// \return the cache.
inline SqlInfoResultMap GetSqlInfoResultMap() {
return {
{flight_sql_pb::SqlInfo::FLIGHT_SQL_SERVER_NAME,
SqlInfoResult(std::string("db_name"))},
{flight_sql_pb::SqlInfo::FLIGHT_SQL_SERVER_VERSION,
SqlInfoResult(std::string("sqlite 3"))},
{flight_sql_pb::SqlInfo::FLIGHT_SQL_SERVER_ARROW_VERSION,
SqlInfoResult(std::string("7.0.0-SNAPSHOT" /* Only an example */))},
{flight_sql_pb::SqlInfo::FLIGHT_SQL_SERVER_READ_ONLY, SqlInfoResult(false)},
{flight_sql_pb::SqlInfo::SQL_DDL_CATALOG,
SqlInfoResult(false /* SQLite 3 does not support catalogs */)},
{flight_sql_pb::SqlInfo::SQL_DDL_SCHEMA,
SqlInfoResult(false /* SQLite 3 does not support schemas */)},
{flight_sql_pb::SqlInfo::SQL_DDL_TABLE, SqlInfoResult(true)},
{flight_sql_pb::SqlInfo::SQL_IDENTIFIER_CASE,
SqlInfoResult(int64_t(flight_sql_pb::SqlSupportedCaseSensitivity::
SQL_CASE_SENSITIVITY_CASE_INSENSITIVE))},
{flight_sql_pb::SqlInfo::SQL_IDENTIFIER_QUOTE_CHAR,
SqlInfoResult(std::string("\""))},
{flight_sql_pb::SqlInfo::SQL_QUOTED_IDENTIFIER_CASE,
SqlInfoResult(int64_t(flight_sql_pb::SqlSupportedCaseSensitivity::
SQL_CASE_SENSITIVITY_CASE_INSENSITIVE))},
{flight_sql_pb::SqlInfo::SQL_ALL_TABLES_ARE_SELECTABLE, SqlInfoResult(true)},
{flight_sql_pb::SqlInfo::SQL_NULL_ORDERING,
SqlInfoResult(int64_t(flight_sql_pb::SqlNullOrdering::SQL_NULLS_SORTED_AT_START))},
{flight_sql_pb::SqlInfo::SQL_KEYWORDS,
SqlInfoResult(std::vector<std::string>({"ABORT",
"ACTION",
"ADD",
"AFTER",
"ALL",
"ALTER",
"ALWAYS",
"ANALYZE",
"AND",
"AS",
"ASC",
"ATTACH",
"AUTOINCREMENT",
"BEFORE",
"BEGIN",
"BETWEEN",
"BY",
"CASCADE",
"CASE",
"CAST",
"CHECK",
"COLLATE",
"COLUMN",
"COMMIT",
"CONFLICT",
"CONSTRAINT",
"CREATE",
"CROSS",
"CURRENT",
"CURRENT_DATE",
"CURRENT_TIME",
"CURRENT_TIMESTAMP",
"DATABASE",
"DEFAULT",
"DEFERRABLE",
"DEFERRED",
"DELETE",
"DESC",
"DETACH",
"DISTINCT",
"DO",
"DROP",
"EACH",
"ELSE",
"END",
"ESCAPE",
"EXCEPT",
"EXCLUDE",
"EXCLUSIVE",
"EXISTS",
"EXPLAIN",
"FAIL",
"FILTER",
"FIRST",
"FOLLOWING",
"FOR",
"FOREIGN",
"FROM",
"FULL",
"GENERATED",
"GLOB",
"GROUP",
"GROUPS",
"HAVING",
"IF",
"IGNORE",
"IMMEDIATE",
"IN",
"INDEX",
"INDEXED",
"INITIALLY",
"INNER",
"INSERT",
"INSTEAD",
"INTERSECT",
"INTO",
"IS",
"ISNULL",
"JOIN",
"KEY",
"LAST",
"LEFT",
"LIKE",
"LIMIT",
"MATCH",
"MATERIALIZED",
"NATURAL",
"NO",
"NOT",
"NOTHING",
"NOTNULL",
"NULL",
"NULLS",
"OF",
"OFFSET",
"ON",
"OR",
"ORDER",
"OTHERS",
"OUTER",
"OVER",
"PARTITION",
"PLAN",
"PRAGMA",
"PRECEDING",
"PRIMARY",
"QUERY",
"RAISE",
"RANGE",
"RECURSIVE",
"REFERENCES",
"REGEXP",
"REINDEX",
"RELEASE",
"RENAME",
"REPLACE",
"RESTRICT",
"RETURNING",
"RIGHT",
"ROLLBACK",
"ROW",
"ROWS",
"SAVEPOINT",
"SELECT",
"SET",
"TABLE",
"TEMP",
"TEMPORARY",
"THEN",
"TIES",
"TO",
"TRANSACTION",
"TRIGGER",
"UNBOUNDED",
"UNION",
"UNIQUE",
"UPDATE",
"USING",
"VACUUM",
"VALUES",
"VIEW",
"VIRTUAL",
"WHEN",
"WHERE",
"WINDOW",
"WITH",
"WITHOUT"}))},
{flight_sql_pb::SqlInfo::SQL_NUMERIC_FUNCTIONS,
SqlInfoResult(std::vector<std::string>(
{"acos", "acosh", "asin", "asinh", "atan", "atan2", "atanh", "ceil",
"ceiling", "cos", "cosh", "degrees", "exp", "floor", "ln", "log",
"log", "log10", "log2", "mod", "pi", "pow", "power", "radians",
"sin", "sinh", "sqrt", "tan", "tanh", "trunc"}))},
{flight_sql_pb::SqlInfo::SQL_STRING_FUNCTIONS,
SqlInfoResult(
std::vector<std::string>({"SUBSTR", "TRIM", "LTRIM", "RTRIM", "LENGTH",
"REPLACE", "UPPER", "LOWER", "INSTR"}))},
{flight_sql_pb::SqlInfo::SQL_SUPPORTS_CONVERT,
SqlInfoResult(std::unordered_map<int32_t, std::vector<int32_t>>(
{{flight_sql_pb::SqlSupportsConvert::SQL_CONVERT_BIGINT,
std::vector<int32_t>(
{flight_sql_pb::SqlSupportsConvert::SQL_CONVERT_INTEGER})}}))}};
}

/// \brief Convert a column type to a ArrowType.
/// \param sqlite_type the sqlite type.
/// \return The equivalent ArrowType.
Expand Down Expand Up @@ -274,8 +79,7 @@ class SQLiteFlightSqlServer : public FlightSqlServerBase {
arrow::Result<std::unique_ptr<FlightDataStream>> DoGetSchemas(
const ServerCallContext& context, const GetSchemas& command) override;
arrow::Result<int64_t> DoPutCommandStatementUpdate(
const ServerCallContext& context, const StatementUpdate& update,
std::unique_ptr<FlightMessageReader>& reader) override;
const ServerCallContext& context, const StatementUpdate& update) override;
arrow::Result<ActionCreatePreparedStatementResult> CreatePreparedStatement(
const ServerCallContext& context,
const ActionCreatePreparedStatementRequest& request) override;
Expand Down
Loading

0 comments on commit e8d8a13

Please sign in to comment.