-
Notifications
You must be signed in to change notification settings - Fork 7.9k
PDO_Firebird: Supported Firebird 4.0 datatypes #14897
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
Changes from 1 commit
950de97
546af78
af79543
a70cf1b
0e056cb
841a1e5
4b71218
af31c84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -460,6 +460,51 @@ static int php_firebird_preprocess(const zend_string* sql, char* sql_out, HashTa | |
return 1; | ||
} | ||
|
||
#if FB_API_VER >= 40 | ||
/* set coercing a data type */ | ||
static void set_coercing_data_type(XSQLDA* sqlda) | ||
{ | ||
/* Data types introduced in Firebird 4.0 are difficult to process using the Firebird Legacy API. */ | ||
/* These data types include DECFLOAT(16), DECFLOAT(34), INT128 (NUMERIC/DECIMAL(38, x)), */ | ||
/* TIMESTAMP WITH TIME ZONE, and TIME WITH TIME ZONE. In any case, at least the first three data types */ | ||
/* can only be mapped to strings. The last two too, but for them it is potentially possible to set */ | ||
/* the display format, as is done for TIMESTAMP. This function allows you to ensure minimal performance */ | ||
/* of queries if they contain columns and parameters of the above types. */ | ||
unsigned int i; | ||
short dtype; | ||
short nullable; | ||
XSQLVAR* var; | ||
for (i=0, var = sqlda->sqlvar; i < sqlda->sqld; i++, var++) { | ||
dtype = (var->sqltype & ~1); /* drop flag bit */ | ||
nullable = (var->sqltype & 1); | ||
switch(dtype) { | ||
case SQL_INT128: | ||
var->sqltype = SQL_VARYING + nullable; | ||
var->sqllen = 46; | ||
var->sqlscale = 0; | ||
break; | ||
case SQL_DEC16: | ||
var->sqltype = SQL_VARYING + nullable; | ||
var->sqllen = 24; | ||
break; | ||
case SQL_DEC34: | ||
var->sqltype = SQL_VARYING + nullable; | ||
var->sqllen = 43; | ||
break; | ||
case SQL_TIMESTAMP_TZ: | ||
var->sqltype = SQL_VARYING + nullable; | ||
var->sqllen = 58; | ||
break; | ||
case SQL_TIME_TZ: | ||
var->sqltype = SQL_VARYING + nullable; | ||
var->sqllen = 46; | ||
break; | ||
} | ||
} | ||
} | ||
/* }}} */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not necessary |
||
#endif | ||
|
||
/* map driver specific error message to PDO error */ | ||
void php_firebird_set_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, const char *state, const size_t state_len, | ||
const char *msg, const size_t msg_len) /* {{{ */ | ||
|
@@ -605,6 +650,11 @@ static bool firebird_handle_preparer(pdo_dbh_t *dbh, zend_string *sql, /* {{{ */ | |
break; | ||
} | ||
|
||
#if FB_API_VER >= 40 | ||
/* set coercing a data type */ | ||
set_coercing_data_type(&S->out_sqlda); | ||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto, please use consistent indentation. |
||
|
||
/* allocate the input descriptors */ | ||
if (isc_dsql_describe_bind(H->isc_status, &s, PDO_FB_SQLDA_VERSION, &num_sqlda)) { | ||
break; | ||
|
@@ -618,6 +668,11 @@ static bool firebird_handle_preparer(pdo_dbh_t *dbh, zend_string *sql, /* {{{ */ | |
if (isc_dsql_describe_bind(H->isc_status, &s, PDO_FB_SQLDA_VERSION, S->in_sqlda)) { | ||
break; | ||
} | ||
|
||
#if FB_API_VER >= 40 | ||
/* set coercing a data type */ | ||
set_coercing_data_type(S->in_sqlda); | ||
#endif | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto |
||
} | ||
|
||
stmt->driver_data = S; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,52 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
--TEST-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
PDO_Firebird: Supported Firebird 4.0 datatypes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
--EXTENSIONS-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
pdo_firebird | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
--SKIPIF-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
<?php require('skipif.inc'); ?> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Versions lower than 4 may want to skip this test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How to set this restriction? $dbh->getAttribute(PDO::ATTR_SERVER_VERSION); Gets the server version, but it looks like this
You also need to use a regular expression to make this information simple. On the other hand, the engine version can be obtained by request select rdb$get_context('SYSTEM', 'ENGINE_VERSION')
from rdb$database There is only a version number. But in any case, the server version can only be found after connecting to the database. Create a separate function that returns the engine version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What matters here is the client version, not the server version. So, could you please try the following constant?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It also returns a complex version of the client LI-V6.3.1.1391 Firebird 5.0 The success of this test depends less on the client (although it does too) and more on the Firebird header files. #if FB_API_VER >= 40
...
#endif FB_API_VER is defined in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you assuming the case that there is a difference between the client version and Now, probably the simplest solution here is to be able to retrieve Could you please wait a moment while I open the PR to add? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sure. The API version is essentially the version of the header files. And the client version is the version of What is the point of the patch? After preparing query, the server says I have these types of columns and input parameters. We cannot process some of them, so in response we tell the server, instead of these types, return VARCHAR(N) to us. And this part, in theory, can generally work independently of the client (I have not tested).
OK. I also think this will be useful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opened PR |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
--XLEAK-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
A bug in firebird causes a memory leak when calling `isc_attach_database()`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
See https://github.com/FirebirdSQL/firebird/issues/7849 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
--FILE-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
<?php | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
require 'testdb.inc'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
$sql = <<<'SQL' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
SELECT | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST(15 AS BIGINT) AS i64, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST(15 AS INT128) AS i128, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
123.97 AS N, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST(123.97 AS NUMERIC(38,2)) AS N2, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST('2024-05-04 12:59:34.239' AS TIMESTAMP) AS TS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST('2024-05-04 12:59:34.239 Europe/Moscow' AS TIMESTAMP WITH TIME ZONE) AS TS_TZ, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST('12:59:34.239' AS TIME) AS T, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST('12:59:34.239 Europe/Moscow' AS TIME WITH TIME ZONE) AS T_TZ, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST(1.128 AS DECFLOAT(16)) AS df16, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
CAST(1.128 AS DECFLOAT(34)) AS df34 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
FROM RDB$DATABASE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
SQL; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
$dbh = getDbConnection(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
$stmt = $dbh->prepare($sql); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
$stmt->execute(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
$data = $stmt->fetch(\PDO::FETCH_ASSOC); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
$stmt->closeCursor(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
$str = json_encode($data, JSON_PRETTY_PRINT); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo $str; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
echo "\ndone\n"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
?> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
--EXPECT-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"I64": 15, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"I128": "15", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"N": "123.97", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"N2": "123.97", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"TS": "2024-05-04 12:59:34", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"TS_TZ": "2024-05-04 12:59:34.2390 Europe\/Moscow", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"T": "12:59:34", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"T_TZ": "12:59:34.2390 Europe\/Moscow", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"DF16": "1.128", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
"DF34": "1.128" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In my environment, the date format is probably different than yours.
I thought about how to specify the format, but I don't want to do that yet because I think it would be quite complicated. Therefore, could you please change your test expectations to:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
done |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
--TEST-- | ||
PDO_Firebird: Supported Firebird 4.0 datatypes (parameters) | ||
--EXTENSIONS-- | ||
pdo_firebird | ||
--SKIPIF-- | ||
<?php require('skipif.inc'); ?> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto |
||
--XLEAK-- | ||
A bug in firebird causes a memory leak when calling `isc_attach_database()`. | ||
See https://github.com/FirebirdSQL/firebird/issues/7849 | ||
--FILE-- | ||
<?php | ||
require 'testdb.inc'; | ||
|
||
$sql = <<<'SQL' | ||
SELECT | ||
CAST(? AS INT128) AS i128, | ||
CAST(? AS NUMERIC(38,2)) AS N2, | ||
CAST(? AS TIMESTAMP WITH TIME ZONE) AS TS_TZ, | ||
CAST(? AS TIME WITH TIME ZONE) AS T_TZ, | ||
CAST(? AS DECFLOAT(16)) AS df16, | ||
CAST(? AS DECFLOAT(34)) AS df34 | ||
FROM RDB$DATABASE | ||
SQL; | ||
|
||
$dbh = getDbConnection(); | ||
|
||
$stmt = $dbh->prepare($sql); | ||
$stmt->execute([12, 12.34, '2024-05-04 12:59:34.239 Europe/Moscow', '12:59 Europe/Moscow', 12.34, 12.34]); | ||
$data = $stmt->fetch(\PDO::FETCH_ASSOC); | ||
$stmt->closeCursor(); | ||
$str = json_encode($data, JSON_PRETTY_PRINT); | ||
echo $str; | ||
echo "\ndone\n"; | ||
?> | ||
--EXPECT-- | ||
{ | ||
"I128": "12", | ||
"N2": "12.34", | ||
"TS_TZ": "2024-05-04 12:59:34.2390 Europe\/Moscow", | ||
"T_TZ": "12:59:00.0000 Europe\/Moscow", | ||
"DF16": "12.34", | ||
"DF34": "12.34" | ||
} | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please standardize indentation to TAB.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, there is an
EMPTY_SWITCH_DEFAULT_CASE()
macro that can be used for the default case where the switch does nothing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you insert this macro, the driver crashes as soon as the dtype goes beyond the cases described in case. In my case, for other types you just need to do nothing.