When using $smcFunc['db_create_table'] if you have an incorrect table definition, like defining a size value for a TEXT field, the table is not created but there are no errors either -even when you are on debug mode-.
This gives mod authors a hard time debugging the issue and figuring out the problem. Here some examples of it.
http://www.simplemachines.org/community/index.php?topic=224166.msg1814902#msg1814902
http://www.simplemachines.org/community/index.php?topic=276008.0
http://www.simplemachines.org/community/index.php?topic=299177.0
Thanks,
Bug #3405: Table creation does not return success of failure (http://dev.simplemachines.org/mantis/view.php?id=3405)
You can fix this by modifying DbPackages-mysq.php
Find:
// Create the table!
$smcFunc['db_query']('', $table_query,
'security_override'
);
Add After:
// Does it exist?
$request = $smcFunc['db_query']('', '
SHOW TABLES LIKE {string:table}',
array(
'table' => $table_name,
));
// Oh no!
if ($smcFunc['db_num_rows']($request) < 1)
{
loadLanguage('Errors');
log_error($txt['database_error'] . ':' . sprintf($txt['db_table_creation'], $table_name), $database, null, null);
$smcFunc['db_free_result']($request);
return false;
}
$smcFunc['db_free_result']($request);
return true;
Then add to Errors.english.php
$txt['db_table_creation'] = 'Unable to create %1s table in database';
I don't think that's a neat solution. In my opinion, it should return the real error occurring. Here is the actual problem:
Subs-Db-mysql.php
if (empty($db_unbuffered))
$ret = @mysql_query($db_string, $connection);
else
$ret = @mysql_unbuffered_query($db_string, $connection);
if ($ret === false && empty($db_values['db_error_skip']))
$ret = smf_db_error($db_string, $connection);
It won't return an error for $smcFunc['db_query'] if $db_values['db_error_skip'] is not empty. When $db_values is set as the string 'security_override' (which is how it is done for most of the package functions) $db_values['db_error_skip'] will be equal to 's' and it won't return an error.
My suggested fixes: (just for MySQL as an example)
Sources/Subs-Db-mysql.php
Quick and easy way:
if ($ret === false && empty($db_values['db_error_skip']))
if ($ret === false && (empty($db_values['db_error_skip']) || $db_values === 'security_override'))
Maybe a cleaner way:
// Overriding security? This is evil!
$security_override = $db_values === 'security_override' || !empty($db_values['security_override']);
// It is better to have db values as an array here.
$db_values = $db_values === 'security_override' ? array('security_override' => true) : $db_values;
// Overriding security? This is evil!
$security_override = !empty($db_values['security_override']);
As i pointe out to sleepy,
We need to find out why security_override is used when creating the table.
For alot of things it has been used INTENTIONALLY.
Well, I don't have a problem with security_override parameter. I just say that, regardless of the value of security_override parameter, it should return an error if there are any.
I assumed that it was done that way on purpose to prevent SMF from trying to throw security measures at something we hand worked out ourselves.
In any case then. We just add => true, after 'security_override' and this should still work.
Quote from: SleePy on May 04, 2009, 12:22:35 PM
I assumed that it was done that way on purpose to prevent SMF from trying to throw security measures at something we hand worked out ourselves.
I'm not sure if I got what you mean correctly, but SMF doesn't show the actual error to user, unless s/he has the 'admin_forum' permission.
Quote from: SleePy on May 04, 2009, 12:22:35 PM
In any case then. We just add => true, after 'security_override' and this should still work.
Yeah, with the
array() it would work. However, there are quite a lot of queries using it as a string. Also, if that is to make only the desired queries to display errors, I don't think that's a clean way either.
Fixed with Revision 9883.