#18948 closed enhancement (fixed)
Introduce $wpdb->delete()
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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)
Change History (33)
justindgivens
— 20 months ago
comment:2
justindgivens
— 20 months ago
- Keywords has-patch added; needs-patch removed
comment:3
scribu
— 20 months ago
Yep, that's what I had in mind. The bigger task will be using it throughout the WP codebase.
comment:5
DrewAPicture
— 20 months ago
It seems like a no-brainer to me.
+1.
comment:6
follow-up:
↓ 8
justindgivens
— 20 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 Kaiser
— 20 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.
comment:8
in reply to:
↑ 6
;
follow-up:
↓ 9
scribu
— 20 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
justindgivens
— 20 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
scribu
— 20 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' ) );
comment:11
justindgivens
— 20 months ago
Made the changes in 18948.3.diff.
comment:12
follow-up:
↓ 13
scribu
— 20 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
justindgivens
— 20 months ago
comment:14
scribu
— 20 months ago
Just because existing code is bad doesn't mean new code also has to be bad.
comment:15
scribu
— 20 months ago
Also, maybe this can be extracted into a helper method:
protected function get_form( &$where_formats, $where_format ) {
comment:16
scribu
— 20 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.
justindgivens
— 20 months ago
comment:17
justindgivens
— 20 months ago
18948.4.diff Updated with the quotes removed.
comment:18
scribu
— 20 months ago
Related: #19019
comment:19
GaryJ
— 20 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.
comment:20
scribu
— 17 months ago
- Milestone changed from Future Release to 3.4
- refreshed against trunk
- fixed fatal error (!) in comment.php
- leave deprecated.php alone
- cleaned up wpdb::delete()
comment:21
nacin
— 17 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
scribu
— 17 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.
comment:23
scribu
— 17 months ago
18948.6.diff removes the redundant format parameters and replaces " with ' where appropriate.
comment:24
nacin
— 15 months ago
- Owner set to nacin
- Resolution set to fixed
- Status changed from new to closed
In [20287]:
comment:25
GaryJ
— 15 months ago
[20287] appears to show the @since for the new method as 2.5.0, rather that 3.4.0 ?
comment:26
scribu
— 15 months ago
Yeah, also this usage example in the dockblock should be removed:
wpdb::delete( 'table', array( 'ID' => 1 ), array( '%d' ), 1 )
comment:27
duck_
— 15 months ago
In [20292]:
my suggestion