insertion errors

Subscribing

Can you try going to the Search settings page and clicking on the link that rebuilds the search index, and then running cron from the Status Report page until Search reports that everything is indexed?

That's very interesting. I'm looking at those lines in the search module code:

    db_query("UPDATE {search_total} SET count = %f WHERE word = '%s'", $total, $word);
    if (!db_affected_rows()) {
      db_query("INSERT INTO {search_total} (word, count) VALUES ('%s', %f)", $word, $total);
    }

In other words, it is first doing an UPDATE query to try to update an existing row in the search total table. And then if the UPDATE says "No rows were affected", it's assuming that the word was not in the database table, and trying to use an INSERT query instead.

So it looks like the db_affected_rows() function is not working correctly for you: it should be reporting that it updated that row, and instead it is reporting that it updated nothing.

Very interesting....

So we need to investigate why db_affected_rows() isn't doing the right thing, but in the meantime I think you can probably ignore those errors, because the UPDATE probably worked.

Hah! I think I have possibly found the answer. From the documentation of the PHP function mysql_affected_rows():

When using UPDATE, MySQL will not update columns where the new value is the same as the old value. This creates the possibility that mysql_affected_rows() may not actually equal the number of rows matched, only the number of rows that were literally affected by the query.

So possibly this is the issue. If the index is being rebuilt and the count is the same as it was before, it's possible that the affected row count will be zero.

Looking at the code base for Drupal 6, I see that in most cases, when db_affected_rows() is used to figure out if an update took place, the next INSERT statement is preceded by @ to suppress the error that could result when the insert figures out the row actually did exist. This was not done in two places: the search module being one of them. Here's a patch to remedy that.

Example of another place it's done this way in Drupal 6:

function variable_set($name, $value) {
  global $conf;

  $serialized_value = serialize($value);
  db_query("UPDATE {variable} SET value = '%s' WHERE name = '%s'", $serialized_value, $name);
  if (!db_affected_rows()) {
    @db_query("INSERT INTO {variable} (name, value) VALUES ('%s', '%s')", $name, $serialized_value);
  }

  cache_clear_all('variables', 'cache');

  $conf[$name] = $value;
}

Huh, I didn't know we had testing now for D6 patches. This patch applies fine for me. I'll try changing the version.

#6: 638702.patch queued for re-testing.

I just filed a related issue about what the return value should be for D7 update queries (because the same problem exists there):
#718540: Better documentation in query.inc

Ehm,
so the solution to this is to supress the warning??? Then we could just skip the whole if clause in general. Other option would be to do something like

if (!db_fetch_object(db_query("SELECT count FROM {search_total} WHERE word = '%s'", $word))) {

(could probably be done in a nicer way)
to replace

if (!db_affected_rows()) {

PeterP - Right, it bothers me too, but it's definitely the pattern elsewhere in Drupal 6 core. What you're suggesting would be better, but less efficient, as it would require another query.

Subscribing..

I just started getting the same error as the op since upgrading to Ubuntu 10.04 lucid with mysql Ver 14.14 Distrib 5.1.41.

The patch at #6 applied fine to Pressflow 6.16.77 and stopped the error messages.

Thanks.

This makes sense, even if I cannot explain why this shows up so late in the game.

I too am getting this since my lucid upgrade, but only with mysqli. mysql seems to work as before. Perhaps the flags passed to mysqli_real_connect() are not respected anymore?

I'm pretty sure this problem is caused by a bug in the db_affected_rows() function.

I've reported the bug and post a suggested fix (that works) here.

http://drupal.org/node/805858

I don't have "commit" access to fix the CVS version.

This bug seems to be the cause of a number of errors in the issue queue ?

Incidently, suppressing the warning is not a good solution at all (sorry)!

#798626: Cron produces duplicate entry error in search.module just marked as a duplicate of this issue.

@PeterP: that would run at least two queries: a SELECT and an INSERT/UPDATE, while our current code only runs one UPDATE query if possible or an INSERT if the update failed.

@simg: the db_affected_rows() function works as named and documented. It returns 0 when the row was updated to be the same and we pay a little performance penalty in that case, however your suggested patch in the linked issue makes us pay a penalty at all invocations of the function.

Committed, thank you.

Gábor Hojtsy,

Note that you are making improper use of the db_affacted_rows() function:

...
if (!db_affected_rows()) {
...

As you can see here: http://api.drupal.org/api/function/db_affected_rows/6 the function may return -1 which means TRUE in your situation. You ought to use > 0 instead.

...
if (db_affected_rows() > 0) {
...

Or have the Core people change the code and check for the return value, if -1, return 0.

Thank you.
Alexis Wilke

@AlexisWilke: this seems like both a documentation omission and a code problem if that is the case. Where did you open an issue for these?