Make WordPress Core

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#18948 closed enhancement (fixed)

Introduce $wpdb->delete()

Reported by: scribu's profile scribu Owned by: nacin's profile nacin
Milestone: 3.4 Priority: normal
Severity: normal Version:
Component: Database Keywords: has-patch early
Focuses: Cc:

Description

We already have update() and insert() methods, which are awesome, but no delete() method.

It seems like a no-brainer to me.

Attachments (6)

18948.diff (2.4 KB) - added by justindgivens 12 years ago.
my suggestion
18948.2.diff (14.7 KB) - added by justindgivens 12 years ago.
Updated wpdb->delete and the files I've updated the DELETE queries.
18948.3.diff (14.2 KB) - added by justindgivens 12 years ago.
updated from scribu notes.
18948.4.diff (14.1 KB) - added by justindgivens 12 years ago.
18948.5.diff (13.0 KB) - added by scribu 12 years ago.
18948.6.diff (14.1 KB) - added by scribu 12 years ago.

Download all attachments as: .zip

Change History (33)

#1 @scribu
12 years ago

  • Component changed from General to Database

@justindgivens
12 years ago

my suggestion

#2 @justindgivens
12 years ago

  • Keywords has-patch added; needs-patch removed

#3 @scribu
12 years ago

Yep, that's what I had in mind. The bigger task will be using it throughout the WP codebase.

#4 @scribu
12 years ago

  • Keywords early added

#5 @DrewAPicture
12 years ago

It seems like a no-brainer to me.

+1.

#6 follow-up: @justindgivens
12 years ago

Been going through all the code that contains sql DELETE queries and I ran across the below query. 18948.diff won't handle this type of delete.

/wp-admin/includes/upgrade.php#L295

"DELETE FROM $wpdb->usermeta WHERE user_id != %d AND meta_key = %s"

Any suggestions?

#7 @F J Kaiser
12 years ago

+10 Adding it as suggested in the patch wouldn't harm anyone. Porting it to different places in core can be done later with another (some other) tickets.

@justindgivens
12 years ago

Updated wpdb->delete and the files I've updated the DELETE queries.

#8 in reply to: ↑ 6 ; follow-up: @scribu
12 years ago

Replying to justindgivens:

Been going through all the code that contains sql DELETE queries and I ran across the below query. 18948.diff won't handle this type of delete.

Just skip it.

#9 in reply to: ↑ 8 @justindgivens
12 years ago

Replying to scribu:

Just skip it.

That's what I did. Going to look one more time through the core but I think I got most of the files updates.

#10 @scribu
12 years ago

$result = $wpdb->delete( "$wpdb->options" , array( "option_name" => $option) , array( '%s' ) );

should be

$result = $wpdb->delete( "$wpdb->options" , array( "option_name" => $option ) , array( '%s' ) );

function delete( $table, $where, $where_format = null, $limit = null) {

should be

function delete( $table, $where, $where_format = null, $limit = null ) { 

Also, this won't work:

$wpdb->delete( "$wpdb->options" , array( 'option_name' => "REGEXP '^rss_[0-9a-f]{32}(_ts)?$'" ) , array( '%s' ) );

REGEXP is a different operator, not a value.


$wpdb->delete( "$wpdb->usermeta" , array( "user_id" => $id , "meta_key" => "'{$level_key}'" ) , array( '%d' , '%s' ) );

can be

$wpdb->delete( "$wpdb->usermeta" , array( "user_id" => $id , "meta_key" => $level_key ) , array( '%d' , '%s' ) );

@justindgivens
12 years ago

updated from scribu notes.

#11 @justindgivens
12 years ago

Made the changes in 18948.3.diff.

#12 follow-up: @scribu
12 years ago

The quotes around the table name are not necessary:

$wpdb->delete( "$wpdb->usermeta",

should be

$wpdb->delete( $wpdb->usermeta,

This goes for all queries.

Also, this line is really hard to follow:

$form = ( $form = array_shift( $where_formats ) ) ? $form : $where_format[0];

Please split it up into an assignment and a normal if.

#13 in reply to: ↑ 12 @justindgivens
12 years ago

Replying to scribu:

Also, this line is really hard to follow:

$form = ( $form = array_shift( $where_formats ) ) ? $form : $where_format[0];

Please split it up into an assignment and a normal if.

How is that any different from the update() and _insert_replace_helper() functions?? wp-db.php

#14 @scribu
12 years ago

Just because existing code is bad doesn't mean new code also has to be bad.

Last edited 12 years ago by scribu (previous) (diff)

#15 @scribu
12 years ago

Also, maybe this can be extracted into a helper method:

protected function get_form( &$where_formats, $where_format ) {

#16 @scribu
12 years ago

Although I think this will probably be handled better in a separate ticket. Let's just take care of the unnecessary quotes around table names.

#17 @justindgivens
12 years ago

18948.4.diff Updated with the quotes removed.

#18 @scribu
12 years ago

Related: #19019

#19 @GaryJ
12 years ago

Might need to be another ticket but the addition in this patch of:

@see wpdb:delete()

should be

@see wpdb::delete()

(the existing @see tags above it also wrong).

Several bits of whitespace inside ( ) and around ! also needed within the new method to follow code standards.

@scribu
12 years ago

#20 @scribu
12 years ago

  • Milestone changed from Future Release to 3.4

18948.5.diff:

  • refreshed against trunk
  • fixed fatal error (!) in comment.php
  • leave deprecated.php alone
  • cleaned up wpdb::delete()

#21 @nacin
12 years ago

There should be no need to pass a $where_format for core fields, as these field types are already initialized in wp_set_wpdb_vars().

#22 @scribu
12 years ago

Was wondering about that. So, that means no queries made by Core ever need to pass the third parameter.

Will update the patch shortly.

@scribu
12 years ago

#23 @scribu
12 years ago

18948.6.diff removes the redundant format parameters and replaces " with ' where appropriate.

#24 @nacin
12 years ago

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

In [20287]:

Introduce $wpdb->delete(). props justindgivens, scribu. fixes #18948.

#25 @GaryJ
12 years ago

[20287] appears to show the @since for the new method as 2.5.0, rather that 3.4.0 ?

#26 @scribu
12 years ago

Yeah, also this usage example in the docblock should be removed:

wpdb::delete( 'table', array( 'ID' => 1 ), array( '%d' ), 1 )
Last edited 12 years ago by scribu (previous) (diff)

#27 @duck_
12 years ago

In [20292]:

Correct @since and remove invalid code example in wpdb::delete() documentation. Props GaryJ, scribu. See #18948.

Note: See TracTickets for help on using tickets.