Re: [PHP-DEV] Re: PHP PDO comment on IRC
Christopher Jones wrote: > Matteo Beccati wrote: >> Here's a proposal: http://www.beccati.com/misc/php/pdo_streams_v4.diff >> >> The idea is to add a new #define PDO_DRIVER_API_CHECK which is verified >> at compile time by the C preprocessor. If its value doesn't match the >> main PDO_DRIVER_API an #error is triggered, e.g: >> >> In file included from /tmp/PDO_PGSQL-1.0.2/pdo_pgsql.c:29: >> /usr/local/include/php/ext/pdo/php_pdo_driver.h:50:2: error: #error "PDO >> driver not compatible with the PDO API version in use" > > Hi Matteo, > > I'll just comment on the internal API versioning part - let's get that > sorted out first. > > To make it easier to write database specific PDO drivers that work > with a range of versions of the internal PDO API, I think > PDO_DRIVER_API_CHECK could be changed to be min & max values that the > driver supports. The check would be: > > #if !defined(PDO_DRIVER_API_IMPLEMENTED_MIN) || \ > !defined(PDO_DRIVER_API_IMPLEMENTED_MAX) || \ > !(PDO_DRIVER_API_IMPLEMENTED_MIN <= PDO_DRIVER_API && \ > PDO_DRIVER_API <= PDO_DRIVER_API_IMPLEMENTED_MAX) > #error "PDO driver not compatible with the PDO API version in use" > #endif > > Adding code documentation to the macros explaining what driver writers > can & should do would be useful (i.e. please add some). > > I'm not keen on the introduction of php_pdo_version.h. It seems a > kludge so that PDO passes its own version check. It makes PDO itself > defines the same value in two places. If you're going to bump > PDO_DRIVER_API and change the API, thus making all current PDO drivers > incompatible, you may as well continue making changes. Perhaps put > the version check code in a new file that is only included in DB > driver files, and not in PDO .c files. Updated patch available at: http://www.beccati.com/misc/php/pdo_streams_v5.diff with MIN/MAX, a bit more documentation and replaced php_pdo_version.h with php_pdo_main.h which defines proper MIN/MAX constant for the extension. I've also posted to pecl-dev and pdo mailing lists asking for feedback from the driver mainteiners. Cheers -- Matteo Beccati OpenX - http://www.openx.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: PHP PDO comment on IRC
Matteo Beccati wrote: > Here's a proposal: http://www.beccati.com/misc/php/pdo_streams_v4.diff > > The idea is to add a new #define PDO_DRIVER_API_CHECK which is verified > at compile time by the C preprocessor. If its value doesn't match the > main PDO_DRIVER_API an #error is triggered, e.g: > > In file included from /tmp/PDO_PGSQL-1.0.2/pdo_pgsql.c:29: > /usr/local/include/php/ext/pdo/php_pdo_driver.h:50:2: error: #error "PDO > driver not compatible with the PDO API version in use" Hi Matteo, I'll just comment on the internal API versioning part - let's get that sorted out first. To make it easier to write database specific PDO drivers that work with a range of versions of the internal PDO API, I think PDO_DRIVER_API_CHECK could be changed to be min & max values that the driver supports. The check would be: #if !defined(PDO_DRIVER_API_IMPLEMENTED_MIN) || \ !defined(PDO_DRIVER_API_IMPLEMENTED_MAX) || \ !(PDO_DRIVER_API_IMPLEMENTED_MIN <= PDO_DRIVER_API && \ PDO_DRIVER_API <= PDO_DRIVER_API_IMPLEMENTED_MAX) #error "PDO driver not compatible with the PDO API version in use" #endif Adding code documentation to the macros explaining what driver writers can & should do would be useful (i.e. please add some). I'm not keen on the introduction of php_pdo_version.h. It seems a kludge so that PDO passes its own version check. It makes PDO itself defines the same value in two places. If you're going to bump PDO_DRIVER_API and change the API, thus making all current PDO drivers incompatible, you may as well continue making changes. Perhaps put the version check code in a new file that is only included in DB driver files, and not in PDO .c files. Chris -- Email: christopher.jo...@oracle.com Twitter: http://twitter.com/ghrd Free PHP Book: http://www.oracle.com/technology/tech/php/pdf/underground-php-oracle-manual.pdf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: PHP PDO comment on IRC
Hi Chris, Cc-ing Internals > In case you don't log IRC chats, below are some comments from today. > The intent is to prevent externally maintained PDO drivers (e.g. IBM's) > from being used with PDO unless they have been updated to any > new API changes. > > I believe simply updating PDO_DRIVER_API is not sufficient. Here's a proposal: http://www.beccati.com/misc/php/pdo_streams_v4.diff The idea is to add a new #define PDO_DRIVER_API_CHECK which is verified at compile time by the C preprocessor. If its value doesn't match the main PDO_DRIVER_API an #error is triggered, e.g: In file included from /tmp/PDO_PGSQL-1.0.2/pdo_pgsql.c:29: /usr/local/include/php/ext/pdo/php_pdo_driver.h:50:2: error: #error "PDO driver not compatible with the PDO API version in use" That said, being able to change a function signature made it very easy to cleanly fix the bug I was trying to squash (i.e. PDO not able to distinguish between stream and empty string LOBs returned by the driver because both have 0 as len parameter). As a result PDO now now returns all LOBs as streams, unless instructed to do otherwise (STRINGIFY). Previously only non-empty LOBs were streams: I know that the change breaks BC but seems more consistent to me. If it's not desirable most likely I'd need some help with streams... IRC conversation excerpt follows: > > > cjorcl: Spritz, johannes_ the pdo api version for PDO_DRIVER_API check > seems to be a binary runtime check. The macro is only defined > in PDO and is used in all drivers built against that PDO code. > This doesn't guarantee the driver code was designed for that > PDO API. Perhaps pdo_driver_t can have a field added at the > end. Each driver would need to set this to a driver-specific > hard coded value equal to the value of PDO_DRIVER_API. Then > php_pdo_register_driver could verify the value matches the API > version. This is still a runtime check, but stops any > externally maintained driver from being used. Also a compile > time check could be added to drivers under the control of the > PHP community. > > johannes_: cjorcl: jut make sure that API changes are backwards > compatible or break compiling :-p > > cjorcl: johannes_, that's why the pdo_driver_t field would beed to be > added at the end fo the struct > > johannes_: and making it part of the structure is bad if you have lot > of additions not breaking BC, it wuld have to be a callback > function > > cjorcl: johannes_, adding a register time callback is a bigger task - > but would break compilation compatibility nicely for > non-updated drivers > > Cheers -- Matteo Beccati OpenX - http://www.openx.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
