From 2401b66e9eeea89914273b15be3777505e3771fd Mon Sep 17 00:00:00 2001 From: Dimitry Sibiryakov Date: Sun, 14 Jun 2020 19:01:53 +0200 Subject: [PATCH] Receive features list from a real provider (#266) * Receive features list from a real provider * Prevent statement caching if the provider doesn't support it * Changes requested by Dmitry Yemanov --- doc/sql.extensions/README.isc_info_xxx | 11 ++++ src/include/firebird/impl/inf_pub.h | 16 ++++++ src/jrd/constants.h | 8 +++ src/jrd/extds/ExtDS.cpp | 10 ++-- src/jrd/extds/ExtDS.h | 29 +++------- src/jrd/extds/InternalDS.cpp | 5 +- src/jrd/extds/InternalDS.h | 1 - src/jrd/extds/IscDS.cpp | 79 ++++++++++++++++---------- src/jrd/extds/IscDS.h | 5 +- src/jrd/inf.cpp | 8 +++ 10 files changed, 114 insertions(+), 58 deletions(-) diff --git a/doc/sql.extensions/README.isc_info_xxx b/doc/sql.extensions/README.isc_info_xxx index e6aa9d0740..959488849e 100644 --- a/doc/sql.extensions/README.isc_info_xxx +++ b/doc/sql.extensions/README.isc_info_xxx @@ -45,6 +45,17 @@ New items for isc_database_info isc_dpb_addr_flag_conn_encrypted - connection is encrypted; fb_info_wire_crypt - name of connection encryption plugin. +6. fb_info_provider_features: + return list of features supported by current connection's provider. + Each byte in returned array shall be one of following info_provider_features: + + fb_feature_multi_statements - multiple prepared statements in single attachment + fb_feature_multi_transactions - multiple concurrent transactions in single attachment + fb_feature_named_parameters - query parameters can be named + fb_feature_session_reset - ALTER SESSION RESET is supported + fb_feature_read_consistency - read consistency TIL is supported + fb_feature_statement_timeout - statement timeout is supported + New items for isc_transaction_info: diff --git a/src/include/firebird/impl/inf_pub.h b/src/include/firebird/impl/inf_pub.h index 003ef9a68d..45e270bf69 100644 --- a/src/include/firebird/impl/inf_pub.h +++ b/src/include/firebird/impl/inf_pub.h @@ -162,6 +162,9 @@ enum db_info_types fb_info_wire_crypt = 140, + // Return list of features supported by provider of current connection + fb_info_provider_features = 141, + isc_info_db_last_value /* Leave this LAST! */ }; @@ -171,6 +174,19 @@ enum db_info_crypt /* flags set in fb_info_crypt_state */ fb_info_crypt_process = 0x02 }; +enum info_provider_features // response to fb_info_provider_features +{ + fb_feature_multi_statements = 1, // Multiple prepared statements in single attachment + fb_feature_multi_transactions = 2, // Multiple concurrent transaction in single attachment + fb_feature_named_parameters = 3, // Query parameters can be named + fb_feature_session_reset = 4, // ALTER SESSION RESET is supported + fb_feature_read_consistency = 5, // Read consistency TIL is supported + fb_feature_statement_timeout = 6, // Statement timeout is supported + fb_feature_statement_long_life = 7, // Prepared statement can survive transaction end + + info_provider_features_max // Not really a feature. Keep this last. +}; + #define isc_info_version isc_info_isc_version diff --git a/src/jrd/constants.h b/src/jrd/constants.h index 49d0e386d1..da352e96f1 100644 --- a/src/jrd/constants.h +++ b/src/jrd/constants.h @@ -466,6 +466,14 @@ const int OPT_STATIC_ITEMS = 64; #define CURRENT_ENGINE "Engine13" #define EMBEDDED_PROVIDERS "Providers=" CURRENT_ENGINE +// Feature mask for current version of engine provider +#define ENGINE_FEATURES {fb_feature_multi_statements, \ + fb_feature_multi_transactions, \ + fb_feature_session_reset, \ + fb_feature_read_consistency, \ + fb_feature_statement_timeout, \ + fb_feature_statement_long_life} + const int WITH_GRANT_OPTION = 1; const int WITH_ADMIN_OPTION = 2; diff --git a/src/jrd/extds/ExtDS.cpp b/src/jrd/extds/ExtDS.cpp index b4f18cb3dd..ecd096c6b9 100644 --- a/src/jrd/extds/ExtDS.cpp +++ b/src/jrd/extds/ExtDS.cpp @@ -524,7 +524,7 @@ Connection::Connection(Provider& prov) : m_sqlDialect(0), m_wrapErrors(true), m_broken(false), - m_features(0) + m_features{} { } @@ -639,7 +639,7 @@ void Connection::releaseStatement(Jrd::thread_db* tdbb, Statement* stmt) { fb_assert(stmt && !stmt->isActive()); - if (stmt->isAllocated() && m_free_stmts < MAX_CACHED_STMTS) + if (stmt->isAllocated() && testFeature(fb_feature_statement_long_life) && m_free_stmts < MAX_CACHED_STMTS) { stmt->m_nextFree = m_freeStatements; m_freeStatements = stmt; @@ -1753,7 +1753,7 @@ void Statement::prepare(thread_db* tdbb, Transaction* tran, const string& sql, b string sql2(getPool()); const string* readySql = &sql; - if (named && !(m_provider.getFlags() & prvNamedParams)) + if (named && !m_connection.testFeature(fb_feature_named_parameters)) { preprocess(sql, sql2); readySql = &sql2; @@ -1772,7 +1772,7 @@ void Statement::setTimeout(thread_db* tdbb, unsigned int timeout) } void Statement::execute(thread_db* tdbb, Transaction* tran, - const MetaName* const* in_names, const ValueListNode* in_params, const ParamNumbers* in_excess, + const MetaName* const* in_names, const ValueListNode* in_params, const ParamNumbers* in_excess, const ValueListNode* out_params) { fb_assert(isAllocated() && !m_stmt_selectable); @@ -1787,7 +1787,7 @@ void Statement::execute(thread_db* tdbb, Transaction* tran, } void Statement::open(thread_db* tdbb, Transaction* tran, - const MetaName* const* in_names, const ValueListNode* in_params, const ParamNumbers* in_excess, + const MetaName* const* in_names, const ValueListNode* in_params, const ParamNumbers* in_excess, bool singleton) { fb_assert(isAllocated() && m_stmt_selectable); diff --git a/src/jrd/extds/ExtDS.h b/src/jrd/extds/ExtDS.h index 504fca2b7a..b12840dd71 100644 --- a/src/jrd/extds/ExtDS.h +++ b/src/jrd/extds/ExtDS.h @@ -197,10 +197,7 @@ protected: }; // Provider flags -const int prvMultyStmts = 0x0001; // supports many active statements per connection -const int prvMultyTrans = 0x0002; // supports many active transactions per connection -const int prvNamedParams = 0x0004; // supports named parameters -const int prvTrustedAuth = 0x0008; // supports trusted authentication +const int prvTrustedAuth = 0x0001; // supports trusted authentication class ConnectionsPool @@ -494,12 +491,12 @@ public: virtual Blob* createBlob() = 0; - // Test specified flags, return true if all bits present - bool testFeature(ULONG value) const { return m_features & value; } - // Set specified flags, return new value - ULONG setFeature(ULONG value) { return m_features |= value; } - // Clear specified flags, return new value - ULONG clearFeature(ULONG value) { return m_features &= ~value; } + // Test specified feature flag + bool testFeature(info_provider_features value) const { return m_features[value]; } + // Set specified flag + void setFeature(info_provider_features value) { m_features[value] = true; } + // Clear specified flag + void clearFeature(info_provider_features value) { m_features[value] = false; } protected: virtual Transaction* doCreateTransaction() = 0; @@ -531,17 +528,9 @@ protected: int m_sqlDialect; // must be filled in attach call bool m_wrapErrors; bool m_broken; - ULONG m_features; // bitmask + bool m_features[info_provider_features_max]; }; -// Connection features flags -const ULONG conFtrSessionReset = 0x01; // supports ALTER SESSION RESET -const ULONG conFtrReadConsistency = 0x02; // supports READ COMMITTED READ CONSISTENCY -const ULONG conFtrStatementTimeout = 0x04; // supports statements timeout - -// Features of Firebird 4 -const ULONG conFtrFB4 = conFtrSessionReset | conFtrReadConsistency | conFtrStatementTimeout; - class Transaction : public Firebird::PermanentStorage { protected: @@ -618,7 +607,7 @@ public: const Firebird::MetaName* const* in_names, const Jrd::ValueListNode* in_params, const ParamNumbers* in_excess, const Jrd::ValueListNode* out_params); void open(Jrd::thread_db* tdbb, Transaction* tran, - const Firebird::MetaName* const* in_names, const Jrd::ValueListNode* in_params, + const Firebird::MetaName* const* in_names, const Jrd::ValueListNode* in_params, const ParamNumbers* in_excess, bool singleton); bool fetch(Jrd::thread_db* tdbb, const Jrd::ValueListNode* out_params); void close(Jrd::thread_db* tdbb, bool invalidTran = false); diff --git a/src/jrd/extds/InternalDS.cpp b/src/jrd/extds/InternalDS.cpp index b80d9a976c..4ea3830c9e 100644 --- a/src/jrd/extds/InternalDS.cpp +++ b/src/jrd/extds/InternalDS.cpp @@ -188,7 +188,10 @@ void InternalConnection::attach(thread_db* tdbb) m_sqlDialect = (attachment->att_database->dbb_flags & DBB_DB_SQL_dialect_3) ? SQL_DIALECT_V6 : SQL_DIALECT_V5; - m_features = conFtrFB4; + memset(m_features, false, sizeof(m_features)); + static const info_provider_features features[] = ENGINE_FEATURES; + for (int i = 0; i < sizeof(features); i++) + setFeature(features[i]); } void InternalConnection::doDetach(thread_db* tdbb) diff --git a/src/jrd/extds/InternalDS.h b/src/jrd/extds/InternalDS.h index 31ba8d8869..92662fe697 100644 --- a/src/jrd/extds/InternalDS.h +++ b/src/jrd/extds/InternalDS.h @@ -34,7 +34,6 @@ public: explicit InternalProvider(const char* prvName) : Provider(prvName) { - m_flags = (prvMultyStmts | prvMultyTrans); } ~InternalProvider() diff --git a/src/jrd/extds/IscDS.cpp b/src/jrd/extds/IscDS.cpp index 9d4847b938..33c7c515d5 100644 --- a/src/jrd/extds/IscDS.cpp +++ b/src/jrd/extds/IscDS.cpp @@ -144,17 +144,20 @@ void IscConnection::attach(thread_db* tdbb) } } - char buff[16]; + char buff[BUFFER_TINY]; { EngineCallbackGuard guard(tdbb, *this, FB_FUNCTION); - const char info[] = {isc_info_db_sql_dialect, isc_info_end}; + const unsigned char info[] = {isc_info_db_sql_dialect, fb_info_provider_features, isc_info_end}; m_iscProvider.isc_database_info(&status, &m_handle, sizeof(info), info, sizeof(buff), buff); } if (status->getState() & IStatus::STATE_ERRORS) { raise(&status, tdbb, "isc_database_info"); } + memset(m_features, false, sizeof(m_features)); + m_sqlDialect = 1; + const char* p = buff, *end = buff + sizeof(buff); while (p < end) { @@ -168,22 +171,42 @@ void IscConnection::attach(thread_db* tdbb) m_sqlDialect = m_iscProvider.isc_vax_integer(p, len); break; + case fb_info_provider_features: + for (int i = 0; i < len; i++) + { + if (p[i] == 0) + { + ERR_post(Arg::Gds(isc_random) << Arg::Str("Bad provider feature value")); + } + + if (p[i] < info_provider_features_max) + { + setFeature(static_cast(p[i])); + } + // else this provider supports unknown feature, ignore it. + } + break; + case isc_info_error: - if (*p == isc_info_db_sql_dialect) - { - const ULONG err = m_iscProvider.isc_vax_integer(p + 1, len - 1); - if (err == isc_infunk) - { - // Remote server don't understand isc_info_db_sql_dialect. - // Consider it as pre-IB6 server and use SQL dialect 1 to work with it. - m_sqlDialect = 1; - break; - } - } - // fall thru + { + const ULONG err = m_iscProvider.isc_vax_integer(p + 1, len - 1); + if (err == isc_infunk) + { + if (*p == fb_info_provider_features) + { + // Used provider follow Firebird error reporting conventions but is not aware of + // this info item. Assume Firebird 3 or earlier. + m_features[fb_feature_multi_statements] = true; + m_features[fb_feature_multi_transactions] = true; + m_features[fb_feature_statement_long_life] = true; + } + break; + } + ERR_post(Arg::Gds(isc_random) << Arg::Str("Unexpected error in isc_database_info")); + } case isc_info_truncated: - ERR_post(Arg::Gds(isc_random) << Arg::Str("Unexpected error in isc_database_info")); + ERR_post(Arg::Gds(isc_random) << Arg::Str("Result truncation in isc_database_info")); case isc_info_end: p = end; @@ -192,7 +215,6 @@ void IscConnection::attach(thread_db* tdbb) p += len; } - m_features = conFtrFB4; // Exact feature set will be detected at first usage } void IscConnection::doDetach(thread_db* tdbb) @@ -240,7 +262,7 @@ bool IscConnection::resetSession() if (!m_handle) return false; - if (!testFeature(conFtrSessionReset)) + if (!testFeature(fb_feature_session_reset)) return true; FbLocalStatus status; @@ -252,7 +274,7 @@ bool IscConnection::resetSession() if (status->getErrors()[1] == isc_dsql_error) { - clearFeature(conFtrSessionReset); + clearFeature(fb_feature_session_reset); return true; } @@ -267,11 +289,10 @@ bool IscConnection::resetSession() // transactions bool IscConnection::isAvailable(thread_db* tdbb, TraScope traScope) const { - const int flags = m_provider.getFlags(); - if (m_used_stmts && !(flags & prvMultyStmts)) + if (m_used_stmts && !testFeature(fb_feature_multi_statements)) return false; - if (m_transactions.getCount() && !(flags & prvMultyTrans) && !findTransaction(tdbb, traScope)) + if (m_transactions.getCount() && !testFeature(fb_feature_multi_transactions) && !findTransaction(tdbb, traScope)) { return false; } @@ -288,7 +309,7 @@ bool IscConnection::validate(Jrd::thread_db* tdbb) EngineCallbackGuard guard(tdbb, *this, FB_FUNCTION); - char info[] = {isc_info_attachment_id, isc_info_end}; + const unsigned char info[] = {isc_info_attachment_id, isc_info_end}; char buff[32]; return m_iscProvider.isc_database_info(&status, &m_handle, @@ -315,7 +336,7 @@ Statement* IscConnection::doCreateStatement() void IscTransaction::generateTPB(thread_db* tdbb, ClumpletWriter& tpb, TraModes traMode, bool readOnly, bool wait, int lockTimeout) const { - if (traMode == traReadCommitedReadConsistency && !m_connection.testFeature(conFtrReadConsistency)) + if (traMode == traReadCommitedReadConsistency && !m_connection.testFeature(fb_feature_read_consistency)) traMode = traConcurrency; Transaction::generateTPB(tdbb, tpb, traMode, readOnly, wait, lockTimeout); @@ -335,7 +356,7 @@ void IscTransaction::doStart(FbStatusVector* status, thread_db* tdbb, Firebird:: if ((status->getState() & IStatus::STATE_ERRORS) && (status->getErrors()[1] == isc_bad_tpb_form) && tpb.find(isc_tpb_read_consistency) && - m_connection.testFeature(conFtrReadConsistency)) + m_connection.testFeature(fb_feature_read_consistency)) { tpb.deleteWithTag(isc_tpb_read_committed); tpb.deleteWithTag(isc_tpb_read_consistency); @@ -348,7 +369,7 @@ void IscTransaction::doStart(FbStatusVector* status, thread_db* tdbb, Firebird:: } if (!(status->getState() & IStatus::STATE_ERRORS)) - m_connection.clearFeature(conFtrReadConsistency); + m_connection.clearFeature(fb_feature_read_consistency); } } @@ -562,7 +583,7 @@ void IscStatement::doPrepare(thread_db* tdbb, const string& sql) void IscStatement::doSetTimeout(thread_db* tdbb, unsigned int timeout) { - if (!m_connection.testFeature(conFtrStatementTimeout)) + if (!m_connection.testFeature(fb_feature_statement_timeout)) return; FbLocalStatus status; @@ -578,7 +599,7 @@ void IscStatement::doSetTimeout(thread_db* tdbb, unsigned int timeout) // or loaded client library if (status[0] == isc_arg_gds && (status[1] == isc_wish_list || status[1] == isc_unavailable)) { - m_connection.clearFeature(conFtrStatementTimeout); + m_connection.clearFeature(fb_feature_statement_timeout); return; } @@ -1047,7 +1068,7 @@ ISC_STATUS ISC_EXPORT IscProvider::isc_create_database(FbStatusVector* user_stat ISC_STATUS ISC_EXPORT IscProvider::isc_database_info(FbStatusVector* user_status, isc_db_handle* db_handle, short info_len, - const char* info, + const unsigned char* info, short res_len, char* res) { @@ -1055,7 +1076,7 @@ ISC_STATUS ISC_EXPORT IscProvider::isc_database_info(FbStatusVector* user_status return notImplemented(user_status); return (*m_api.database_info) (IscStatus(user_status), db_handle, - info_len, info, res_len, res); + info_len, reinterpret_cast(info), res_len, res); } void ISC_EXPORT IscProvider::isc_decode_date(const ISC_QUAD*, diff --git a/src/jrd/extds/IscDS.h b/src/jrd/extds/IscDS.h index 6f2a6e5307..a21059ce6e 100644 --- a/src/jrd/extds/IscDS.h +++ b/src/jrd/extds/IscDS.h @@ -186,7 +186,7 @@ public: virtual ISC_STATUS ISC_EXPORT isc_database_info(Jrd::FbStatusVector*, isc_db_handle*, short, - const char*, + const unsigned char*, short, char*); @@ -497,7 +497,8 @@ public: explicit FBProvider(const char* prvName) : IscProvider(prvName) { - m_flags = (prvMultyStmts | prvMultyTrans | prvTrustedAuth); + // Assume that winsspi auth plugin is enabled in configuration + m_flags = prvTrustedAuth; } protected: diff --git a/src/jrd/inf.cpp b/src/jrd/inf.cpp index 5a55adc721..b3bb49bd4b 100644 --- a/src/jrd/inf.cpp +++ b/src/jrd/inf.cpp @@ -855,6 +855,14 @@ void INF_database_info(thread_db* tdbb, length = INF_convert(att->getActualIdleTimeout(), buffer); break; + case fb_info_provider_features: + { + static const unsigned char features[] = ENGINE_FEATURES; + length = sizeof(features); + memcpy(buffer, features, length); + break; + } + default: buffer[0] = item; item = isc_info_error;