Re: Re: cvs: pecl /ibm_db2 TODO config.m4 ibm_db2.c php_ibm_db2.h

On Apr 6, 2005 11:36 AM, Andrey Hristov <php@hristov.com> wrote: > Dan Scott wrote: > > On Apr 6, 2005 3:28 AM, Andrey Hristov <php@hristov.com> wrote: > ... > > >>> > >>> > >>> > >>>------------------------------------------------------------------------ > >> > >>It's great that there is now a DB2 extension! > > > > > > We're pretty excited about it -- obviously we still have some work to > > do (looking in the TODO file) but it seemed like the right time to get > > code out there for the keen eyes of the PECL folk to look over. And > > good thing, too :) > > > Hi Dan, > few more things :) > 1) You mentioned that ou have tests. Are they in the PHP test format (.phpt files) > or ? It would be nice if you commit them to the repository so the people can > try before coding. This may give you better testing. Yep, we have a set of tests in phpt format. I plan to commit those in the next day or two, followed by docs. There are a few tests that depend on the DB2 sample database; I was hoping to remove that dependency to make them true unit tests (and so that they work better with Cloudscape and Derby as target database servers), but rather than hold up the commit I'll add them as-is, with a TODO to remove that dependency later. Hopefully sooner rather than later :) > 2) Forgot to mention that last time, But there are several functions which do the same > and have a difference in the form of one underscore. Example : > > PHP_FE(db2_specialcolumns, NULL) > PHP_FE(db2_special_columns, NULL) > > The first one is not according to the naming conventions and it would be good if you > drop it (all other similar cases) before the API is publically available. The issue here is that one of our goals is to make it as painless as possible to migrate between Unified ODBC (the only driver that currently supports DB2 / Cloudscape / Derby databases) and the ibm_db2 driver, and Unified ODBC currently only supports the smashedtogethernames. We plan to document these only as aliases for the fully_supported_names (that is, they don't get their own page in the manual). > 3) I saw that you use globals (IBM_DB2_G) and I have some concerns about that which > I would like to express :) . These globals are per process. In a multithreaded enviroment > IMO a race condition may appear since while a connection has just failed another one > is maybe in process of successful creation which will call _php_clear_conn_err_cache(), > which in turn will clear the error message. So, at the end a connection may fail and still > the user may get an empty error message. So, your approach is valid only for non-MT envs. > Therefore, may be it is better to add a new member of type char* to you conn_handle and > stmt_handle. Interesting -- I'll pass that on. We definitely want to support multithreaded environments :) Obviously we're still on the learning curve -- Sara's extension writing tutorial has been a great help, and we're looking forward to part II. Your feedback also helps greatly. Thank you very much! Dan