Opened 19 months ago

Closed 14 months ago

Last modified 14 months ago

#18948 closed enhancement (fixed)

Introduce $wpdb->delete()

Reported by: scribu Owned by: nacin
Priority: normal Milestone: 3.4
Component: Database Version:
Severity: normal Keywords: has-patch early
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 19 months ago.
my suggestion
18948.2.diff (14.7 KB) - added by justindgivens 19 months ago.
Updated wpdb->delete and the files I've updated the DELETE queries.
18948.3.diff (14.2 KB) - added by justindgivens 19 months ago.
updated from scribu notes.
18948.4.diff (14.1 KB) - added by justindgivens 19 months ago.
18948.5.diff (13.0 KB) - added by scribu 16 months ago.
18948.6.diff (14.1 KB) - added by scribu 16 months ago.

Download all attachments as: .zip

Change History (33)

  • Component changed from General to Database

my suggestion

  • Keywords has-patch added; needs-patch removed

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

  • Keywords early added

It seems like a no-brainer to me.

+1.

comment:6 follow-up: ↓ 8   justindgivens19 months 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?

+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.

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

comment:8 in reply to: ↑ 6 ; follow-up: ↓ 9   scribu19 months 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.

comment:9 in reply to: ↑ 8   justindgivens19 months 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.

$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' ) );

updated from scribu notes.

Made the changes in 18948.3.diff.

comment:12 follow-up: ↓ 13   scribu19 months 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.

comment:13 in reply to: ↑ 12   justindgivens19 months 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

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

Last edited 19 months ago by scribu (previous) (diff)

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

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

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.

18948.4.diff Updated with the quotes removed.

Related: #19019

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.

  • 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()

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().

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

Will update the patch shortly.

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

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

In [20287]:

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

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

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

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

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.