WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#18948 closed enhancement (fixed)

Introduce $wpdb->delete()

Reported by: scribu Owned by: 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 3 years ago.
my suggestion
18948.2.diff (14.7 KB) - added by justindgivens 2 years ago.
Updated wpdb->delete and the files I've updated the DELETE queries.
18948.3.diff (14.2 KB) - added by justindgivens 2 years ago.
updated from scribu notes.
18948.4.diff (14.1 KB) - added by justindgivens 2 years ago.
18948.5.diff (13.0 KB) - added by scribu 2 years ago.
18948.6.diff (14.1 KB) - added by scribu 2 years ago.

Download all attachments as: .zip

Change History (33)

comment:1 scribu3 years ago

  • Component changed from General to Database

justindgivens3 years ago

my suggestion

comment:2 justindgivens3 years ago

  • Keywords has-patch added; needs-patch removed

comment:3 scribu3 years ago

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

comment:4 scribu3 years ago

  • Keywords early added

comment:5 DrewAPicture3 years ago

It seems like a no-brainer to me.

+1.

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

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

justindgivens2 years ago

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

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

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

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

justindgivens2 years ago

updated from scribu notes.

comment:11 justindgivens2 years ago

Made the changes in 18948.3.diff.

comment:12 follow-up: scribu2 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.

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

comment:14 scribu2 years ago

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

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

comment:15 scribu2 years ago

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

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

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

justindgivens2 years ago

comment:17 justindgivens2 years ago

18948.4.diff Updated with the quotes removed.

comment:18 scribu2 years ago

Related: #19019

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

scribu2 years ago

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

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

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

scribu2 years ago

comment:23 scribu2 years ago

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

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

comment:25 GaryJ2 years ago

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

comment:26 scribu2 years ago

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

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

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