Running Tests on Data module results in unrecoverable error "Call to undefined function db_escape_string()"

I suspect db_escape_table() is simply no longer needed and that db_select() and the rest of the DB API does its own sanitization -- though I can't find any documentation or mention of db_escape_table()'s removal.

Also getting this when calling $table->handler()->delete();

Fatal error: Call to undefined function db_escape_string() in /var/www/d7/sites/all/modules/contrib/data/includes/DataHandler.inc on line 155

There is no db_escape_string() in D7 -- only _field and _table.

Also

Fatal error: Call to undefined function db_type_placeholder() in /var/www/d7/sites/all/modules/contrib/data/includes/DataHandler.inc on line 155.

Also db_affected_rows(); -- same function.

Hm.. here's a first stab at fixing it..

Instead of using typical placeholders, I'm using :[name-of-the-field], and the arguments would be array(':[name-of-the-field]', $value).

Adjusted execute() to properly handle taking in arrays as argument values.

I removed the return on delete(), which was return db_affected_rows(), only because I'm not sure how to do this in D7 yet, when using db_query(). Seems like the only way to return the amount of items deleted would be to have execute() return the items, using D7's $query->execute, then having delete() return that value.

Patch probably needs some work..

+++ b/includes/DataHandler.inc
@@ -152,13 +152,12 @@ class DataHandler {
+      $query->addWhere(db_escape_table($this->table) . '.' . db_escape_field($key) . " " . $operator . " :" . $fields[$key]['type'], array(":{$fields[$key]['type']}" => $value));

This looks scary. What is it actually supposed to be doing?

+++ b/includes/DataHandler.inc
@@ -152,13 +152,12 @@ class DataHandler {
       $query->execute();

This will return the number of rows deleted. See http://drupal.org/node/310081

Fixing the scary part.. I messed up something..one minute

Here's another one..

This is what is changing..

Before, the addWhere was passing in something like "table.field = %d", and the argument would be 2.

Now, the addWhere is passing in "table.field = :table_field", and the argument is array(":table_field" => 2).

And no, that $query->execute(); on line 161 (DataHandler.inc) will not return the amount of items deleted. For two reasons. 1) delete() doesn't return the result of $query->execute(). 2) $query->execute(), in DataHandler.inc, returns a db_query() result.

You're thinking of Drupal's $query->execute(), which isn't being used here -- but should be.

class DataQuery should probably be removed in favor of D7's database API. It's basically doing the same thing.

Omg. We have our own entire delete query class? I never even knew.

> class DataQuery should probably be removed in favor of D7's database API. It's basically doing the same thing.

Yes! But probably in a new issue.

Ha yea.. as I'm looking through it, it appears that class is only used for deletes.

What do you think of the patch, for a temporary solution?

+      $query->addWhere("{$field} {$operator} :{$placeholder}", array(":{$placeholder}" => $value));

There should only be {} around the table parts surely.
We're not doing anything especially wacky are we, such as using column names as values?

The docs example suggests: "'feed_nid' => 10,"

> Yes! But probably in a new issue.

Actually, let's rip it out here.I can't get my head round how this class is meant to be used. It's buggy and hard to fix. The whole of the delete() function could be rewritten to use a D7 DB API db_delete query in half a dozen lines.

The {} won't appear in the query. If we wanted them to, they would be doubled {{}} - since the string is wrapped with "".

> The docs example suggests: "feed_nid" => 10

That would translate as:

table_name.feed_nid = :table_name_feed_nid

And the value would be

array(":table_name_feed_nid" => 10);

It works. Just like: http://drupal.org/node/310072#placeholders

I do agree, it should be ripped out now. And I suppose that entire class should be removed, too, right?

Huh... DeleteQuery doesn't support joins..

http://api.drupal.org/api/drupal/includes%21database%21query.inc/class/D...

Maybe we need to keep it with my patch then..

Unless I'm missing something

I expect DeleteQuery doesn't do joins because Postgres can't: http://drupal.org/node/555562

I wonder if we can extend the DB API deletequery. Maybe as part of a follow-up issue though. Let's get these errors fixed first.

I've filed #1467264: replace DataQuery with DB API delete query for this.

Ok changed my mind *again*... let's rip it out now.

- only the delete method uses this class
- only the tests use the delete method (!!!!)
- only data_taxonomy uses the query alter hook here and it is likely to be deprecated for D7 anyway -- see #1363554: deprecate data_node, data_taxonomy on D7, upgrade path?.

Testing on my local D7 seems to be broken right now... can you give this patch a try please?

Alright - patch looks pretty good - nice work. I'm still learning exactly how Data works - I just actually stumbled across the module and really love the concept and execution.. you guys did a great job.

So, like you said, with this patch, data_node and data_taxonomy are gone, right?

I also don't see any info in data.api.php about hook_data_delete_row(), nor any other submodules implementing it - also, why is the hook called hook_data_delete_row(), when it's potentially deleting more than one row.

It's a little unfortunate not being able to add joins to the delete query. I'm trying to figure out how that would be done now. For instance, I planned on using flag_entity to be able to flag data items. When a data item is deleted, how would this hook allow me to delete rows in the flag table? The only things available in the hook are the data class and the clause used to delete. I guess you'd have to then query the table with the same WHERE clause to determine which items are going to be removed, before they are. Or I missing something entirely (very likely)?

Thanks

Next step is getting data support back into Feeds for 7.x.

> you guys did a great job

Heh I can't take much of the credit if any -- I just became co-maintainer back in December to get the D7 branch moving forward :)

> So, like you said, with this patch, data_node and data_taxonomy are gone, right?

IMO they're more or less obsolete with the data_entity module I added to D7. I'm waiting for other people to comment on the issue about it. If people still need them, and are prepared to upgrade them to D7 we can keep them going.

> I also don't see any info in data.api.php about hook_data_delete_row(), nor any other submodules implementing it - also, why is the hook called hook_data_delete_row(), when it's potentially deleting more than one row.

Oops. Good catch on both of those. I'll update my patch with both of those when I have time; setting it back to needs work for that.

> flag_entity

Presumably that has the hook to detect entity deletion, which data_entity should invoke. Not that I'm saying either of these things happen at the moment ;)

Yea I had a feeling that entity would take care of it - but you never know.. that's how it should be handled any way.. I think flag_entity needs work, so perhaps that's the problem.

I agree with your approach to data_entity taking over for data_node and data_taxonomy - makes sense.

Well "great job" to anyone who had a commit on this module - it's f'ing brilliant..

Let me know if there's anything else I can do for this issue (or any others)

- #1419798 by mikestefff, joachim: Fixed error in tests; changed custom query class to use db_delete(); removed hook_data_delete_query_alter().

Here's the final patch.

Thanks for all your help on this!

Nice... looks good..

My pleasure.

I am getting the return message below in Drupal 7:

Fatal error: Call to undefined function db_escape_string() in C:\xampp\htdocs\drupal\sites\all\modules\contrib\data\data_search\data_search.module on line 159

I am using module data-7.x-1.0-alpha3.tar Note that it also exists in the current dev version

Per the discussions this has been depreciated in Drupal 7

See #1744510: Port Data Search to D7.