Implement db_insert and db_update APIs?

db_insert('accesslog', array(
  array('title', "s", strip_tags(drupal_get_title())),
  array('uid', "d", $user->uid),
));

Interesting, but it's not very usefull IMO.

I'm for a better db api with some more abstraction, but let's think big, and let's make something really useful

How about..

db_insert('accesslog', array( 'title' => $title, 'path' => $path ... ))
);

I mean authomatic table prefixing, typing, field checking, etc....

I am very much in favor of db_insert(), db_update(), and db_delete() utility functions. I wrote something very similar at work for the same reason: SQL's vastly different syntaxes for insert and update statements piss the hell out of me. :-) What I ended up with was to the effect of:

$fields['field1'] = 5;
$fields['field2'] = 'my string';

$where['uid'] = 4;

db_insert('tablename', $fields);
db_update('tablename', $fields, $where, 'AND');
db_delete('tablename', $where, 'AND');

(The last parameter being AND or OR, and defaulting to AND.)

The advantage here is that not only is the code easier to write, but insert and update then have the same API. That makes code that would do either an update or an insert far easier and cleaner, particularly if there's a lot of fields. Internally it would just use db_query(), so it's database driver agnostic.

I'd be happy to code these up once HEAD opens up again if there's interest, which it seems there is.

Hi! I'm another voice that would like to see calls such as:

db_insert('tablename', array('field' => 'value'));

This would especially be useful with the new form api, which passes you an array of the data elements already...

Assigning to me so that I remember to do it. :-)

Open question to the higher-ups: Would these be better implemented as wrappers atop db_query(), in which case they're inherently db-agnostic, or as alternate versions for each database the way the rest of the db_* functions are? And if agnostic, what's the best place for them to live?

On the mailinglists, someone asked if these functions are better off in helpers.module or elsewhere.

Here a short reaction:
IMO each and every function in helpers.module should be a candidate for core. because IMO core should be a usefull toolbox, more then anything else. I'm planning on adding those db utility functions. As per the question I
asked in the thread, should I be looking to add those to helper.module rather
than core somewhere? I'm just wary of people not even knowing they exist.

So right now, I see helpers module as two things:
* A place to store functions untill they get into core (there is a helpers.module coding guideline to put the url of a drupal issue with all function that have an issue)
* A place to test out usefull functions and concepts before they get into core.

Bèr

OK then, here's db_insert(), db_update(), and db_delete(). They should all be db-agnostic at this point, and all are just wrappers around db_query() so all db_query()-based magic and record keeping should all still work. It also introdeuces a new utility function, db_where_clause(), which can build most SQL where clauses.

The patch also alters comment.module to demonstrate the use of db_insert() and db_update(). I suspect a final patch will not do so, but it's good for testing at the moment.

Please stress-test! I'm especially interested if there are any injection errors I missed by accident. I think they're fairly tight, but I'd like a second opinion.

Rerolling for HEAD. Come on, guys. You asked for it, here it is; review it or you won't get it. :-)

Dries has basically rejected this notion of query building. I have to agree. I'd like functions for schema add/edit, but not standard queries. "SQL is already a DB abstraction"

Sorry, I disagree, even Dries can be made to change his opinion. :p

A simple(!) query builder will allow us to do more stuff with Drupal and thus is a good thing.

I wouldn't even classify this as a query builder per se, actually. When people say "query builder", I think something like PEAR::DB_DataObjects. Some way to build complex select statements methodically or in a data-driven fashion. That's a separate matter.

What I'm aiming for here is to simplify three SQL syntaxes that are, frankly, far more complex than they have any right to be. :-) (What fool made INSERT and UPDATE have completely different syntaxes???) I have run into cases before with custom Drupal modules for a client where I was editing a table that was simply by nature insanely long (~30 fields), and in 4.6 I ended up doing a delete/insert instead of an update, just to avoid having to write the nasty code for both an insert and an update. That saved me a lot of development time, even though it's bad programming technique.

With proper thin wrappers (this patch), the difference between an insert and an update is a one-line if-statement. It makes larger queries easier to write, and data-driven actions easier to manage. The resulting code is also, IMO, prettier and cleaner.

A "query builder" for select statements is a subject for a different thread.

Killes, you marked needs work instead of needs review. What needs work, then?

a) How would you recommend dealing with NULL? I use it so rarely myself that I'm actually not certain how it should be escaped. :-)

b) If you want to do an OR, then you simply pass OR as the last parameter.

db_update($table, $fields, $where, 'OR');

That would join the $where parameters with a series of ORs instead of ANDs. In my experience, the vast majority of update or delete queries are very simple ANDs, or sometimes ORs. If you really need to have a mixed AND-and-OR update/delete query, you could, for instance, build a series of ORs separately and then make that whole string one of your AND parameters. It's actually quite flexible.

c) I'd prefer to keep the input arrays simple associative arrays rather than nested. It makes the API easier to work with, as well as allows for code that simply passes through an existing array without having to build a custom data structure just for this function.

I don't see the point of db_insert() / db_update(). What exactly does it buy us? Tempted to mark this "won't fix".

... and it can introduce bugs. When the programmer specifies %d and %s, the input data is cast to the proper types (preventing errors). With this patch, what should be an integer could still be cast to a string because we'd resolve it to %s. So, we'd loose the type checking.

The format_date() function is not ment to replace date(). date() is actually faster and easier to use. The format_date() function exists because we want all dates to look similar and be configurable. It's sole purpose is consistency.

Sorry, but I really don't see the point / advantage. It's less accurate, slower, non-standard and less transparant. It increases the barrier to entry for developers that know SQL.

Sorry, but this is not something for core. Writing an insert SQL query is not difficult, and when you got it wrong, it is easy to fix.

By the way, the patches that did not use '%s' and %d are insecure. They are vulnerable to SQL injection attacks ...

The fact that Dries does not like this in core does not mean they cannot be a very good addition.

Hence moving this issue into helpers.module.

I really need to get around and extract a database_helpers.module from that fast.

Having central places to let your code go through is generally considered a Very Good Thing.

After all, can't we all type . Sure! So why bother about a forms abstraction layer. "its only a way to introduce bugs"....

Well dur. I leave town for a few days and this thread is suddenly active again.

I still disagree that this is "unnecessary". As I said, I've run into cases where these utilities would have saved me hours of keeping track of a zillion commas and single quotes. They're less of a "Query builder" than the existing db_* API. But if Dries is firmly against it in core, then I guess helpers module it is.

Here's a new patch against helpers module. It's mostly the same as the earlier patch, with 2 changes:

- The db_where_clause() function now supports another parameter to skip the "WHERE" keyword. That allows it to be used to build a portion of a WHERE clause as I mentioned earlier. That is, build one series of AND statements without a WHERE, then another series of AND statements without a WHERE, then put those in an array together and build a statement with the WHERE using OR. You can build pretty much any WHERE query this way, even for use in a SELECT query.

- I've switched from is_numeric to gettype() for determining the correct escape format. That means it's now dependent on the type of the passed value. To ensure that something gets treated as an int, simply make sure you cast it to an int before you pass it in. Ibid for float. While at first it may seem like it's punting needlessly, it's actually no more work in the calling routine than db_query(). With db_query, you have to make sure you correctly pick %d, %f, or '%s'. With db_insert()/db_update()/db_delete(), you have to decide to cast the value first. The net effect is the same, so there shouldn't be any more injection risk than with normal db_query().

Reviews welcome, of course.

On a sidenote (but important for this patch, and maybe others in this line): Should I add a helpers_db.module for patches like these? That way you don't need all the other stuff (theme functions, country/states forms etc) if all you want is query builders?

FWIW, I splitted out the helpers into a helpers.module (general, might need some more weeding still) helpers_database.module and helpers_form.module.

If someone can give me a "go", or a comment that the functions work as expected, I will move them into helpers. I'd like such a comment, because I don't have time to actually test the functions. A patch is not nessecary, IMO, as long as the functions in the last patch just work.

Anyone? Beuler? I can't really RTBC my own patch, so someone else has to jump in here. :-)

Comiited to helpers_database in CVS. (cvs helpers_database is still 4.7 compatible...)

Thanks a lot. I love these functions.

Heh. So around 2 years after Dries said this won't be in core, he is going to let it into core as part of the new DB API for Drupal 7.

I'm so glad we are getting to having good DB interfaces finally -- after arguing for them about 4 years ago. :-)

Hi, All.
I am new to DRUPAL,

Currently I am learning Drupal module development. I am using Drupal 6.19.

I want to create a module, which will have module.install file which will create database tables.
I also want these database tables will have some initial values inserted at the time of install only.
So I want to write multiple row inserts in "module_install()" after Drupal_install_schema('module');

I have tried drupal_write_record(), but it do not allow me to insert multiple records in one go like,

$data = array
(
'id' => 1, 'name' => 'name1111' , 'status' => 1,
'id' => 2, 'name' => 'name2222' , 'status' => 1
);
drupal_write_record('table', $data);

I have also tried,

$values = array(
array(
'id' => 1, 'name' => 'name1111' , 'status' => 1,
),
array(
'id' => 1, 'name' => 'name2222' , 'status' => 1,
),
);
$query = db_insert('table')->fields(array('id', 'name', 'status'));
foreach ($values as $record) {
$query->values($record);
}
$query->execute();

But it shows fatal error "Fatal error: Call to undefined function db_insert() in...."

Please help me.

The db_insert() function you're trying to use does not exist until Drupal 7. If you're running Drupal 6 it's not there. You'll have to run individual insert queries using db_query().

This is really a support question, so please use one of the various support channels rather than posting a long-closed issue.

how do we insert into a datetime column a value of sysdate()?

i tried: db_insert("table")->fields(array("datetime_field"=>"sysdate()"))->execute();

didn't work.