db_index_exists() missing, module updates cannot handle indexes properly

Ran across the need for this today in #363262: Missing index on url_alias table.

1. Needs to be fixed in D7 first.
2. We already have db_column_exists() and db_table_exists(), so there's no reason to prefix this function with an underscore.
3. The function is missing PHPDoc.

webchick asked for my input here. I've no problem with an indexExists() method on the schema API, with a procedural wrapper. It should follow the same logical design as the rest of schema, of course, but I like having a rich set of primitives. :-)

You can go ahead. :-) A D7 implementation, however, must put the actual logic back in the schema class, and then provide a DB-specific implementation in each DB's schema class. Then add a wrapper function that mirrors the other functions in database.inc.

I fail to see how this is database specific. Isn't MySQL also returning some kind of error when you try to DROP a non existing index?

Plus, I'm -1 on this approach. Every inconsistency should throw a warning during the update process. It's the job of the administrator to choose to ignore them or not.

db_result() is deprecated. Use ->fetchField() instead.

Seems like this should add to the database tests. Just checking for an existing and non-existing index ought to work no?

The code looks good to me, but as catch suggests, it needs tests.

Bug in latest patch: $table needs to be escaped somehow when enclosed between {}. Otherwise table prefix substitution will fail.

Fix:

-  return db_query("SHOW INDEX FROM {$table} WHERE key_name = '$name'")->fetchField();
+  return db_query("SHOW INDEX FROM {" . $table . "} WHERE key_name = '$name'")->fetchField();

subscribing and promoting to PostgreSQL Sprint

I implemented this method to check for existence of indexes in CCK, which is available in CCK 2.5. Reference: #231453: Allow indexing columns.

But it has been reported (see #562260: content_db_index_exists has wrong syntax for postgres) that the following query doesn't work in PostgreSQL.

db_query("SELECT COUNT(indexname) FROM pg_indexes WHERE indexname = '$name'")

That it should look like this:

db_query("SELECT COUNT(indexname) FROM pg_indexes WHERE indexname = '" . $table . '_' . $name . "_idx'")

Is it possible that the contents of the indexname column varies depending on PostgreSQL versions? I've been unable to find information about how PostgreSQL stores data in this column, so I cannot really tell. :(

I hope the experience collected by the use of such a feature in CCK could potentially help provide a more reliable method for D7.

Does anyone knows more about the format of the indexname column in pg_indexes table throughout the different PostgreSQL versions supported by Drupal itself?

LOL... it was sooo much easy. The fact is that the Schema API for PostgreSQL is building the index names like that, so the statement to check if an index exists should be built using the same pattern for index names as the one used when creating the index. The following syntax is then correct:

db_query("SELECT COUNT(indexname) FROM pg_indexes WHERE indexname = '" . $table . '_' . $name . "_idx'")

And the one implemented in the patch in #17 is not, which is like this:

db_query("SELECT COUNT(indexname) FROM pg_indexes WHERE indexname = '$name'")

For references, see methods dropIndex() and _createIndexSql() in class DatabaseSchema_pgsql, implemented in file includes/database/pgsql/schema.inc

I hope that helps here. ;-)

I think this will be easily done with #302327: Support cross-schema/database prefixing like we claim to in. So lets wait for that. I'm sure this will go in after the freeze too.

Absolutely required. Also for the module update numbering scheme we teach in http://api.drupal.org/api/function/hook_update_N/7

If that helps, here's the code that's using CCK for a few months already:

/**
 * Checks if an index exists.
 *
 * @todo: May we remove this funcion when implemented by Drupal core itself?
 * @link http ://drupal.org/node/360854
 * @link http ://dev.mysql.com/doc/refman/5.0/en/extended-show.html
 *
 * @param $table
 *   Name of the table.
 * @param $name
 *   Name of the index.
 * @return
 *   TRUE if the table exists. Otherwise FALSE.
 */
function content_db_index_exists($table, $name) {
  global $db_type;
  if ($db_type == 'mysql' || $db_type == 'mysqli') {
    if (version_compare(db_version(), '5.0.3') < 0) {
      // Earlier versions of MySQL don't support a WHERE clause for SHOW.
      $result = db_query('SHOW INDEX FROM {'. $table .'}');
      while ($row = db_fetch_array($result)) {
        if ($row['Key_name'] == $name) {
          return TRUE;
        }
      }
      return FALSE;
    }
    return (bool)db_result(db_query("SHOW INDEX FROM {". $table ."} WHERE key_name = '$name'"));
  }
  elseif ($db_type == 'pgsql') {
    // Note that the index names in Schema API for PostgreSQL are prefixed by
    // the table name and suffixed by '_idx'.
    return (bool)db_result(db_query("SELECT COUNT(indexname) FROM pg_indexes WHERE indexname = '{". $table ."}_{$name}_idx'"));
  }
  return FALSE;
}

use schema api instead of querying db directly

Chasing HEAD from #17, tidied the code to use the DB API properly (internal calls should always use $connection->query(), never db_query()), and adds unit tests.

+++ modules/simpletest/tests/schema.test	2009-12-14 02:47:45 +0000
@@ -1,4 +1,4 @@
-<?php
+g<?php

Looks wrong. I'll correct that manually before committing. Interestingly, all tests pass.

Fixed, and committed to CVS HEAD. Thanks!

Weird, but thanks. :-)