Description: ------------ It appears that the mapNativeDatatype method in the MDB2_Driver_Datatype_mysql class is inadvertently assinging an internal datatype of 'blob' to MySQL 'TEXT' columns. This occurs on my setup (Linux, PHP 4.3.8, MySQL 3.23). I fixed this by editing line 344. Replace: $type[] = 'blob'; With: if (substr_count($field['flags'],'binary')) { $type[] = 'blob'; } else { $type[] = 'text'; } This solves the internal datatype issue, however tableInfo() still reports the actual 'type' as a 'blob'. It seems that the mysql_field_type() call in the tableInfo() method in the MDB2_Driver_Reverse_mysql class is inconsistent in how it reports various MySQL data types. Read the PHP Manual entry for mysql_field_type() to see other's results. I modified the tableInfo() method as follows to get more consistent data back on the datatype from MySQL: Replaced: $db->loadModule('Datatype', null, true); for ($i = 0; $i < $count; $i++) { $res[$i] = array( 'table' => $case_func(@mysql_field_table($id, $i)), 'name' => $case_func(@mysql_field_name($id, $i)), 'type' => @mysql_field_type($id, $i), 'length' => @mysql_field_len($id, $i), 'flags' => @mysql_field_flags($id, $i), ); With: if ($got_string) { $x_query = 'SHOW COLUMNS FROM ' . $db->quoteIdentifier($result); $x_result =& $db->queryAll($x_query, false); if (PEAR::isError($x_result)) { return $id; } } $db->loadModule('Datatype', null, true); for ($i = 0; $i < $count; $i++) { $res[$i] = array( 'table' => $case_func(@mysql_field_table($id, $i)), 'name' => $case_func(@mysql_field_name($id, $i)), 'type' => @mysql_field_type($id, $i), 'length' => @mysql_field_len($id, $i), 'flags' => @mysql_field_flags($id, $i), ); if (($got_string) && ($x_result)) { $x_result[$i]['type'] = preg_replace('/\(.*?\)$/','',$x_result[$i]['type']); $res[$i]['type'] = $case_func($x_result[$i]['type']); } This got the actual MySQL datatype into the array before it was passed off to the datatype module. I didn't do anything with the 'flags' array key. It still read 'blob' on my setup.

Could you try CVS?

I tested with the CSV version and the issue remains. I did some further hacking around and discovered that if I made the right mods in the tableInfo() method in the MDB2_Driver_Reverse_mysql class, no modifications were needed in the mapNativeDatatype() method in the MDB2_Driver_Datatype_mysql class. Below is a full copy of my patched tableInfo() method. I modified the original code posted substantially: 1. Now accounts for getting info on a result set instead of just a table ($got_string can == true or false). 2. Optimized for single table ($got_string == true) however will work if using a result set and the table returned is different. You may want to optimize further as right now this code is calling the 'SHOW COLUMNS FROM '... SQL for each column if a result set is being worked on. I only call tableInfo() on static tables so this doesn't matter much to me. 3. Attempts to use the 'SHOW COLUMNS FROM '... SQL but on failure reverts back to old behavior if that can't be done. 4. Strips 'binary' and 'blob' flags if 'mdb2type' is 'text'. (MySQL was returning a 'blob' tag for 'text' types for some reason. 5. NOTE: Now I'm looking at the mysqli driver which I imagine will need a similar fix. I won't post a bug there as that driver seems to behave correctly even though better info could be returned with this new methodology. ==============BEGIN_CUT function tableInfo($result, $mode = null) { $db =& $this->getDBInstance(); if (PEAR::isError($db)) { return $db; } if (is_string($result)) { /* * Probably received a table name. * Create a result resource identifier. */ $query = 'SELECT * FROM '.$db->quoteIdentifier($result).' LIMIT 0'; $id =& $db->_doQuery($query, false); if (PEAR::isError($id)) { return $id; } $got_string = true; } elseif (MDB2::isResultCommon($result)) { /* * Probably received a result object. * Extract the result resource identifier. */ $id = $result->getResource(); $got_string = false; } else { /* * Probably received a result resource identifier. * Copy it. * Deprecated. Here for compatibility only. */ $id = $result; $got_string = false; } if (!is_resource($id)) { return $db->raiseError(MDB2_ERROR_NEED_MORE_DATA); } if ($db->options['portability'] & MDB2_PORTABILITY_FIX_CASE) { if ($db->options['field_case'] == CASE_LOWER) { $case_func = 'strtolower'; } else { $case_func = 'strtoupper'; } } else { $case_func = 'strval'; } $count = @mysql_num_fields($id); $res = array(); if ($mode) { $res['num_fields'] = $count; } ///////// BEGIN PATCH - webmaster@kryptronic.com $x_result = null; if ($got_string) { $x_query = 'SHOW COLUMNS FROM ' . $db->quoteIdentifier($result); $x_result =& $db->queryAll($x_query, false); if (PEAR::isError($x_result)) {$x_result = null;} } ///////// END PATCH - webmaster@kryptronic.com $db->loadModule('Datatype', null, true); for ($i = 0; $i < $count; $i++) { $res[$i] = array( 'table' => $case_func(@mysql_field_table($id, $i)), 'name' => $case_func(@mysql_field_name($id, $i)), 'type' => @mysql_field_type($id, $i), 'length' => @mysql_field_len($id, $i), 'flags' => @mysql_field_flags($id, $i), ); ///////// BEGIN PATCH - webmaster@kryptronic.com if (!($got_string)) { $x_query = 'SHOW COLUMNS FROM ' . $db->quoteIdentifier($res[$i]['table']); $x_result =& $db->queryAll($x_query, false); if (PEAR::isError($x_result)) {$x_result = null;} } if ($x_result) { foreach ($x_result as $x) { if ($x['field'] == $res[$i]['name']) { $x['type'] = preg_replace('/\(.*?\)$/','',$x['type']); $res[$i]['type'] = $x['type']; } } } ///////// END PATCH - support@kryptronic.com if ($res[$i]['type'] == 'string') { $res[$i]['type'] = 'char'; } elseif ($res[$i]['type'] == 'unknown') { $res[$i]['type'] = 'decimal'; } $mdb2type_info = $db->datatype->mapNativeDatatype($res[$i]); if (PEAR::isError($mdb2type_info)) { return $mdb2type_info; } $res[$i]['mdb2type'] = $mdb2type_info[0][0]; if ($mode & MDB2_TABLEINFO_ORDER) { $res['order'][$res[$i]['name']] = $i; } if ($mode & MDB2_TABLEINFO_ORDERTABLE) { $res['ordertable'][$res[$i]['table']][$res[$i]['name']] = $i; } ///////// BEGIN PATCH - support@kryptronic.com if ($x_result) { if ($res[$i]['mdb2type'] == 'text') { $x_flags = explode(' ',$res[$i]['flags']); $z_flags = array(); foreach ($x_flags as $x) { if (($x != 'binary') && ($x != 'blob')) {$z_flags[] = $x;} } $res[$i]['flags'] = implode(' ',$z_flags); } } ///////// END PATCH - support@kryptronic.com } // free the result only if we were called on a table if ($got_string) { @mysql_free_result($id); } return $res; } ==============END_CUT

I was referring to changes in mapNativeDatatype(). The issue with tableInfo() is well .. there is already functionality to read the information schema in MDB2 using getTableFieldDefinition(). The code for tableInfo() is taken from PEAR::DB. Obviously for result set introspection getTableFieldDefinition() cannot be used. However when just fetching infos about a given table it could be used. Then again getTableFieldDefinition() does not expose any native information, since it only returns the mapped datatypes. So I am not sure how to move forward with this. I do not want to duplicate a lot of code and I also do not want to needlessly break a lot of code that migrates from PEAR::DB.

I think the issue here is that mysql_field_type() reports the field type inconsistently in different versions of PHP. Read here for info: http://us2.php.net/manual/en/function.mysql-field-type.php In my case mysql_field_type() was reporting a type of BLOB for TEXT, MEDIUMTEXT and LONGTEXT. Additionally, mysql_field_type() was reporting VARCHAR and CHAR types as CHARs. This was causing problems when doing table introspection. For my purposes I need to be able to compare the actual table definition with the definition in my XML schema so as to prepare a changes array for MDB2. The MDB2 native types did not have enough detail in them for me to be able to tell the difference between something like a CHAR or a VARCHAR or a TEXT field. They are all 'text'. So when the time comes to compare the schema to the actual table definition I had no idea whether I need to make changes to the column datatypes or not. I know dealing with the rdbms types in an application goes against what MDB2 stands for, however when you have users using utilities like PHPMyAdmin to change table structures in addition to an MDB2-based app problems can arise if a good level of detail for the column type is not available. Consider the following: (1) MDB2 already uses 'SHOW COLUMNS FROM ' ... SQL in getTableFieldDefinition() so using this SQL in tableInfo() is not out of the question. (2) Because tableInfo() relies on mysql_field_type() to get the rdbms datatype, result consistency in the 'type' key between PHP and MySQL versions is non-existent. Any programmer currently relying on the tableInfo 'type' field instead of the 'mdb2type' field for table field info is crazy if they care about application portability. (3) Because of (2) above, the 'type' key returned by tableInfo() really should be used for informational purposes only. (4) Using the new SQL format in tableInfo() creates more consistency within the driver as getTableFieldDefinition() uses those results already to map back to MDB2 types. Right now the two methods are inconsistent because they're mapping different data back to the same native types. I recommend the changes be made to tableInfo() to use the 'SHOW COLUMNS FROM ' ... SQL for these reasons. Right now there is no way to get the actual column types for MySQL tables out of MDB2. These are just my thoughts. I am going to use my patched mysql and mysql drivers in production as they do what I need them to do. I really like MDB2 and only posted this as a bug as I thought the behavior was unreasonable and easilly fixed. BTW: I just tested this same patch against the mysqli tableInfo() method and it worked fine without any changes.

Thats the point. Why dont you just use getTableFieldDefinition()? tableInfo() is more about exposing the native information using the functions provided by the given extension. the get*Definition() methods instead construct their own SQL statements to read from the RDBMS information schema.# As I said I do not want to add SHOW COLUMN to tableInfo() due to code redundancy. The only that might be feasible is to rewrite tableInfo() to use getTableFieldDefinition() if a string with a table name was passed. This would however require that getTableFieldDefinition() also return the native datatype etc.

The issue with getTableFieldDefinition() is that it uses the Datatype method mapNativeDatatype() to return the 'type' key. In order for that to be usable it would have to return 'type' as the actual type and 'mdb2type' as the native type. As it is now getTableFieldDefinition() and tableInfo() return different types for my setup: getTableFieldDefinition() returns 'clob' as 'type' for MySQL 'TEXT' columns. tableInfo() returns 'blob' as 'type' and 'mdb2type' for MySQL 'TEXT' columns. While getTableFieldDefinition() is better, it still doesn't give me an accurate MySQL type.

Yes this is what I mean. It sounds to me like getTableFieldDefinition() should simply be extended to also return the RDBMS native datatype and then it could be used in tableInfo(). This should not be all that hard, just a bit of work. Obviously to maintain backwards compatibility getTableFieldDefinition() will have to return the native type as "nativetype" and then tableInfo() would have to copy the types from type => mdbtype and from nativetype => type

That would totally work. It's a bit confusing because of the use of the key 'type' in both methods, however it would work. To recap: (1) tableInfo() 'type' key returns RDBMS native type (2) tableInfo() 'mdb2type' key returns MDB2 type (3) getTableFieldDefinition() 'type' key returns MDB2 type (4) getTableFieldDefinition() 'nativetype' key returns RDBMS native type This would bypass (read: eliminate) the problems associated with mysql_field_type() altogether as MySQL would report the native column type itself. I assume this would be impelemented in the mysqli driver as well. Correct? What about the other drivers? My research indicates: (1) MySQL - see above (2) MySQLi - see above (3) PostgreSQL - uses pg_field_type() which (according to the PHP Manual) returns accurate info. It looks like getTableFieldDefinition() gets info right from PostgreSQL which would be much better. (4) MS SQL Server - uses mssql_field_type() which appears to have the same sort of issues. There is a great post by huib at bestia dot com on 27-May-2005 03:18 here: http://us2.php.net/manual/en/function.mssql-field-type.php That gives good SQL to use. It looks like getTableFieldDefinition() is extended from the Common driver in this module currently. (5) Oracle - Uses SQL in both methods to get column info but looks to hit different tables. This may be OK. Not sure. (6) SQLite - Uses SQL in both methods to get column info but looks to hit different tables. This may be OK. Not sure. Maybe you can explain whether types matter at all in SQLite when creating tables or adding new columns (please?). Concerning SQLite (very off subject): You may be able to support a 'remove' in the ALTER TABLE code with SQLite. Read here for more info (FAQ: #13 How do I add or delete columns from an existing table in SQLite.): http://www.sqlite.org/faq.html#q12 Applying the SQLite example above further you may be able to support full ALTER TABLE functionality. ------- Sorry for all the extra non-MySQL driver details here. I figured they may help if you're re-arranging how things work.

I would probably rewrite tableInfo() to always use getTableFieldDefinition() when receiving a table name.

I just had an idea which would help the API make a little more sense. Add a 'mdb2type' key to getTableFieldDefinition() and set it equal to the 'type' key. Add a 'nativetype' key to tableInfo() and set it equal to the 'type' key. In the end you would have: (1) tableInfo() 'type' key returns RDBMS native type (2) tableInfo() 'mdb2type' key returns MDB2 type (3) tableInfo() 'nativetype' key returns RDBMS native type (4) getTableFieldDefinition() 'type' key returns MDB2 type (5) getTableFieldDefinition() 'mdb2type' key returns MDB2 type (6) getTableFieldDefinition() 'nativetype' key returns RDBMS native type At some point you could deprecate the 'type' key in the two methods.

sounds good to me .. though deprecation is not really an option until a new major version is written.

This is a more general issue.

I've determined after testing the remaining drivers that in it's current state MDB2 is not going to fit well with my project. Over the past couple of weeks I've rolled my own DBI interface directly into my application based on how MDB2 works using all the same system calls. In my programming I determined that there is no good way to map an internal (MDB2) type back to a native type during a tableInfo() lookup because length parameters preclude getting matching anything exactly. The best method I've come up with is the following: (1) Use MDB2 internal types to create database columns by using them plus length, notnull and default parameters to create a solid declaration string. (2) Use a native type declaration (calculated as the same type declaration used to create a column) in tableInfo(). (3) When doing comparissons of existing columns and XML db definitions, do the comparisson on what the application thinks the actual native declaration string should be versus what the database reports. If you'd like any of the SQL statements needed to get proper tableInfo() for the RDBMSs, please let me know.

Uhm .. didn't we agree that the plan was to start using getTableFieldDefinition() in tableInfo() when a table name was passed and not a result resource. And in this case we would return 'mdb2type' and 'nativetype' and for BC 'type' as the 'nativetype' in tableInfo() and 'type' as the 'mdb2type' in getTableFieldDefinition(). Did you change any of the SQL statements compared to what we have in getTableFieldDefinition()?

This bug has been fixed in CVS. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better. - reworked tableInfo() to use a common implementation based on getTableFieldDefinition() when a table name is passed (Bug #8124) - added nativetype output to tableInfo() and getTableFieldDefinition() - added mdb2type output to getTableFieldDefinition() this is a slight BC break, but has become necessary since tableInfo() was providing unreliable and not portable information most noteably 'flags' is no longer supported in tableInfo() when passing in a table name, which was not portable anyways

I have fixed the BC issue in flags support.