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

Ongoing fixes #356

Merged
merged 21 commits into from
Aug 3, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 50 additions & 27 deletions README.md

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion contrib/folly
Submodule folly updated 1671 files
2 changes: 1 addition & 1 deletion contrib/googletest
Submodule googletest updated 176 files
3 changes: 2 additions & 1 deletion driver/api/impl/impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,8 @@ SQLRETURN fillBinding(
}

if (binding_info.c_type == SQL_C_DEFAULT) {
binding_info.c_type = convertSQLTypeToCType(statement.getTypeInfo(result_set.getColumnInfo(column_idx).type_without_parameters).sql_type);
const auto & column_info = result_set.getColumnInfo(column_idx);
binding_info.c_type = convertSQLTypeToCType(statement.getTypeInfo(column_info.type, column_info.type_without_parameters).sql_type);
}

if (
Expand Down
17 changes: 9 additions & 8 deletions driver/api/odbc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <Poco/Net/HTTPClientSession.h>
#include <Poco/NumberFormatter.h>
#include <Poco/Timezone.h>

#include <iostream>
#include <locale>
Expand Down Expand Up @@ -968,8 +969,8 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)(
: public ResultMutator
{
public:
explicit ColumnsMutator(Environment & env_)
: env(env_)
explicit ColumnsMutator(Statement & statement_)
: statement(statement_)
{
}

Expand All @@ -984,7 +985,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)(
TypeAst ast;

if (parser.parse(&ast)) {
tmp_column_info.assignTypeInfo(ast);
tmp_column_info.assignTypeInfo(ast, Poco::Timezone::name());

if (convertUnparametrizedTypeNameToTypeId(tmp_column_info.type_without_parameters) == DataSourceTypeId::Unknown) {
// Interpret all unknown types as String.
Expand All @@ -999,7 +1000,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)(
tmp_column_info.updateTypeInfo();
}, row.fields.at(5).data);

const TypeInfo & type_info = env.getTypeInfo(tmp_column_info.type, tmp_column_info.type_without_parameters);
const TypeInfo & type_info = statement.getTypeInfo(tmp_column_info.type, tmp_column_info.type_without_parameters);

row.fields.at(4).data = DataSourceType<DataSourceTypeId::Int16>{type_info.sql_type};
row.fields.at(5).data = DataSourceType<DataSourceTypeId::String>{type_info.sql_type_name};
Expand All @@ -1009,7 +1010,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)(
}

private:
Environment & env;
Statement & statement;
};

auto func = [&](Statement & statement) {
Expand Down Expand Up @@ -1093,7 +1094,7 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLColumns)(
}

query << " ORDER BY TABLE_CAT, TABLE_SCHEM, TABLE_NAME, ORDINAL_POSITION";
statement.executeQuery(query.str(), std::make_unique<ColumnsMutator>(statement.getParent().getParent()));
statement.executeQuery(query.str(), std::make_unique<ColumnsMutator>(statement));

return SQL_SUCCESS;
};
Expand Down Expand Up @@ -1171,13 +1172,13 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLGetTypeInfo)(
// are SQL_TYPE_DATE, SQL_TYPE_TIME, and SQL_TYPE_TIMESTAMP, respectively;
// in ODBC 2.x, the data types are SQL_DATE, SQL_TIME, and SQL_TIMESTAMP.
{
auto info = statement.getParent().getParent().getTypeInfo("Date");
auto info = statement.getTypeInfo("Date", "Date");
info.sql_type = SQL_DATE;
add_query_for_type("Date", info);
}

{
auto info = statement.getParent().getParent().getTypeInfo("DateTime");
auto info = statement.getTypeInfo("DateTime", "DateTime");
info.sql_type = SQL_TIMESTAMP;
add_query_for_type("DateTime", info);
}
Expand Down
5 changes: 5 additions & 0 deletions driver/config/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ void readDSNinfo(ConnInfo * ci, bool overwrite) {
GET_CONFIG(server, INI_SERVER, INI_SERVER_DEFAULT);
GET_CONFIG(port, INI_PORT, INI_PORT_DEFAULT);
GET_CONFIG(timeout, INI_TIMEOUT, INI_TIMEOUT_DEFAULT);
GET_CONFIG(verify_connection_early, INI_VERIFY_CONNECTION_EARLY, INI_VERIFY_CONNECTION_EARLY_DEFAULT);
GET_CONFIG(sslmode, INI_SSLMODE, INI_SSLMODE_DEFAULT);
GET_CONFIG(database, INI_DATABASE, INI_DATABASE_DEFAULT);
GET_CONFIG(huge_int_as_string, INI_HUGE_INT_AS_STRING, INI_HUGE_INT_AS_STRING_DEFAULT);
GET_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH, INI_STRINGMAXLENGTH_DEFAULT);
GET_CONFIG(driverlog, INI_DRIVERLOG, INI_DRIVERLOG_DEFAULT);
GET_CONFIG(driverlogfile, INI_DRIVERLOGFILE, INI_DRIVERLOGFILE_DEFAULT);
Expand Down Expand Up @@ -90,8 +92,10 @@ void writeDSNinfo(const ConnInfo * ci) {
WRITE_CONFIG(server, INI_SERVER);
WRITE_CONFIG(port, INI_PORT);
WRITE_CONFIG(timeout, INI_TIMEOUT);
WRITE_CONFIG(verify_connection_early, INI_VERIFY_CONNECTION_EARLY);
WRITE_CONFIG(sslmode, INI_SSLMODE);
WRITE_CONFIG(database, INI_DATABASE);
WRITE_CONFIG(huge_int_as_string, INI_HUGE_INT_AS_STRING);
WRITE_CONFIG(stringmaxlength, INI_STRINGMAXLENGTH);
WRITE_CONFIG(driverlog, INI_DRIVERLOG);
WRITE_CONFIG(driverlogfile, INI_DRIVERLOGFILE);
Expand Down Expand Up @@ -329,6 +333,7 @@ key_value_map_t readDSNInfo(const std::string & dsn_utf8) {
INI_HOST,
INI_PORT,
INI_TIMEOUT,
INI_VERIFY_CONNECTION_EARLY,
INI_SSLMODE,
INI_PRIVATEKEYFILE,
INI_CERTIFICATEFILE,
Expand Down
2 changes: 2 additions & 0 deletions driver/config/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ struct ConnInfo {
std::string server;
std::string port;
std::string timeout;
std::string verify_connection_early;
std::string sslmode;
std::string privateKeyFile;
std::string certificateFile;
std::string caLocation;
std::string database;
std::string huge_int_as_string;
std::string stringmaxlength;
std::string driverlog;
std::string driverlogfile;
Expand Down
4 changes: 4 additions & 0 deletions driver/config/ini_defines.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
#define INI_HOST "Host"
#define INI_PORT "Port" /* Port on which the ClickHouse is listening */
#define INI_TIMEOUT "Timeout" /* Connection timeout */
#define INI_VERIFY_CONNECTION_EARLY "VerifyConnectionEarly"
#define INI_SSLMODE "SSLMode" /* Use 'require' for https connections */
#define INI_PRIVATEKEYFILE "PrivateKeyFile"
#define INI_CERTIFICATEFILE "CertificateFile"
#define INI_CALOCATION "CALocation"
#define INI_PATH "Path" /* Path portion of the URL */
#define INI_DATABASE "Database" /* Database Name */
#define INI_HUGE_INT_AS_STRING "HugeIntAsString"
#define INI_STRINGMAXLENGTH "StringMaxLength"
#define INI_DRIVERLOG "DriverLog"
#define INI_DRIVERLOGFILE "DriverLogFile"
Expand All @@ -43,8 +45,10 @@
#define INI_SERVER_DEFAULT ""
#define INI_PORT_DEFAULT ""
#define INI_TIMEOUT_DEFAULT "30"
#define INI_VERIFY_CONNECTION_EARLY_DEFAULT "off"
#define INI_SSLMODE_DEFAULT ""
#define INI_DATABASE_DEFAULT ""
#define INI_HUGE_INT_AS_STRING_DEFAULT "off"
#define INI_STRINGMAXLENGTH_DEFAULT "1048575"

#ifdef NDEBUG
Expand Down
39 changes: 39 additions & 0 deletions driver/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ Connection::Connection(Environment & environment)
resetConfiguration();
}

const TypeInfo & Connection::getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const {
auto tmp_type_name = type_name;
auto tmp_type_name_without_parameters = type_name_without_parameters;

const auto tmp_type_without_parameters_id = convertUnparametrizedTypeNameToTypeId(tmp_type_name_without_parameters);

if (huge_int_as_string && tmp_type_without_parameters_id == DataSourceTypeId::UInt64) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about other wide types, like Int128, UInt128, Decimal128 and wider?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Int128/UInt128 are not supported in the driver yet. Decimal128 is extracted as string or double anyway (there is no standard Decimal C type), so no need to alter the way it is reported to the ODBC clients.

tmp_type_name = "String";
tmp_type_name_without_parameters = "String";
}

return getParent().getTypeInfo(tmp_type_name, tmp_type_name_without_parameters);
}

void Connection::connect(const std::string & connection_string) {
if (session && session->connected())
throw SqlException("Connection name in use", "08002");
Expand Down Expand Up @@ -132,6 +146,10 @@ void Connection::connect(const std::string & connection_string) {
session->setKeepAlive(true);
session->setTimeout(Poco::Timespan(connection_timeout, 0), Poco::Timespan(timeout, 0), Poco::Timespan(timeout, 0));
session->setKeepAliveTimeout(Poco::Timespan(86400, 0));

if (verify_connection_early) {
verifyConnection();
}
}

void Connection::resetConfiguration() {
Expand Down Expand Up @@ -238,6 +256,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v
timeout = typed_value;
}
}
else if (Poco::UTF8::icompare(key, INI_VERIFY_CONNECTION_EARLY) == 0) {
recognized_key = true;
valid_value = (value.empty() || isYesOrNo(value));
if (valid_value) {
verify_connection_early = isYes(value);
}
}
else if (Poco::UTF8::icompare(key, INI_SSLMODE) == 0) {
recognized_key = true;
valid_value = (
Expand Down Expand Up @@ -285,6 +310,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v
database = value;
}
}
else if (Poco::UTF8::icompare(key, INI_HUGE_INT_AS_STRING) == 0) {
recognized_key = true;
valid_value = (value.empty() || isYesOrNo(value));
if (valid_value) {
huge_int_as_string = isYes(value);
}
}
else if (Poco::UTF8::icompare(key, INI_STRINGMAXLENGTH) == 0) {
recognized_key = true;
unsigned int typed_value = 0;
Expand Down Expand Up @@ -445,6 +477,13 @@ void Connection::setConfiguration(const key_value_map_t & cs_fields, const key_v
stringmaxlength = TypeInfo::string_max_size;
}

void Connection::verifyConnection() {
LOG("Verifying connection and credentials...");
auto & statement = allocateChild<Statement>();
statement.executeQuery("SELECT 1");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can executeQuery() throw an exception? If yes, will the statement be successfully deallocated later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I should have added a proper cleanup there. Will fix.

statement.deallocateSelf();
}

std::string Connection::buildCredentialsString() const {
std::ostringstream user_password_base64;
Poco::Base64Encoder base64_encoder(user_password_base64, Poco::BASE64_URL_ENCODING);
Expand Down
8 changes: 8 additions & 0 deletions driver/connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ class Connection
std::uint16_t port = 0;
std::uint32_t connection_timeout = 0;
std::uint32_t timeout = 0;
bool verify_connection_early = false;
std::string sslmode;
std::string privateKeyFile;
std::string certificateFile;
std::string caLocation;
std::string path;
std::string default_format;
std::string database;
bool huge_int_as_string = false;
std::int32_t stringmaxlength = 0;

public:
Expand All @@ -48,6 +50,9 @@ class Connection
public:
explicit Connection(Environment & environment);

// Lookup TypeInfo for given name of type.
const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const;

void connect(const std::string & connection_string);

// Return a Base64 encoded string of "user:password".
Expand Down Expand Up @@ -84,6 +89,9 @@ class Connection
// c) values deduced from values of other fields, if unintialized.
void setConfiguration(const key_value_map_t & cs_fields, const key_value_map_t & dsn_fields);

// Verify the connection and credentials by trying to remotely execute a simple "SELECT 1" query.
void verifyConnection();

private:
std::unordered_map<SQLHANDLE, std::shared_ptr<Descriptor>> descriptors;
std::unordered_map<SQLHANDLE, std::shared_ptr<Statement>> statements;
Expand Down
14 changes: 10 additions & 4 deletions driver/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,12 @@ void Driver::writeLogMessagePrefix(std::ostream & stream) {
void Driver::writeLogSessionStart(std::ostream & stream) {
stream << "==================== ODBC Driver logging session started";
{
auto t = std::time(nullptr);
std::tm tm = {};
const auto time = std::time(nullptr);
toLocalTime(time, tm);

char mbstr[100] = {};
if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", std::localtime(&t)))
if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", &tm))
stream << " (" << mbstr << ")";
}
stream << " ====================" << std::endl;
Expand Down Expand Up @@ -127,9 +130,12 @@ void Driver::writeLogSessionStart(std::ostream & stream) {
void Driver::writeLogSessionEnd(std::ostream & stream) {
stream << "==================== ODBC Driver logging session ended";
{
auto t = std::time(nullptr);
std::tm tm = {};
const auto time = std::time(nullptr);
toLocalTime(time, tm);

char mbstr[100] = {};
if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", std::localtime(&t)))
if (std::strftime(mbstr, sizeof(mbstr), "%F %T %Z", &tm))
stream << " (" << mbstr << ")";
}
stream << " ====================" << std::endl;
Expand Down
2 changes: 1 addition & 1 deletion driver/environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Environment
template <typename T> T & allocateChild();
template <typename T> void deallocateChild(SQLHANDLE) noexcept;

const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters = "") const;
const TypeInfo & getTypeInfo(const std::string & type_name, const std::string & type_name_without_parameters) const;

public:
#if defined(SQL_OV_ODBC3_80)
Expand Down
15 changes: 10 additions & 5 deletions driver/format/ODBCDriver2.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "driver/format/ODBCDriver2.h"
#include "driver/utils/resize_without_initialization.h"

ODBCDriver2ResultSet::ODBCDriver2ResultSet(AmortizedIStreamReader & stream, std::unique_ptr<ResultMutator> && mutator)
ODBCDriver2ResultSet::ODBCDriver2ResultSet(const std::string & timezone, AmortizedIStreamReader & stream, std::unique_ptr<ResultMutator> && mutator)
: ResultSet(stream, std::move(mutator))
{
std::int32_t num_header_rows = 0;
Expand Down Expand Up @@ -36,7 +36,7 @@ ODBCDriver2ResultSet::ODBCDriver2ResultSet(AmortizedIStreamReader & stream, std:
TypeAst ast;

if (parser.parse(&ast)) {
columns_info[i].assignTypeInfo(ast);
columns_info[i].assignTypeInfo(ast, timezone);

if (convertUnparametrizedTypeNameToTypeId(columns_info[i].type_without_parameters) == DataSourceTypeId::Unknown) {
// Interpret all unknown types as String.
Expand Down Expand Up @@ -131,6 +131,7 @@ void ODBCDriver2ResultSet::readValue(Field & dest, ColumnInfo & column_info) {
else switch (column_info.type_without_parameters_id) {
case DataSourceTypeId::Date: readValueAs<DataSourceType< DataSourceTypeId::Date >>(value, dest, column_info); break;
case DataSourceTypeId::DateTime: readValueAs<DataSourceType< DataSourceTypeId::DateTime >>(value, dest, column_info); break;
case DataSourceTypeId::DateTime64: readValueAs<DataSourceType< DataSourceTypeId::DateTime64 >>(value, dest, column_info); break;
case DataSourceTypeId::Decimal: readValueAs<DataSourceType< DataSourceTypeId::Decimal >>(value, dest, column_info); break;
case DataSourceTypeId::Decimal32: readValueAs<DataSourceType< DataSourceTypeId::Decimal32 >>(value, dest, column_info); break;
case DataSourceTypeId::Decimal64: readValueAs<DataSourceType< DataSourceTypeId::Decimal64 >>(value, dest, column_info); break;
Expand Down Expand Up @@ -168,6 +169,10 @@ void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType<DataSourc
return value_manip::from_value<std::string>::template to_value<DataSourceType<DataSourceTypeId::DateTime>>::convert(src, dest);
}

void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType<DataSourceTypeId::DateTime64> & dest, ColumnInfo & column_info) {
return value_manip::from_value<std::string>::template to_value<DataSourceType<DataSourceTypeId::DateTime64>>::convert(src, dest);
}

void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType<DataSourceTypeId::Decimal> & dest, ColumnInfo & column_info) {
return value_manip::from_value<std::string>::template to_value<DataSourceType<DataSourceTypeId::Decimal>>::convert(src, dest);
}
Expand Down Expand Up @@ -240,13 +245,13 @@ void ODBCDriver2ResultSet::readValue(std::string & src, DataSourceType<DataSourc
return value_manip::from_value<std::string>::template to_value<DataSourceType<DataSourceTypeId::UUID>>::convert(src, dest);
}

ODBCDriver2ResultReader::ODBCDriver2ResultReader(std::istream & raw_stream, std::unique_ptr<ResultMutator> && mutator)
: ResultReader(raw_stream, std::move(mutator))
ODBCDriver2ResultReader::ODBCDriver2ResultReader(const std::string & timezone_, std::istream & raw_stream, std::unique_ptr<ResultMutator> && mutator)
: ResultReader(timezone_, raw_stream, std::move(mutator))
{
if (stream.eof())
return;

result_set = std::make_unique<ODBCDriver2ResultSet>(stream, releaseMutator());
result_set = std::make_unique<ODBCDriver2ResultSet>(timezone, stream, releaseMutator());
}

bool ODBCDriver2ResultReader::advanceToNextResultSet() {
Expand Down
5 changes: 3 additions & 2 deletions driver/format/ODBCDriver2.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ODBCDriver2ResultSet
: public ResultSet
{
public:
explicit ODBCDriver2ResultSet(AmortizedIStreamReader & stream, std::unique_ptr<ResultMutator> && mutator);
explicit ODBCDriver2ResultSet(const std::string & timezone, AmortizedIStreamReader & stream, std::unique_ptr<ResultMutator> && mutator);
virtual ~ODBCDriver2ResultSet() override = default;

protected:
Expand All @@ -32,6 +32,7 @@ class ODBCDriver2ResultSet

void readValue(std::string & src, DataSourceType< DataSourceTypeId::Date > & dest, ColumnInfo & column_info);
void readValue(std::string & src, DataSourceType< DataSourceTypeId::DateTime > & dest, ColumnInfo & column_info);
void readValue(std::string & src, DataSourceType< DataSourceTypeId::DateTime64 > & dest, ColumnInfo & column_info);
void readValue(std::string & src, DataSourceType< DataSourceTypeId::Decimal > & dest, ColumnInfo & column_info);
void readValue(std::string & src, DataSourceType< DataSourceTypeId::Decimal32 > & dest, ColumnInfo & column_info);
void readValue(std::string & src, DataSourceType< DataSourceTypeId::Decimal64 > & dest, ColumnInfo & column_info);
Expand Down Expand Up @@ -61,7 +62,7 @@ class ODBCDriver2ResultReader
: public ResultReader
{
public:
explicit ODBCDriver2ResultReader(std::istream & raw_stream, std::unique_ptr<ResultMutator> && mutator);
explicit ODBCDriver2ResultReader(const std::string & timezone_, std::istream & raw_stream, std::unique_ptr<ResultMutator> && mutator);
virtual ~ODBCDriver2ResultReader() override = default;

virtual bool advanceToNextResultSet() override;
Expand Down
Loading