Home » Database » MDB2 » Bug #8124

[2006-07-06 06:15 UTC] webmaster at kryptronic dot com

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.

[2006-07-06 14:41 UTC] webmaster at kryptronic dot com

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

[2006-07-06 15:20 UTC] webmaster at kryptronic dot com

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.

[2006-07-06 16:56 UTC] webmaster at kryptronic dot com

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.

[2006-07-06 17:24 UTC] webmaster at kryptronic dot com

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.

[2006-07-06 17:57 UTC] webmaster at kryptronic dot com

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.

[2006-07-16 18:35 UTC] webmaster at kryptronic dot com

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.