Make WordPress Core

Opened 17 years ago

Closed 16 years ago

#7171 closed task (blessed) (fixed)

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

Reported by: dd32's profile DD32 Owned by: westi's profile 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 16 years ago.
format_insert_update.7171.diff (2.4 KB) - added by filosofo 16 years ago.
7171.2.diff (3.7 KB) - added by mdawaffe 16 years ago.
move $db_field_types to $wpdb->field_types

Download all attachments as: .zip

Change History (17)

#1 @westi
17 years ago

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

#2 @DD32
16 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

#3 @DD32
16 years ago

  • Milestone changed from 2.7 to 2.8

@DD32
16 years ago

#4 @DD32
16 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.

#5 @ryan
16 years ago

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

#6 @DD32
16 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 :)

#7 @filosofo
16 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.

#8 @ryan
16 years ago

  • Type changed from enhancement to task (blessed)

#9 @ryan
16 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

#10 @ryan
16 years ago

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

#11 @ryan
16 years ago

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

#12 @mdawaffe
16 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.

@mdawaffe
16 years ago

move $db_field_types to $wpdb->field_types

#13 @ryan
16 years ago

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

#14 @ryan
16 years ago

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