WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 15 months ago

Last modified 15 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 20 months ago.
my suggestion
18948.2.diff (14.7 KB) - added by justindgivens 20 months ago.
Updated wpdb->delete and the files I've updated the DELETE queries.
18948.3.diff (14.2 KB) - added by justindgivens 20 months ago.
updated from scribu notes.
18948.4.diff (14.1 KB) - added by justindgivens 20 months ago.
18948.5.diff (13.0 KB) - added by scribu 17 months ago.
18948.6.diff (14.1 KB) - added by scribu 17 months ago.

Download all attachments as: .zip

Change History (33)

comment:1 scribu20 months ago

  • Component changed from General to Database

justindgivens20 months ago

my suggestion

comment:2 justindgivens20 months ago

  • Keywords has-patch added; needs-patch removed

comment:3 scribu20 months ago

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

comment:4 scribu20 months ago

  • Keywords early added

comment:5 DrewAPicture20 months ago

It seems like a no-brainer to me.

+1.

comment:6 follow-up: justindgivens20 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?

comment:7 F J Kaiser20 months 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.

justindgivens20 months ago

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

comment:8 in reply to: ↑ 6 ; follow-up: scribu20 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 justindgivens20 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.

comment:10 scribu20 months 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' ) );

justindgivens20 months ago

updated from scribu notes.

comment:11 justindgivens20 months ago

Made the changes in 18948.3.diff.

comment:12 follow-up: scribu20 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 justindgivens20 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

comment:14 scribu20 months ago

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

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

comment:15 scribu20 months ago

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

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

comment:16 scribu20 months 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.

justindgivens20 months ago

comment:17 justindgivens20 months ago

18948.4.diff Updated with the quotes removed.

comment:19 GaryJ20 months 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.

scribu17 months ago

comment:20 scribu17 months 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()

comment:21 nacin17 months 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().

comment:22 scribu17 months 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.

scribu17 months ago

comment:23 scribu17 months ago

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

comment:24 nacin15 months 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.

comment:25 GaryJ15 months ago

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

comment:26 scribu15 months ago

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

wpdb::delete( 'table', array( 'ID' => 1 ), array( '%d' ), 1 )
Version 0, edited 15 months ago by scribu (next)

comment:27 duck_15 months 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.