WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 5 years ago

#19467 closed defect (bug) (fixed)

wpdb does not always set rows_affected or insert_id properties when needed

Reported by: GeertDD Owned by: nacin
Milestone: 3.5 Priority: normal
Severity: minor Version: 3.3
Component: Database Keywords: has-patch commit
Focuses: Cc:

Description

If SQL keywords like INSERT or UPDATE are not followed by a space, the rows_affected and insert_id properties of the wpdb class are not updated. Those keywords might be followed by a newline or a tab character. A simple regex tweak fixes this issue.

<?php

// Note the newline after "UPDATE"
$wpdb->query('
        UPDATE
        test_table
        SET foo = "bar"
        WHERE id = 5
');

// Should not be int(0) in this case
var_dump($wpdb->rows_affected);

Attachments (1)

wp-db.diff (891 bytes) - added by GeertDD 6 years ago.

Download all attachments as: .zip

Change History (10)

@GeertDD
6 years ago

#1 @scribu
6 years ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to Future Release

#2 @bpetty
5 years ago

  • Cc bpetty added

I am still able to reproduce this issue in SVN trunk. By that, I mean wpdb->query() incorrectly identifies perfectly valid SQL, resulting in missing rows_affected and insert_id values for SQL statements when they contain white space other than a space after the first query keyword.

I have tested both with and without this patch using one of the existing SQL queries in WP (the attachment post_parent UPDATE) by adding a newline immediately after "UPDATE". This patch has a 4 lines offset, but still applies cleanly, and it does address the issue correctly, and fixes this.

#3 @scribu
5 years ago

  • Milestone changed from Future Release to 3.5
  • Severity changed from trivial to minor

Wouldn't it be even better if we allowed multiple spaces, like \s+ ?

#4 @bpetty
5 years ago

It isn't an anchored expression, so it does ;)

#5 @scribu
5 years ago

Oh, you're right; nevermind.

#6 @Bellfalasch
5 years ago

  • Cc Bellfalasch added

I'm not that all familiar with WP, but I am with MySQL. So sorry if I'm all off here =/

An UPDATE which has an valid target with it's WHERE-statement, but doesn't change any data in a table will return rows_affected = 0. An error would produce rows_affected = -1.

If we have the table test with columns id and awesome set to 1 and 'Hi!' respectivly.

First we do this:

UPDATE test SET awesome = 'Bye!' WHERE id = 1

rows_affected = 1

Right after that we run the same SQL and get 0 affected rows since no changes was made in the db.

UPDATE test SET awesome = 'Bye!' WHERE id = 1

rows_affected = 0

#7 @scribu
5 years ago

@Bellfalasch: Yes, that's how it should work, but it doesn't if the query looks like this:

UPDATE
test SET awesome = 'Bye!' WHERE id = 1

as noted in the ticket description.

#8 @nacin
5 years ago

Looks good.

#9 @nacin
5 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [21178]:

Correctly identify queries where a line break follows a keyword, rather than a space. props GeertDD, fixes #19467.

Note: See TracTickets for help on using tickets.