Posted by deekayen on March 2, 2009 at 6:55am
| Project: | Drupal tweaks |
| Version: | 6.x-1.x-dev |
| Component: | db_tweaks module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Needs tests |
Issue Summary
After reading Larry's post on STRICT sql mode in MySQL, I figured I ought to be maintaining my 6.x modules with STRICT compatibility so I don't have the problem suddenly in 7.x upgrades. I wrote a module to help switch SQL modes without having to mess with my my.cnf file. It supports switching to STRICT for the session as well as any combination of the other available sql modes.
http://cvs.drupal.org/viewvc.py/drupal/contributions/sandbox/deekayen/mo...
There's a 5.x version as the first commit. Just wondered if there was interest in putting this kind of thing in the devel project.
Comments
#1
Looks good to me. The default for 7 should be strict. And admin should be able to select 'server default' which does nothing in hook _boot(). default for d5 and d6 ought to be server default IMO.
#2
I guess I don't understand the CNW status. I re-read and interpreted it several ways:
As it is, there is no attempt to grab the current server default because the MySQL docs say
If there are no checkboxes selected, the module defaults to the documented default.
#3
Note to self:
Grabbing the current sql_mode is
SELECT @@SESSION.sql_mode;which could act as a "server default" option.#4
Well, there was no patch in the OP despite being marked CNR. I don't think this merits a new module in devel so this should become a patch. Downgrading to Active Needs Info for now.
1. in your list is correct.
If the admin has chosen server default, we should not ever SET a SQL mode.
We also need to not interfere with postgres and sqlite
#5
Here's a patch against D5. If you like it, I'll make some upgrade versions. It differs from my sandbox version by adding a server default checkbox and switches from a checkbox list to a multiselect. It also checks $db_url for MySQL.
#6
Perhaps a 6.x patch is more desirable?
#7
Same thing as #5 and #6, but for D7.
#8
Here's a screenshot of the new SQL Mode fieldset on the Devel settings page. It's in the middle. The environment is Firefox in Xfce 4.6 on Drupal HEAD/7 for whatever I downloaded today.
#9
I'd love it if there was a link to the MySQL docs explaining the differences between them. Perhaps http://dev.mysql.com/doc/refman/5.0/en/server-sql-mode.html ?
#10
#5, #6, and #7 already have that URL as a link in the new fieldset's description.
#11
you are correct. looking at the screen shot i would not have concluded the link was information on the modes.
#12
I hate to elevate this, but the new http://drupal.org/project/db_tweaks is doing things where this patch might fit. Should I take this patch over there?
#13
yes, i think so.
#14
moving to Database tweaks module
#15
Functionality committed into db_tweaks
http://drupal.org/cvs?commit=212924
#16
Is there a particular reason you moved it into hook_init() instead of _boot?
#17
No special reason, the difference is that _init() is always executed (apart of maintenance mode) and you have t() accessible,
to execute hook_boot() you have to register bootstrap to 1 somehow, but I'm not sure what's the proper way for that with Drupal API.
You can do that on install hook:
db_query("UPDATE {system} SET bootstrap = '%d' WHERE name = 'db_tweaks'", 1);But probably during module_rebuild it's reseting to default again, so you need to execute this query again during _notify()
Because I've got a lot of problems with hook_boot in other module.
You know the simple way for hook_boot usage?
And it's better idea to move it to hook_boot?
#18
Fixed.
#477114: Unconfigured site throws user warnings
If there are any other issues, please open new one.
#19
hook_init() doesn't load on every page request and t() isn't needed for switching sql mode.
You could register it people already using the module through an update function, but just re-saving the modules page will fire module_rebuild_cache(), which is responsible for re-setting the bootstrap to 1 where hook_boot and hook_exit are implemented. Merely activating the module will have the bootstrap set to 1 by the mere existence of db_tweaks_boot().
If you want to do the update function, there's no point in doing string replacement on a constant in db_query(), so you can use update_sql() and provide feedback to the update script. Otherwise, the update instructions would be to just re-save the modules page. I have no idea what you're talking about with _notify().
<?php/**
* Move sql mode switching to hook_boot(), so register with bootstrap.
*/
function db_tweaks_update_6000() {
$ret = array();
$ret[] = update_sql("UPDATE {system} SET bootstrap = 1 WHERE name = 'db_tweaks'");
return $ret;
}
?>
#20
Moved to hook_boot()
http://drupal.org/cvs?commit=229598
Added weight priority in hook_install and changed max_packet_size from kB to MB, so probably module have to be re-installed.
#21
Supposing that after 1 week somebody test it already.
#22
I have not tested it since I made the patch for devel. I'll start you a .test file.
#23
This should get you started. The problem with saying this has been tested is that there's no feedback route to see if a MySQL setting actually stuck during bootstrap.
Perhaps adding hook_requirements() could be a way to get that feedback on REQUIREMENT_INFO level. One row for each setting perhaps? Then this test could load the core status page to assert for the strings it expects.
<?php**
* Implementation of hook_requirements().
*/
function db_tweaks_requirements($phase) {
$requirements = array();
$t = get_t();
if ($phase == 'runtime') {
$modes = db_result(db_query('SELECT @@SESSION.sql_mode'));
$requirements['google_pr_rank'] = array(
'title' => $t('SQL mode'),
'value' => $modes,
'severity' => REQUIREMENT_INFO
);
}
return $requirements;
}
?>
Then the requirements and test can be expanded to all the other variables based on the template of requirements and .test file.
#24
Thanks a lot.
I'll test this test, probably on Monday.
#25
While testing this feature, I selected POSTGRESQL on the settings page. When I submitted, it redirected to a blank page with just a forward slash / on it. It still saved the setting, but ending on a blank page wasn't productive.
#26
I've done hook_requirements and updated test plan.
http://drupal.org/cvs?commit=235296
http://drupal.org/cvs?commit=235320
http://drupal.org/cvs?commit=235334
http://drupal.org/cvs?commit=235338
Only one case is failing when checking mysql_max_allowed_packet from settings page, but I've no idea why.
I've set mysql_max_allowed_packet to 3 and it was saved without problems.
Without simpletest normally it's the same value, with simpletest environment it's 1 on settings page.
#27
Merged project with: http://drupal.org/project/drupal_tweaks
#28
Feature is done.
If you have any problem, please raise separate issue with steps how to reproduce it.
#29
Automatically closed -- issue fixed for 2 weeks with no activity.