PDO Firebird bugfixes

Unified diff for ext/pdo_firebird/php_pdo_firebird_int.h: diff -u -r1.10.2.1.2.1 php_pdo_firebird_int.h --- php_pdo_firebird_int.h 1 Jan 2007 09:36:04 -0000 1.10.2.1.2.1 +++ php_pdo_firebird_int.h 18 Oct 2007 19:52:09 -0000 @@ -31,6 +31,10 @@ #define PDO_FB_DIALECT 3 +#define PDO_FB_DEF_DATE_FMT "%Y-%m-%d" +#define PDO_FB_DEF_TIME_FMT "%H:%M:%S" +#define PDO_FB_DEF_TIMESTAMP_FMT PDO_FB_DEF_DATE_FMT " " PDO_FB_DEF_TIME_FMT + #define SHORT_MAX (1 << (8*sizeof(short)-1)) #if SIZEOF_LONG == 8 @@ -73,6 +77,11 @@ /* the last error that didn't come from the API */ char const *last_app_error; + /* date and time format strings, can be set by the set_attribute method */ + char *date_format; + char *time_format; + char *timestamp_format; + } pdo_firebird_db_handle; @@ -115,6 +124,12 @@ void _firebird_error(pdo_dbh_t *dbh, pdo_stmt_t *stmt, char const *file, long line TSRMLS_DC); +enum { + PDO_FB_ATTR_DATE_FORMAT = PDO_ATTR_DRIVER_SPECIFIC, + PDO_FB_ATTR_TIME_FORMAT, + PDO_FB_ATTR_TIMESTAMP_FORMAT, +}; + #endif /* PHP_PDO_FIREBIRD_INT_H */ /*

ext/pdo_firebird/pdo_firebird.c: diff -u -r1.4.2.3.2.1.2.1 pdo_firebird.c --- pdo_firebird.c 27 Sep 2007 18:00:42 -0000 1.4.2.3.2.1.2.1 +++ pdo_firebird.c 18 Oct 2007 19:42:47 -0000 @@ -55,6 +55,10 @@ PHP_MINIT_FUNCTION(pdo_firebird) /* {{{ */ { + REGISTER_PDO_CLASS_CONST_LONG("FB_ATTR_DATE_FORMAT", (long) PDO_FB_ATTR_DATE_FORMAT); + REGISTER_PDO_CLASS_CONST_LONG("FB_ATTR_TIME_FORMAT", (long) PDO_FB_ATTR_TIME_FORMAT); + REGISTER_PDO_CLASS_CONST_LONG("FB_ATTR_TIMESTAMP_FORMAT", (long) PDO_FB_ATTR_TIMESTAMP_FORMAT); + php_pdo_register_driver(&pdo_firebird_driver); return SUCCESS;

ext/pdo_firebird/firebird_driver.c: diff -u -r1.17.2.2.2.4 firebird_driver.c --- firebird_driver.c 27 Feb 2007 03:28:16 -0000 1.17.2.2.2.4 +++ firebird_driver.c 18 Oct 2007 20:01:47 -0000 @@ -114,6 +114,16 @@ RECORD_ERROR(dbh); } + if (H->date_format) { + efree(H->date_format); + } + if (H->time_format) { + efree(H->time_format); + } + if (H->timestamp_format) { + efree(H->timestamp_format); + } + pefree(H, dbh->is_persistent); return 0; @@ -482,6 +492,30 @@ dbh->auto_commit = Z_BVAL_P(val); } return 1; + + case PDO_FB_ATTR_DATE_FORMAT: + convert_to_string(val); + if (H->date_format) { + efree(H->date_format); + } + spprintf(&H->date_format, 0, "%s", Z_STRVAL_P(val)); + return 1; + + case PDO_FB_ATTR_TIME_FORMAT: + convert_to_string(val); + if (H->time_format) { + efree(H->time_format); + } + spprintf(&H->time_format, 0, "%s", Z_STRVAL_P(val)); + return 1; + + case PDO_FB_ATTR_TIMESTAMP_FORMAT: + convert_to_string(val); + if (H->timestamp_format) { + efree(H->timestamp_format); + } + spprintf(&H->timestamp_format, 0, "%s", Z_STRVAL_P(val)); + return 1; } return 0; } @@ -500,7 +534,7 @@ /* }}} */ /* called by PDO to get a driver-specific dbh attribute */ -static int firebird_handle_get_attribute(pdo_dbh_t const *dbh, long attr, zval *val TSRMLS_DC) /* {{{ */ +static int firebird_handle_get_attribute(pdo_dbh_t *dbh, long attr, zval *val TSRMLS_DC) /* {{{ */ { pdo_firebird_db_handle *H = (pdo_firebird_db_handle *)dbh->driver_data; @@ -615,7 +649,7 @@ char dpb_buffer[256] = { isc_dpb_version1 }, *dpb; dpb = dpb_buffer + 1; - + /* loop through all the provided arguments and set dpb fields accordingly */ for (i = 0; i < sizeof(dpb_flags); ++i) { if (dpb_values[i] && buf_len > 0) { @@ -627,15 +661,14 @@ } /* fire it up baby! */ - if (isc_attach_database(H->isc_status, 0, vars[0].optval, &H->db,(short)(dpb-dpb_buffer), - dpb_buffer)) { + if (isc_attach_database(H->isc_status, 0, vars[0].optval, &H->db,(short)(dpb-dpb_buffer), dpb_buffer)) { break; } dbh->methods = &firebird_methods; dbh->native_case = PDO_CASE_UPPER; dbh->alloc_own_columns = 1; - + ret = 1; } while (0); @@ -646,6 +679,14 @@ } } + if (!dbh->methods) { + char errmsg[512]; + ISC_STATUS *s = H->isc_status; + isc_interprete(errmsg, &s); + zend_throw_exception_ex(php_pdo_get_exception(), 0 TSRMLS_CC, "SQLSTATE[%s] [%d] %s", + "HY000", (short) H->isc_status[1], errmsg); + } + if (!ret) { firebird_handle_closer(dbh TSRMLS_CC); } @@ -654,6 +695,7 @@ } /* }}} */ + pdo_driver_t pdo_firebird_driver = { /* {{{ */ PDO_DRIVER_HEADER(firebird), pdo_firebird_handle_factory

ext/pdo_firebird/firebird_statement.c: diff -u -r1.18.2.1.2.5 firebird_statement.c --- firebird_statement.c 27 Feb 2007 03:04:40 -0000 1.18.2.1.2.5 +++ firebird_statement.c 18 Oct 2007 19:52:35 -0000 @@ -170,43 +170,11 @@ col->maxlen = var->sqllen; col->namelen = var->aliasname_length; col->name = estrndup(var->aliasname,var->aliasname_length); - - return 1; -} -/* }}} */ + col->param_type = PDO_PARAM_STR; -#if abies_0 -/* internal function to override return types of parameters */ -static void set_param_type(enum pdo_param_type *param_type, XSQLVAR const *var) /* {{{ */ -{ - /* set the param type after the field type */ - switch (var->sqltype & ~1) { - case SQL_INT64: -#if SIZEOF_LONG < 8 - if (0) /* always a string if its size exceeds native long */ -#endif - case SQL_SHORT: - case SQL_LONG: - if (var->sqlscale == 0) { - *param_type = PDO_PARAM_INT; - break; - } - case SQL_TEXT: - case SQL_VARYING: - case SQL_TYPE_DATE: - case SQL_TYPE_TIME: - case SQL_TIMESTAMP: - case SQL_BLOB: - *param_type = PDO_PARAM_STR; - break; - case SQL_FLOAT: - case SQL_DOUBLE: - *param_type = PDO_PARAM_DBL; - break; - } + return 1; } /* }}} */ -#endif #define FETCH_BUF(buf,type,len,lenvar) ((buf) = (buf) ? (buf) : \ emalloc((len) ? (len * sizeof(type)) : ((*(unsigned long*)lenvar) = sizeof(type)))) @@ -302,15 +270,24 @@ *ptr = NULL; *len = 0; } else { - /* override the column param type */ - /* set_param_type(&stmt->columns[colno].param_type,var); */ - stmt->columns[colno].param_type = PDO_PARAM_STR; - if (var->sqlscale < 0) { - static ISC_INT64 const scales[] = { 1, 10, 100, 1000, 10000, 100000, 1000000, - 100000000, 1000000000, 1000000000, LL_LIT(10000000000),LL_LIT(100000000000), - LL_LIT(10000000000000), LL_LIT(100000000000000),LL_LIT(1000000000000000), - LL_LIT(1000000000000000),LL_LIT(1000000000000000000) }; + static ISC_INT64 const scales[] = { 1, 10, 100, 1000, + 10000, + 100000, + 1000000, + 10000000, + 100000000, + 1000000000, + LL_LIT(10000000000), + LL_LIT(100000000000), + LL_LIT(1000000000000), + LL_LIT(10000000000000), + LL_LIT(100000000000000), + LL_LIT(1000000000000000), + LL_LIT(10000000000000000), + LL_LIT(100000000000000000), + LL_LIT(1000000000000000000) + }; ISC_INT64 n, f = scales[-var->sqlscale]; switch (var->sqltype & ~1) { @@ -340,11 +317,6 @@ struct tm t; char *fmt; -/** -* For the time being, this code has been changed to convert every returned value to a string -* before passing it back up to the PDO driver. -*/ - case SQL_VARYING: *ptr = &var->sqldata[2]; *len = *(short*)var->sqldata; @@ -353,7 +325,6 @@ *ptr = var->sqldata; *len = var->sqllen; break; -/* --- cut here --- */ case SQL_SHORT: *ptr = FETCH_BUF(S->fetch_buf[colno], char, CHAR_BUF_LEN, NULL); *len = slprintf(*ptr, CHAR_BUF_LEN, "%d", *(short*)var->sqldata); @@ -374,51 +345,18 @@ *ptr = FETCH_BUF(S->fetch_buf[colno], char, CHAR_BUF_LEN, NULL); *len = slprintf(*ptr, CHAR_BUF_LEN, "%F" , *(double*)var->sqldata); break; -/* --- cut here --- */ -#if abies_0 - case SQL_SHORT: - *ptr = FETCH_BUF(S->fetch_buf[colno], long, 0, *len); - *(long*)*ptr = *(short*)var->sqldata; - break; - case SQL_LONG: -#if SIZEOF_LONG == 8 - *ptr = FETCH_BUF(S->fetch_buf[colno], long, 0, *len); - *(long*)*ptr = *(ISC_LONG*)var->sqldata; -#else - *ptr = var->sqldata; -#endif - break; - case SQL_INT64: - *len = sizeof(long); -#if SIZEOF_LONG == 8 - *ptr = var->sqldata; -#else - *ptr = FETCH_BUF(S->fetch_buf[colno], char, CHAR_BUF_LEN, NULL); - *len = slprintf(*ptr, CHAR_BUF_LEN, "%" LL_MASK "d", *(ISC_INT64*)var->sqldata); -#endif - break; - case SQL_FLOAT: - *ptr = FETCH_BUF(S->fetch_buf[colno], double, 0, *len); - *(double*)*ptr = *(float*)var->sqldata; - break; - case SQL_DOUBLE: - *ptr = var->sqldata; - *len = sizeof(double); - break; -#endif case SQL_TYPE_DATE: isc_decode_sql_date((ISC_DATE*)var->sqldata, &t); - fmt = INI_STR("ibase.dateformat"); + fmt = S->H->date_format ? S->H->date_format : PDO_FB_DEF_DATE_FMT; if (0) { case SQL_TYPE_TIME: isc_decode_sql_time((ISC_TIME*)var->sqldata, &t); - fmt = INI_STR("ibase.timeformat"); + fmt = S->H->time_format ? S->H->time_format : PDO_FB_DEF_TIME_FMT; } else if (0) { case SQL_TIMESTAMP: isc_decode_timestamp((ISC_TIMESTAMP*)var->sqldata, &t); - fmt = INI_STR("ibase.timestampformat"); + fmt = S->H->timestamp_format ? S->H->timestamp_format : PDO_FB_DEF_TIMESTAMP_FMT; } - /* convert the timestamp into a string */ *ptr = FETCH_BUF(S->fetch_buf[colno], char, *len = 80, NULL); *len = strftime(*ptr, *len, fmt, &t); @@ -618,13 +556,6 @@ ZVAL_LONG(param->parameter, *(long*)value); break; } -#if abies_0 - case PDO_PARAM_DBL: - if (value) { - ZVAL_DOUBLE(param->parameter, *(double*)value); - break; - } -#endif default: ZVAL_NULL(param->parameter); }

"Live and learn" ... Trying once again: Please find attached a unified diff (diff -u) for 4 files addressing bugfixes for PDO_firebird in the PHP_5_3 branch. The following bug reports are addressed: Bug #36128 (Interbase PDO - timestamp columns return NULL) When pdo_firebird is enabled, and interbase is disabled, then timestamps and dates are returned as NULL strings, as the format relies on INI-entries made by the interbase extension. The solution (after dialogue with Wez) is to NOT use INI-entries at all, but give the user the possibility to set format strings with PDO::setAttribute() and to define a set of ISO8601 default format strings. This is handled in: ext/pdo_firebird/php_pdo_firebird_int.h (all changes) # Added ISO8601 format definitions ext/pdo_firebird/pdo_firebird.c (all changes) # Registering of the format attributes ext/pdo_firebird/firebird_driver.c (@@ -114,6 +114,16 @@) (@@ -482,6 +492,30 @@) (@@ -500,7 +534,7 @@) # Removing a compiler warning ext/pdo_firebird/firebird_statement.c (@@ -374,51 +345,18 @@) # Which also removes some dead code Bug #39822 (new PDO() doesn't work with firebird) Bug #41522 (PDO firebird driver returns null if it fails to connect) The driver doesn't throw an exception (which it should according to the specification). The solution is to throw this exception (inspired by the solution in one of the other pdo drivers) in the pdo_firebird_handle_factory() function. This is handled in: ext/pdo_firebird/firebird_driver.c (@@ -646,6 +679,14 @@) Bug #35386 (firebird: first row is null) This was a bit hard to track down, but here goes: When columns of first row is handled (one by one), the datatype of the column is set to STRING in firebird_stmt_get_col(). But this was done AFTER the columntype already was set prior to the call of firebird_stmt_get_col(). Therefore the columntype was correct when the second row was handled. The columntype wasn't initialzed to a sane value in firebird_stmt_describe(). The solution (again with reference to the PDO spec I got from Wez) is to REMOVE the columntype (or param_type) handling from firebird_stmt_get_col() and put it into the right place: firebird_stmt_describe(). All parameters going from the driver to the PDO object should be passed as strings (according to the specification), thus all the code (#ifdef'ed out by abies) regarding native longs, shorts, etc. is removed. Bugfixes: ext/pdo_firebird/firebird_statement.c (@@ -170,43 +170,11 @@) # Set param_type to STRING (@@ -340,11 +317,6 @@) # Remove dead code (@@ -353,7 +325,6 @@) # Remove dead code (@@ -618,13 +556,6 @@) # Remove dead code Bug #39700 (NUMERIC error when result precision are 7,8 or 12-14 ) Took the solution from the comment section of the bugreport and put into play. ext/pdo_firebird/firebird_statement.c (@@ -302,15 +270,24 @@) Hope this is better Best regards Lars