DB Tuner: D7 version?

Obviously I'm for this. Issue is I don't have any d7 sites...

I'm thinking a patch should be fairly straight forward. Some parts might work out of the box with this tool http://upgrade.boombatower.com/ Some of the heavy database operations like collations might not convert over correctly. I would think the views code would convert over though.

subscribe

sub!

Subscribe

Subscribe

Following.

Make module name visible in Dashboard.

+1

subscribes

Hi All!

I've started to migrate this module to D7, but have a problem.

/**
 * Form builder: engine configuration page submit.
 */
function dbtuner_engine_configuration_submit($form, &$form_state) {
  $engine = $form_state['values']['db_engine'];
  $tables = db_query('SHOW TABLE STATUS');
  $counter = 0;
  foreach ($tables as $table) {
    if ($table->engine != $engine) {
      db_query('ALTER TABLE {'. $table->name .'} ENGINE = :engine', array(':engine' => $engine));
      $counter++;
    }
  }
  drupal_set_message(t('@count tables changed to @name', array('@count' => $counter, '@name' => $engine)));
}

This function works, but I'm worried about the query: I guess that SQL injection can't be implemented with $table->name variable, but I'm not sure... I can't use marker ":name" there because then Drupal add quotes to table name.

Could somebody help me with this question? Is it safe to have such SQL query in the code?

Unless you have a table in your DB that is named to be an mysql exploit you should be safe. In other words it's close to impossible.

U try out http://upgrade.boombatower.com/ ?

Hi Mike,

I agree that this is almost impossible, but it could be better to know that it's not only my opinion.
I haven't tried the http://upgrade.boombatower.com yet (I migrate modules based on API manual). Thanks for the link! I will check it soon. Especially that I want to migrate the http://drupal.org/project/xml_parser too.

subscribe

subscribe

subscribe

subscribe

subscribe

Subscribing...

Subscribe. This would be great to have for D7, since using B&M caused me having mixed latin1_swedish_ci and utf8_general_ci tables and MyISAM/InnoDB on several sites.

Did anyone continue work on this?

Subscribe

subscribing

sub

tail -f

sub

subscribe

You can now use the 'Follow' button on the top right of the page instead of commenting.

I don't really code, but I thought running it through upgrade.boombatower.com coder upgrade might get things rolling.

Here's the coding standards patch I ran first, as recommended (against 6.x-1.x-dev).

And here are the unmodified upgrade files (against the coding-standards adapted module):

• The coder upgrade patch
• The coder upgrade review
• The coder upgraded module in a tar.gz

This is the current status of the port:

• Installing the module works, without errors.
• Visiting admin/config/dbtuner just prints Array
• Visiting admin/config/dbtuner/explain-views: Views Not Installed
• Visiting admin/config/dbtuner/engine throws a fatal error: Fatal error: Call to undefined function db_fetch_array() in sites/all/modules/dbtuner/dbtuner.admin.inc on line 179
• Visiting admin/config/dbtuner/collation gives: Fatal error: Call to undefined function db_fetch_array() in sites/all/modules/dbtuner/dbtuner.admin.inc on line 52
• Visiting admin/config/dbtuner/explain-views gives: Fatal error: Call to undefined function db_fetch_array() in sites/all/modules/dbtuner/dbtuner.mysqltuner.inc on line 35
• admin/config/dbtuner/explain-slow-log: Not Implemented

Anyone? I hoped this would get things started!

sub

it looks like db_fetch_array has been replaced by db_query in D7

http://api.drupal.org/api/drupal/includes%21database%21database.inc/func...

Subscribing

interested!

Looks like we can take @ArjanLikesDrupal's patch replace db_fetch_array with db_query & maybe run out a dev version.

That sound right? Who wants to roll up the patch against the latest git repo?

Taken from ArjanLikesDrupal's dbtuner-7.x-1.x-dev.tar.gz. I fix all db query and render problem. Haven't tried to change engine and collocation as I did the fix on a live site :P

If anyone can test and post me any error, I'll try to fix it.

I notice that when I try to run optimize or repair commands on my database it does not work on innodb.

what will?

If I remember correctly, InnoDB doesn't support repair

OPTIMIZE still works for InnoDB, just in a different way - see http://stackoverflow.com/questions/2816044/optimize-innodb-table

For REPAIR related issues (if you think you need it) check also:

http://stackoverflow.com/questions/226172/how-do-i-repair-an-innodb-table
http://www.mysqlperformanceblog.com/2008/07/04/recovering-innodb-table-c...

Here is sg to include for innodb :) #817398: Provide a requirements warning when innodb_flush_log_at_trx_commit is set

#35: You should replace db_prefix_tables with Database::getConnection()->prefixTables in dbtuner.explain.inc line 131 so that you don't get a fatal error :)

@sylv3st3r why didn't you just extend the patch? Usually easier that way to gain adoption.

Well.... I don't know ho to extend the patch :P
And I didn't see the author extending it into D7 yet. At least mine is working to some extend.

Thanks,
-r

It's pretty simple.

1) Pull down the latest code from git for DB Tuner.
2) Copy your files (uploaded into the zip) on-top of it.
3) Follow instructions here http://drupal.org/node/707484

Then it's easy to see what you've changed what you haven't.

Then the community can be involved in seeing it work better.

So guys, it's a pain to see uploaded archives here so I created a patch based on the archive, try it and have fun ;)

Can you please provide steps

How to get the code from git for DB Tuner..?

Thanks
Bhanu

git clone --branch 6.x-1.x http://git.drupal.org/project/dbtuner.git
cd dbtuner
wget https://drupal.org/files/dbtuner-port_to_d7-1055716-44.patch
git apply dbtuner-port_to_d7-1055716-44.patch

To clean that git repository up (because you don't need it) you can execute following after the steps above:

rm dbtuner-port_to_d7-1055716-44.patch
rm -rf .git/

Below is the error when I am applying the patch

git.exe am --3way --ignore-space-change --keep-cr "D:\Meredith\dbtuner-port_to_d7-1055716-44.patch"
Patch format detection failed.

Try it like I said...

I tried applying this patch to:
git clone --branch master http://git.drupal.org/project/dbtuner.git

And it failed. Then tried manually applying the patch (not through git) - and it also failed.

What version of the code did you build the patch against?

Updated the patch. Please use the 6.x-1.x branch instead of the master branch.

waiting for a d7 version

Patch applies nicely. But going to /admin/modules I get this error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'sites/all/modules/dbtuner/dbtuner.module' for key 'PRIMARY': INSERT INTO {system} (filename, name, type, owner, info) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4); Array ( [:db_insert_placeholder_0] => sites/all/modules/dbtuner/dbtuner.module [:db_insert_placeholder_1] => dbtuner [:db_insert_placeholder_2] => module [:db_insert_placeholder_3] => [:db_insert_placeholder_4] => a:10:{s:4:"name";s:8:"DB Tuner";s:11:"description";s:44:"Adds indexes to fields used in views & more.";s:4:"core";s:3:"7.x";s:7:"package";s:27:"Performance and scalability";s:10:"recommends";a:1:{i:0;s:5:"views";}s:12:"dependencies";a:0:{}s:7:"version";N;s:3:"php";s:5:"5.2.4";s:5:"files";a:0:{}s:9:"bootstrap";i:0;} ) in system_update_files_database() (line 2307 of /DRUPAL7/modules/system/system.module).

I know comment #10 was a long time ago but if that is still an issue use db_escape_table().

I just followed the steps described in #46 and applied the patch from #50. There were no errors during installation, but what I'm seeing on the configuration pages looks a little strange. The Indexes page doesn't list any indexes (see attached image), and the Explain Views page is completely blank. The Collation page produces "The website encountered an unexpected error. Please try again later." Here's the error:

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'mydb.drupal_drupal_access' doesn't exist: SHOW full fields FROM {drupal_access}; Array ( ) in dbtuner_collation_configuration() (line 66 of /mypath/public_html/sites/all/modules/dbtuner/dbtuner.admin.inc).

The name of the table in my database is drupal_access, not drupal_drupal_access. Thoughts, anyone?

Patch does not follow the coding standards.

For reference: Coding standards

The name of the table in my database is drupal_access, not drupal_drupal_access. Thoughts, anyone?

This is line 66 of dbtuner.admin.inc:

$result = db_query("SHOW full fields FROM {".$table."}");

The tables in my database are prefixed with "drupal_". The { and } braces cause the prefix to be appended to the table name, which turns "drupal_access" into "drupal_drupal_access". It seems like there's an error in the logic here: either this query shouldn't append the prefix, or the query that produced the tables names shouldn't have retrieved the prefixed names. Which should be modified to make this work?

I haven't actually used the patch but from a look at the patch the code does a SHOW TABLES query and then passes the results of that into other sql queries.

In this case you should not be using {} around the table name.

This is because the purpose of the {} is for drupal to apply the database table prefix if there is one, however when you do SHOW TABLES it returns the name of the table including the prefix.

That is why you are getting double prefix.

the Explain Views page is completely blank

I see what's causing this, but I'm not familiar enough with the Drupal 7 database API to know how to fix it. Function dbtuner_db_query in dbtuner.explain.inc calls db_prefix_tables() and _db_query_callback() and both have been deprecated in Drupal 7.

function dbtuner_db_query($query) {
  $args = func_get_args();
  array_shift($args);
  $query = db_prefix_tables($query);
  if (isset($args[0]) and is_array($args[0])) { // 'All arguments in one array' syntax
    $args = $args[0];
  }
  _db_query_callback($args, TRUE);
  $query = preg_replace_callback(DB_QUERY_REGEXP, '_db_query_callback', $query);
  return $query;
}

Is this function even needed in Drupal 7?

@mgifford #52: looks like some of dbtuner's tables were already present while installing? I did not have this problem on a fresh Drupal 7.23 installation.

I tried changing database engine and collation using patch from #50, both still had errors.

I also wanted to get an overview of the earlier changes, some interdiffs of the earlier patches are attached.

Changing database engine and collation both work for me using the following updated patch (tested to/from MyISAM/InnoDB and to/from utf8_general_ci/ utf8_unicode_ci.

I'll have to look into the coding standards some more to see how to deal with those long sql statements etc., but I'll just post this first since it works for me.

@ar-jan: have you tried to look at the "Explain Views" functionality?

I just took a look but I'm not sure. It seems that _db_query_callback() only takes cares of placeholder substitutions, and this should be handled by the database layer in D7 (?). I don't see why db_prefix_tables() is used in dbtuner_db_query() (which is only called in line 99 to save the query for display, I think), but isn't used in line 104 for the actual EXPLAIN query

$explain = db_query('EXPLAIN ' . $query, $query_args);

I tried removing the whole dbtuner_db_query() function and changing line 104 to

$explain = db_query("EXPLAIN $query", array($query_args));

But this gives PDOException: SQLSTATE[HY093]: Invalid parameter number: no parameters were bound: EXPLAIN SELECT node.title AS node_title, node.nid AS nid, users_node__authmap.authname AS users_node__authmap_authname, users_node.mail AS users_node_mail, users_node.uid AS users_node_uid, comment_node.cid AS comment_node_cid, comment_node.nid AS comment_node_nid, node.created AS node_created FROM {node} node INNER JOIN {users} users_node ON node.uid = users_node.uid LEFT JOIN {comment} comment_node ON node.nid = comment_node.nid LEFT JOIN {authmap} users_node__authmap ON users_node.uid = users_node__authmap.uid WHERE (( (node.status = :db_condition_placeholder_0) )) ORDER BY node_created DESC /* test - default*/; Array ( ) in dbtuner_explain_get_views() (line 104 of modules/dbtuner/dbtuner.explain.inc).

Is this because of the placeholders? Anyone know how this explain query should be run?

Is this because of the placeholders?

I think so and that's exactly what I'm seeing. $query_args and $view->build_info['query_args'] are both arrays with 0 elements. Might this have something to do with a D6 to D7 change in the Views API?

Is the current patch stable enough to use in a test environment or is it still being worked on? I can't wait to try this out! Thanks for everyone's effort on this.