Skip to content

Fix SQL_DBMS_VER to return the Firebird product version #294

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.

Sign up for GitHub

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

Open
fdcastel wants to merge 1 commit into
base: master
Choose a base branch
from
+140 −10
Open

Fix SQL_DBMS_VER to return the Firebird product version #294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 26 additions & 1 deletion IscDbc/Attachment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,32 @@ void Attachment::openDatabase(const char *dbName, Properties *properties)
else
beg++;
}
serverVersion.Format( "%02d.%02d.%04d %.*s %s",major,minor,version, tmp ? tmp - start : 0, start, (const char*)productName );
// Build SQL_DBMS_VER from the FIREBIRD PRODUCT version
// (majorFb/minorFb/versionFb, captured from isc_info_firebird_version)
// rather than from the engine / ODS-compat version parsed above.
//
// Background: isc_info_version returns a legacy InterBase-style
// string whose leading digits are the ENGINE implementation
// version (currently "6.3.x" on every supported Firebird — that
// number tracks the wire protocol / ODS compatibility level, not
// the Firebird release), while isc_info_firebird_version (added
// in FB 2.x) returns the actual Firebird product version string.
// The ODBC specification defines SQL_DBMS_VER as "the version of
// the current DBMS product" — so reporting the engine number
// mis-identifies every Firebird server as 6.x to every ODBC
// consumer (downstream tools, version-gated tests, etc.).
//
// Fall back to the engine numbers only when isc_info_firebird_version
// was not returned (very old InterBase / pre-2.0 Firebird), which
// we detect as the defaults set in the Attachment constructor
// (majorFb=1, minorFb=0, versionFb=0).
const bool haveFbVersion =
majorFb > 1 || minorFb > 0 || versionFb > 0;
serverVersion.Format( "%02d.%02d.%04d %.*s %s",
haveFbVersion ? majorFb : major,
haveFbVersion ? minorFb : minor,
haveFbVersion ? versionFb : version,
tmp ? tmp - start : 0, start, (const char*)productName );
}
break;

Expand Down
123 changes: 114 additions & 9 deletions tests/test_server_version.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,31 @@
// test_server_version.cpp — Tests for server version detection and feature-flagging (Task 4.2)
#include "test_helpers.h"
#include <cstdio>
#include <string>

// ============================================================================
// ServerVersionTest: Verify server version is correctly detected and exposed
// ============================================================================
class ServerVersionTest : public OdbcConnectedTest {};
class ServerVersionTest : public OdbcConnectedTest {
protected:
// Query the Firebird engine version string (e.g. "5.0.3" / "6.0.0" / "3.0.12")
// straight from the server via rdb$get_context — this is the ground truth we
// compare SQL_DBMS_VER against below.
std::string GetEngineVersionFromServer() {
SQLRETURN ret = SQLExecDirect(hStmt,
(SQLCHAR*)"SELECT rdb$get_context('SYSTEM','ENGINE_VERSION') FROM rdb$database",
SQL_NTS);
EXPECT_TRUE(SQL_SUCCEEDED(ret));
ret = SQLFetch(hStmt);
EXPECT_TRUE(SQL_SUCCEEDED(ret));
SQLCHAR buf[64] = {};
SQLLEN ind = 0;
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind);
EXPECT_TRUE(SQL_SUCCEEDED(ret));
SQLCloseCursor(hStmt);
return std::string((char*)buf);
}
};

TEST_F(ServerVersionTest, SQLGetInfoDBMSVer) {
// SQLGetInfo(SQL_DBMS_VER) should return a non-empty version string
Expand Down Expand Up @@ -32,20 +53,104 @@ TEST_F(ServerVersionTest, SQLGetInfoDBMSName) {

TEST_F(ServerVersionTest, EngineVersionFromSQL) {
// Query the engine version directly to cross-check
ExecDirect("SELECT rdb$get_context('SYSTEM','ENGINE_VERSION') FROM rdb$database");
SQLCHAR version[256] = {};
SQLLEN ind = 0;
SQLRETURN ret = SQLFetch(hStmt);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, version, sizeof(version), &ind);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
std::string verStr((char*)version);
std::string verStr = GetEngineVersionFromServer();
// Should be like "5.0.1" or "4.0.5"
EXPECT_GE(verStr.length(), 5u) << "Engine version too short: " << verStr;
// First char should be a digit >= 3
EXPECT_GE(verStr[0], '3') << "Expected Firebird 3.0+: " << verStr;
}

// ----------------------------------------------------------------------------
// Regression tests for the Firebird-product-version-vs-engine-implementation-
// version bug: until #292 + follow-up, SQL_DBMS_VER was built from
// isc_info_version (which reports the ENGINE / ODS-compat implementation
// number, currently "6.3.x" on every supported Firebird) instead of from
// isc_info_firebird_version (the actual product version, e.g. "5.0.3"). On
// Firebird 5.0.3 SQLGetInfo(SQL_DBMS_VER) returned "06.03.1683 WI-V Firebird 5.0"
// — misidentifying every server as Firebird 6.x to any downstream ODBC tool
// or version-gated test (see SKIP_ON_FIREBIRD6 in test_helpers.h, which was
// silently firing on FB 3 / 4 / 5 too).
//
// These tests cross-reference SQL_DBMS_VER against rdb$get_context(ENGINE_VERSION)
// and against SQL_DBMS_NAME. The pre-fix behaviour fails all three of them.
// ----------------------------------------------------------------------------

TEST_F(ServerVersionTest, DBMSVerMajorMatchesEngineVersion) {
// SQL_DBMS_VER per ODBC spec is a "##.##.####" prefix followed by an
// implementation-defined suffix. The first two digits are the product
// major version — they must match the first token of the engine version
// reported by rdb$get_context.
SQLCHAR dbmsVer[256] = {};
SQLSMALLINT len = 0;
SQLRETURN ret = SQLGetInfo(hDbc, SQL_DBMS_VER, dbmsVer, sizeof(dbmsVer), &len);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
std::string dbmsVerStr((char*)dbmsVer, len);

std::string engineVer = GetEngineVersionFromServer();
ASSERT_FALSE(engineVer.empty());

// Parse "MM" prefix of SQL_DBMS_VER (ODBC mandates the format; leading
// whitespace is not allowed).
ASSERT_GE(dbmsVerStr.size(), 2u);
const int dbmsMajor = std::atoi(dbmsVerStr.substr(0, 2).c_str());

// Parse first token of engine version (split at '.').
const int engineMajor = std::atoi(engineVer.c_str());

EXPECT_EQ(dbmsMajor, engineMajor)
<< "SQL_DBMS_VER major must match rdb$get_context('ENGINE_VERSION') major.\n"
<< " SQL_DBMS_VER: '" << dbmsVerStr << "'\n"
<< " ENGINE_VERSION: '" << engineVer << "'\n"
<< " Parsed DBMS_VER major: " << dbmsMajor << "\n"
<< " Parsed engine major: " << engineMajor;
}

TEST_F(ServerVersionTest, DBMSVerMinorMatchesEngineVersion) {
SQLCHAR dbmsVer[256] = {};
SQLSMALLINT len = 0;
SQLRETURN ret = SQLGetInfo(hDbc, SQL_DBMS_VER, dbmsVer, sizeof(dbmsVer), &len);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
std::string dbmsVerStr((char*)dbmsVer, len);

std::string engineVer = GetEngineVersionFromServer();
ASSERT_FALSE(engineVer.empty());

// Parse "MM.mm" minor (bytes 3-4 of SQL_DBMS_VER).
ASSERT_GE(dbmsVerStr.size(), 5u);
const int dbmsMinor = std::atoi(dbmsVerStr.substr(3, 2).c_str());

// Parse second token of engine version.
auto firstDot = engineVer.find('.');
ASSERT_NE(firstDot, std::string::npos);
const int engineMinor = std::atoi(engineVer.substr(firstDot + 1).c_str());

EXPECT_EQ(dbmsMinor, engineMinor)
<< "SQL_DBMS_VER minor must match rdb$get_context('ENGINE_VERSION') minor.\n"
<< " SQL_DBMS_VER: '" << dbmsVerStr << "'\n"
<< " ENGINE_VERSION: '" << engineVer << "'";
}

TEST_F(ServerVersionTest, DBMSVerSuffixContainsDBMSName) {
// Beyond the "##.##.####" prefix the string is implementation-defined, but
// the driver has always appended " <impl-prefix> <product name>" where the
// product name contains the DBMS_NAME. If that invariant ever slips we
// want to know about it — it's the quickest smoke test that the whole
// string wasn't clobbered.
SQLCHAR dbmsVer[256] = {}, dbmsName[256] = {};
SQLSMALLINT verLen = 0, nameLen = 0;
ASSERT_TRUE(SQL_SUCCEEDED(
SQLGetInfo(hDbc, SQL_DBMS_VER, dbmsVer, sizeof(dbmsVer), &verLen)));
ASSERT_TRUE(SQL_SUCCEEDED(
SQLGetInfo(hDbc, SQL_DBMS_NAME, dbmsName, sizeof(dbmsName), &nameLen)));
std::string verStr((char*)dbmsVer, verLen);
std::string nameStr((char*)dbmsName, nameLen);

EXPECT_NE(verStr.find(nameStr), std::string::npos)
<< "SQL_DBMS_VER suffix should include the DBMS_NAME.\n"
<< " SQL_DBMS_VER: '" << verStr << "'\n"
<< " SQL_DBMS_NAME: '" << nameStr << "'";
}

TEST_F(ServerVersionTest, SQLGetTypeInfoShowsAllBaseTypes) {
GTEST_SKIP() << "Requires Phase 4: server version detection for type count";
// SQLGetTypeInfo(SQL_ALL_TYPES) should return at least the 22 base types
Expand Down