Skip to content

Commit b64c011

Browse files
irodushkairodushka
authored
Merge pull request #294 from fdcastel/fix/sql-dbms-ver
Fix SQL_DBMS_VER to return the Firebird product version
2 parents 1e7534e + 9155943 commit b64c011

2 files changed

Lines changed: 140 additions & 10 deletions

File tree

‎IscDbc/Attachment.cpp‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,32 @@ void Attachment::openDatabase(const char *dbName, Properties *properties)
477477
else
478478
beg++;
479479
}
480-
serverVersion.Format( "%02d.%02d.%04d %.*s %s",major,minor,version, tmp ? tmp - start : 0, start, (const char*)productName );
480+
// Build SQL_DBMS_VER from the FIREBIRD PRODUCT version
481+
// (majorFb/minorFb/versionFb, captured from isc_info_firebird_version)
482+
// rather than from the engine / ODS-compat version parsed above.
483+
//
484+
// Background: isc_info_version returns a legacy InterBase-style
485+
// string whose leading digits are the ENGINE implementation
486+
// version (currently "6.3.x" on every supported Firebird — that
487+
// number tracks the wire protocol / ODS compatibility level, not
488+
// the Firebird release), while isc_info_firebird_version (added
489+
// in FB 2.x) returns the actual Firebird product version string.
490+
// The ODBC specification defines SQL_DBMS_VER as "the version of
491+
// the current DBMS product" — so reporting the engine number
492+
// mis-identifies every Firebird server as 6.x to every ODBC
493+
// consumer (downstream tools, version-gated tests, etc.).
494+
//
495+
// Fall back to the engine numbers only when isc_info_firebird_version
496+
// was not returned (very old InterBase / pre-2.0 Firebird), which
497+
// we detect as the defaults set in the Attachment constructor
498+
// (majorFb=1, minorFb=0, versionFb=0).
499+
const bool haveFbVersion =
500+
majorFb > 1 || minorFb > 0 || versionFb > 0;
501+
serverVersion.Format( "%02d.%02d.%04d %.*s %s",
502+
haveFbVersion ? majorFb : major,
503+
haveFbVersion ? minorFb : minor,
504+
haveFbVersion ? versionFb : version,
505+
tmp ? tmp - start : 0, start, (const char*)productName );
481506
}
482507
break;
483508

‎tests/test_server_version.cpp‎

Lines changed: 114 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,31 @@
11
// test_server_version.cpp — Tests for server version detection and feature-flagging (Task 4.2)
22
#include "test_helpers.h"
3+
#include <cstdio>
4+
#include <string>
35

46
// ============================================================================
57
// ServerVersionTest: Verify server version is correctly detected and exposed
68
// ============================================================================
7-
class ServerVersionTest : public OdbcConnectedTest {};
9+
class ServerVersionTest : public OdbcConnectedTest {
10+
protected:
11+
// Query the Firebird engine version string (e.g. "5.0.3" / "6.0.0" / "3.0.12")
12+
// straight from the server via rdb$get_context — this is the ground truth we
13+
// compare SQL_DBMS_VER against below.
14+
std::string GetEngineVersionFromServer() {
15+
SQLRETURN ret = SQLExecDirect(hStmt,
16+
(SQLCHAR*)"SELECT rdb$get_context('SYSTEM','ENGINE_VERSION') FROM rdb$database",
17+
SQL_NTS);
18+
EXPECT_TRUE(SQL_SUCCEEDED(ret));
19+
ret = SQLFetch(hStmt);
20+
EXPECT_TRUE(SQL_SUCCEEDED(ret));
21+
SQLCHAR buf[64] = {};
22+
SQLLEN ind = 0;
23+
ret = SQLGetData(hStmt, 1, SQL_C_CHAR, buf, sizeof(buf), &ind);
24+
EXPECT_TRUE(SQL_SUCCEEDED(ret));
25+
SQLCloseCursor(hStmt);
26+
return std::string((char*)buf);
27+
}
28+
};
829

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

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

63+
// ----------------------------------------------------------------------------
64+
// Regression tests for the Firebird-product-version-vs-engine-implementation-
65+
// version bug: until #292 + follow-up, SQL_DBMS_VER was built from
66+
// isc_info_version (which reports the ENGINE / ODS-compat implementation
67+
// number, currently "6.3.x" on every supported Firebird) instead of from
68+
// isc_info_firebird_version (the actual product version, e.g. "5.0.3"). On
69+
// Firebird 5.0.3 SQLGetInfo(SQL_DBMS_VER) returned "06.03.1683 WI-V Firebird 5.0"
70+
// — misidentifying every server as Firebird 6.x to any downstream ODBC tool
71+
// or version-gated test (see SKIP_ON_FIREBIRD6 in test_helpers.h, which was
72+
// silently firing on FB 3 / 4 / 5 too).
73+
//
74+
// These tests cross-reference SQL_DBMS_VER against rdb$get_context(ENGINE_VERSION)
75+
// and against SQL_DBMS_NAME. The pre-fix behaviour fails all three of them.
76+
// ----------------------------------------------------------------------------
77+
78+
TEST_F(ServerVersionTest, DBMSVerMajorMatchesEngineVersion) {
79+
// SQL_DBMS_VER per ODBC spec is a "##.##.####" prefix followed by an
80+
// implementation-defined suffix. The first two digits are the product
81+
// major version — they must match the first token of the engine version
82+
// reported by rdb$get_context.
83+
SQLCHAR dbmsVer[256] = {};
84+
SQLSMALLINT len = 0;
85+
SQLRETURN ret = SQLGetInfo(hDbc, SQL_DBMS_VER, dbmsVer, sizeof(dbmsVer), &len);
86+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
87+
std::string dbmsVerStr((char*)dbmsVer, len);
88+
89+
std::string engineVer = GetEngineVersionFromServer();
90+
ASSERT_FALSE(engineVer.empty());
91+
92+
// Parse "MM" prefix of SQL_DBMS_VER (ODBC mandates the format; leading
93+
// whitespace is not allowed).
94+
ASSERT_GE(dbmsVerStr.size(), 2u);
95+
const int dbmsMajor = std::atoi(dbmsVerStr.substr(0, 2).c_str());
96+
97+
// Parse first token of engine version (split at '.').
98+
const int engineMajor = std::atoi(engineVer.c_str());
99+
100+
EXPECT_EQ(dbmsMajor, engineMajor)
101+
<< "SQL_DBMS_VER major must match rdb$get_context('ENGINE_VERSION') major.\n"
102+
<< " SQL_DBMS_VER: '" << dbmsVerStr << "'\n"
103+
<< " ENGINE_VERSION: '" << engineVer << "'\n"
104+
<< " Parsed DBMS_VER major: " << dbmsMajor << "\n"
105+
<< " Parsed engine major: " << engineMajor;
106+
}
107+
108+
TEST_F(ServerVersionTest, DBMSVerMinorMatchesEngineVersion) {
109+
SQLCHAR dbmsVer[256] = {};
110+
SQLSMALLINT len = 0;
111+
SQLRETURN ret = SQLGetInfo(hDbc, SQL_DBMS_VER, dbmsVer, sizeof(dbmsVer), &len);
112+
ASSERT_TRUE(SQL_SUCCEEDED(ret));
113+
std::string dbmsVerStr((char*)dbmsVer, len);
114+
115+
std::string engineVer = GetEngineVersionFromServer();
116+
ASSERT_FALSE(engineVer.empty());
117+
118+
// Parse "MM.mm" minor (bytes 3-4 of SQL_DBMS_VER).
119+
ASSERT_GE(dbmsVerStr.size(), 5u);
120+
const int dbmsMinor = std::atoi(dbmsVerStr.substr(3, 2).c_str());
121+
122+
// Parse second token of engine version.
123+
auto firstDot = engineVer.find('.');
124+
ASSERT_NE(firstDot, std::string::npos);
125+
const int engineMinor = std::atoi(engineVer.substr(firstDot + 1).c_str());
126+
127+
EXPECT_EQ(dbmsMinor, engineMinor)
128+
<< "SQL_DBMS_VER minor must match rdb$get_context('ENGINE_VERSION') minor.\n"
129+
<< " SQL_DBMS_VER: '" << dbmsVerStr << "'\n"
130+
<< " ENGINE_VERSION: '" << engineVer << "'";
131+
}
132+
133+
TEST_F(ServerVersionTest, DBMSVerSuffixContainsDBMSName) {
134+
// Beyond the "##.##.####" prefix the string is implementation-defined, but
135+
// the driver has always appended " <impl-prefix> <product name>" where the
136+
// product name contains the DBMS_NAME. If that invariant ever slips we
137+
// want to know about it — it's the quickest smoke test that the whole
138+
// string wasn't clobbered.
139+
SQLCHAR dbmsVer[256] = {}, dbmsName[256] = {};
140+
SQLSMALLINT verLen = 0, nameLen = 0;
141+
ASSERT_TRUE(SQL_SUCCEEDED(
142+
SQLGetInfo(hDbc, SQL_DBMS_VER, dbmsVer, sizeof(dbmsVer), &verLen)));
143+
ASSERT_TRUE(SQL_SUCCEEDED(
144+
SQLGetInfo(hDbc, SQL_DBMS_NAME, dbmsName, sizeof(dbmsName), &nameLen)));
145+
std::string verStr((char*)dbmsVer, verLen);
146+
std::string nameStr((char*)dbmsName, nameLen);
147+
148+
EXPECT_NE(verStr.find(nameStr), std::string::npos)
149+
<< "SQL_DBMS_VER suffix should include the DBMS_NAME.\n"
150+
<< " SQL_DBMS_VER: '" << verStr << "'\n"
151+
<< " SQL_DBMS_NAME: '" << nameStr << "'";
152+
}
153+
49154
TEST_F(ServerVersionTest, SQLGetTypeInfoShowsAllBaseTypes) {
50155
GTEST_SKIP() << "Requires Phase 4: server version detection for type count";
51156
// SQLGetTypeInfo(SQL_ALL_TYPES) should return at least the 22 base types

0 commit comments

Comments
 (0)