WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#7171 closed task (blessed) (fixed)

wpdb::insert() & wpdb::update() need to allow typecasting of params.

Reported by: DD32 Owned by: westi
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.6
Component: Administration Keywords: dev-feedback has-patch early
Focuses: Cc:

Description

As a follow up to: http://trac.wordpress.org/ticket/6836#comment:8

insert() and update() should prepare the values before inserting/updating

I see 2 options;

  1. 3rd param to insert() & update() which specifies the field types; Eg: http://trac.wordpress.org/ticket/6836#comment:9
    The downside to that is it'll require the type to be specified on each insert/update statement
  1. defining the field-types in an array to check against eg:
    $wpdb->fields = array( $wpdb->posts => array('ID' => '%d', 'post_content' => '%s') );
    
    ..
    in insert():
    foreach( (array)$data as $key => $value )
       if( isset($this->fields[ $table ][ $key ]) )
           $data[$key] = sprintf($this->fields[ $table ][ $key ], $value);
    

The downside of this approach, Is that it'll require the list to be kept updated, However, it has the advantage of being kept updated in a single location.

If theres a preference for either method, I'm happy to roll up a patch for it.

Attachments (3)

7171.diff (4.6 KB) - added by DD32 7 years ago.
format_insert_update.7171.diff (2.4 KB) - added by filosofo 7 years ago.
7171.2.diff (3.7 KB) - added by mdawaffe 6 years ago.
move $db_field_types to $wpdb->field_types

Download all attachments as: .zip

Change History (17)

comment:1 @westi7 years ago

  • Owner changed from anonymous to westi
  • Status changed from new to assigned

comment:2 @DD327 years ago

Any comments on the method of typecasting?

3 options:

  • Hard coded list of tables/fields in wpdb
  • ::update() ::insert() add a param to specify types
  • use the mysql_ field functions to do a select against the table, determine the type-casts based on the field type (requires a SELECT * FROM blah LIMIT 1 query) and cache it

the 3rd option half-takes place on all SELECTS allready: http://trac.wordpress.org/browser/trunk/wp-includes/wp-db.php#L631

comment:3 @DD327 years ago

  • Milestone changed from 2.7 to 2.8

@DD327 years ago

comment:4 @DD327 years ago

  • Keywords has-patch early added

attachment 7171.diff added.

  • Auto prepares insert() and update() queries
  • Caches table column types upon query, and uses them later on to determine the sprintf token to use
    • Loads any columns from the DB needed if they're not allready loaded.
  • No changes to calling of insert() / update() needed, Just ensure data is passed non-sql-escaped.
  • Code modified from a version of another app i'm developing off BackPress, So i've tested it over the past few weeks with 0 problems.

comment:5 @ryan7 years ago

How about hard-coding an array of tables, columns, and types so we can avoid any extra queries?

comment:6 @DD327 years ago

How about hard-coding an array of tables, columns, and types so we can avoid any extra queries?

That was an option, But in my testing, Its rare any extra queries are needed, at least on the most commonly accessed tables, My main reasoning for the approach i took was so that it would be usable by all applications/plugins(custom tables), and BackPress without having to define the list for every application.

Of course, Go ahead and hard code them if you want :)

comment:7 @filosofo7 years ago

I added a patch so that you can pass an optional argument to update or insert that specifies the format. If a string, that format will be applied to all of the fields; if an array, then the array member format corresponds to the field value.

comment:8 @ryan6 years ago

  • Type changed from enhancement to task (blessed)

comment:9 @ryan6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10724]) Add typecasting to wpdb::insert() and update(). Props filosofo. fixes #7171

comment:10 @ryan6 years ago

(In [10725]) Add some default field types. see #7171

comment:11 @ryan6 years ago

(In [10726]) Only list field types that are not strings. see #7171

comment:12 @mdawaffe6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Can we put the field_types info in wpdb rather than as a global?

Attached.

@mdawaffe6 years ago

move $db_field_types to $wpdb->field_types

comment:13 @ryan6 years ago

(In [10910]) Store field types in wpdb object. Props mdawaffe. see #7171

comment:14 @ryan6 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.