support subqueries
douggreen - May 14, 2007 - 12:46
| Project: | Views |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
I want to add a query as a table name, but since views doesn't alias the table-as-query in add_table, the generated SQL is invalid (it includes the subselect multiple times int the query). The attached patch, moves $joininfo up a few lines in query(), passes $joininfo to get_table_name, and adds one check in get_table_name to look for the table alias ('right' => 'alias').
Please advise if there is a better way to do this. My code that uses this is as follows:
$sql = "(SELECT vfsn.nid FROM node vfsn LEFT join search_index vfsx on vfsn.nid=vfsx.sid WHERE vfsx.fromsid = 0 AND (". implode(' OR ', $and_clause) .") GROUP BY vfsn.nid HAVING COUNT(*) = ". count($and_clause) .")";
$join = array(
'left' => array(
'table' => 'node',
'field' => 'nid'
),
'right' => array(
'field' => 'nid',
'alias' => 'vfs',
)
);
$query->add_table($sql, false, 1, $join);
$query->add_where("vfs.nid IS NOT NULL", $arguments);I'm not sure yet how portable this code is. It works on mysql 5.
This request is for views_fastsearch. See also #143160.
| Attachment | Size |
|---|---|
| views_5.patch | 1.53 KB |
#1
What about modifying this
function get_table_name($table, $table_num, $joininfo, $alias_prefix = null) {
to be
function get_table_name($table, $table_num, $joininfo = null , $alias_prefix = null) {
This would mean that existing references to get_table_name in contrib functions (votingapi, nodefamily, etc) don't break or throw errors. I haven't tested the patch in terms of performance/query-structure improvements, but if it does improve performance, then it'll give people the opportunity to provide patches for existing contrib modules without generating errors from a views upgrade, and they'll get the performance improvement as the contrib modules get on board.
#2
subqueries are not supported either by drupal or views, so I see no need for fixing this.
But perhaps this should better be a feature request to support subqueries with the views $query object directly - what doesn't mean that views should use or require them, but it would enable others, like you, to make use of them.
However, then I think there should be a proper way to specify the subquery - e.g. by a new method for the query object. -1 for passing the subquery as table name, this looks like a hack to me.
So I take the liberty of recategorizing this issue.
#3
fago - i think we can rephrase - drupal5 does does not use subqueries because of its system requirements include db servers that don't support them. but it does nothing to discourage contrib modules from using them. views does actively discourage them, and this is a patch to remove that limitation. it is perfectly appropriate, IMO.
we can dream about fancier ways of handling subqueris, but unless someone is going to produce code, i'd like to evaluate the patch as it stands. would love a review from fago or mfrederickson especially.
#4
@fago - I'd love to write something more elegant that supports sub-queries using a pretty API. But my implementation here was the minimum amount necessary to get it working.
I agree that this is a bit of a hack. There's a problem with the subquery arguments that will only be solved by rewriting with a nicer query object interface. The problem is that if the subquery uses substitutable arguments, the only way to get these into the query is with add_where(), and if you do that, the ordering of the query arguments gets sketchy.
My implementation here was the minimum amount necessary to get it working. If more code that implements a nicer query interface would actually be considered, then I'll do that.
@moshe, I don't know the official meaning of the status's and if one state over another makes it such that more people look at it. Given what fago said, and what I just commented on, changing this back to "needs work" is fine with me.
As for the comment that Drupal and views does not support subqueries..., This would not actually make views dependent on subqueries, but allow views to support them. With the upcoming 6.x mysql requirements at 4.1 or higher, getting ready to support subqueries is, IMHO, a good thing.
#5
I agree that it would be a great step forward if the views query object is capable of subqueries. But introducing it like that - "unofficially", might just produce troubles. Ok, it gets in, it works and you build upon it. But there is no visible support for it - so as people aren't aware of that, upcoming patches might break it...
that's why I vote for a visible method, add_subquery() or so.
Rough implementation idea:
It could even take another $query object, that will be inserted in the tablequeue as a special "subquery" element - as soon as the query is generated we call ->query() on the subquery object and add it to the main query.
#6
Agreed, yesterday I started to try and implement this yesterday and ran into the wall that this is going to impact quite a bit, especially in query(). But it appears that we're on the same path, which is encouraging I thought I posted this, but evidentially I didn't. Here's the initial function:
function add_subquery($subquery, $joininfo) {if ($joininfo) {
$this->joins['subquery:'. $subquery->query()][] = $joininfo;
}
$this->tablequeue[] = array('query' => $query, 'alias_prefix' => $this->use_alias_prefix);
}
Do you have any suggestions.
#7
Attached is a second pass at this implementation. This used the
add_subquerymethod call to clearly distinguish that we're adding a query and not a table. As such, it also takes some shortcuts in the table definition and joins, since we shouldn't have more than one inclusion of each subquery. I'm currently using this on views_fastsearch 5.x-dev version with a conditional check usingmethod_exists($query, 'add_subquery')#8
this looks great!
perhaps a comment would be fine, but
+1 for adding this to views, so that modules like views_fast_search can make use of it.
#9
Attached patch adds a function comment.
#10
Hrm, subqueries are going to cause problems with db_rewrite_sql because db_rewrite_sql does a str_replace on the WHERE, GROUP, or ORDER words, within knowledge of where they occur in the SQL. I'll create a new issue and attach the issue number here.
#11
i suspect that best approach is for Views to handle this. It should pass primary query to db_rewrite_sql() and then inject any subqueries after. might subqueries sometimes want db_rewrite_sql(). i guess they could run it themselves.
#12
See 151910 for db_rewrite_sql issue.
#13
@Moshe, I think that you are you proposing that _views_query::query() inject a placeholder in the query for the sub-queries and another array element for the actual sub-queries, that _views_build_query and _views_get_query return and that the sub-query be injected on that views.module+538.
That's not an aweful solution, but it feels a bit like a hack. Do you have a reason for wanting to solve this in views rather than in core, other than the fact that it will be easier to get done in views. I will try to code this up for views tomorrow, but I'd still like to work on a cleaner solution for 6.x core.
#14
the only reason is that i can't think of a clean solution for core. if you can, thats great.
#15
I've offered a solution for core (#151910), but as this will at best, not be available until 6.x, I've also taken Moshe's suggestion and re-rolled this patch with such that the sub-query is not passed to db_rewrite_sql.
#16
How do we handle databases that don't support subqueries? Drupal 5 still allows MySQL 3.23 (which, heh, I am running. Don't laugh).
#17
I was thinking that views sub-queries would be used by contrib modules that wanted them and that they'd come with a database/version requirements.
I think that pgsql supports sub-queries as of 7.1 and mysql supports sub-queries as of 4.1. The current Drupal requirements for pgsql of 7.3 already supports sub-queries and the coming 6.x requirements for mysql will support sub-queries. However, if we wanted to be compatible with all databases, I could add a check for mysql prior to 4.1 and fallback to using db_temporary_query() in that case.
Essentially I've done this in views_fastsearch, when this patch isn't available.
if (method_exists($query, 'add_subquery')) {$query->add_subquery($sql, $terms, $join, 'temp_vfs');
$query->add_field('score', 'temp_vfs');
}
elseif (db_query_temporary($sql, $terms, 'temp_vfs')) {
$query->add_table('temp_vfs', FALSE, 1, $join);
$query->add_field('score', 'temp_vfs');
}
$GLOBALS['db_type']returns the database engine (mysql/pgsql). Does Drupal have a handy variable (that I'm not aware of) to detect the mysql version, or do we need save something? If not, it makes sense to do this once on install or update, and not do it again until the next update. Where would this check go? Once the .install update hook runs, it won't get run again. So where do we put something that should run on every update.php?mysql> select version();mysql> show variables like 'version';
#18
Here's a piece of code Views already uses:
<?php// set up the database timezone
if (in_array($GLOBALS['db_type'], array('mysql', 'mysqli'))) {
static $already_set = false;
if (!$already_set) {
if ($GLOBALS['db_type'] == 'mysqli' || version_compare(mysql_get_server_info(), '4.1.3', '>=')) {
db_query("SET @@session.time_zone = '+00:00'");
}
$already_set = true;
}
}
?>
I think Views should probably have a way of detecting if a db supports subqueries so that authors don't need to care what the details are, they just ask Views if subqueries are supported and if so, add the subquery, if not, do their own thing.
#19
The attached patch adds a function
_views_is_subquery()so authors can check for sub-query support, and it uses the function in add_subquery to fallback to db_temporary_query if needed. I like the graceful fallback, but I'm not sure if this is more than you were asking for.#20
Good news and bad news!
Good news: I approve of this patch.
Bad news: I'm just enough concerned that I am not going to commit it until after I can get 1.6 out of beta.
#21
Any news on getting this committed?
#22
Someone has made a patch against 5.2 to allow db_rewrite_sql() to fully support subqueries. See the second patch here: http://drupal.org/node/162970
I'm running it on my site and it has worked great, no bugs.
#23
I'm not sure if views changed, or if I never tested arguments. But here's an updated patch that fixes subquery arguments that get cached.
#24
Marking back to CNR.
I ran into some problems involving subquery arguments and caching. If the subquery has arguments and the query has arguments, the original patch tracked them and let pager_query handle them. But a subsequent change to views did the query substitution before caching, scrambling the subquery arguments.
IMO we just can't views cache subqueries. This is because we can't db_rewrite_sql subqueries. (The core handler simply isn't smart enough to handle the string.) Since we can't db_rewrite_sql subqueries, my solution is to use the [SUBQUERY] replacement string, and with this solution the arguments can't be replaced at cache time.
This new patch just skips caching when views subqueries are used.
#25
And here's one more fix, we need the query_substitution args...
#26
Anyone available to review this important patch?
#27
I try not to RTBC my own patches, and I'm especially cautious with views. But I haven't created a views_fastsearch release for 6 months, because the next generation of VFS relies on this patch. I use this patch with VFS on all of my sites. I last updated the patch in October, and I just confirmed that the patch still applies. Please, could this be committed?
#28
Would someone be able to provide an example of how to use the functionality provided by this patch? Thanks in advance!
#29
I use it all the time, when I write my custom views fields and sorts. The introduction to the issue has one such use.
Here's an example for retrieving the number of people who have answered "i love this", for use with the ilovethis.module:
$sql = "SELECT nid, COUNT(*) AS num FROM {ilovethis} GROUP BY nid";$joininfo = array(
'type' => 'outer',
'left' => array('table' => 'node', 'field' => 'nid'),
'right' => array('field' => 'nid'),
);
$query->add_subquery($sql, array(), $joininfo, 'ilovethis_supporters');
$query->add_field('num', 'ilovethis_supporters', 'num_supporters');
Here's a similiar example from ilovethis, to rank the support:
$sql = "SELECT nid, COUNT(*) AS num, (@rank=COALESCE(@rank, 1)) as rank FROM {ilovethis} GROUP BY nid";$joininfo = array(
'type' => 'outer',
'left' => array('table' => 'node', 'field' => 'nid'),
'right' => array('field' => 'nid'),
);
$query->add_subquery($sql, array(), $joininfo, 'ilovethis_supporters_rank');
$query->add_field('1/ilovethis_supporters_rank.rank', '', 'popularity_support_ranking');
Here's an example for og, to get the number of related documents:
$sql = "SELECT COUNT(*) AS num, group_nid FROM {og_ancestry} GROUP BY group_nid";$joininfo = array(
'type' => 'outer',
'left' => array('table' => 'node', 'field' => 'nid'),
'right' => array('field' => 'group_nid'),
);
$query->add_subquery($sql, array(), $joininfo, 'og_count');
$query->add_field('num', 'og_count', 'og_count_num');
Here's another og example I use in a custom application to return the number of new related documents,
$sql = "SELECT COUNT(*) AS num, a.group_nid FROM {node} n INNER JOIN {og_ancestry} a ON a.nid = n.nid LEFT JOIN {history} h ON h.nid = n.nid WHERE h.uid = %d AND n.changed > h.timestamp GROUP BY a.group_nid";$joininfo = array(
'type' => 'outer',
'left' => array('table' => 'node', 'field' => 'nid'),
'right' => array('field' => 'group_nid'),
);
global $user;
$query->add_subquery($sql, array($user->uid), $joininfo, 'og_new');
$query->add_field('num', 'og_new', 'og_new_num');
Here's an example that sorts the og node based on whether the og documents are in a particular nodequeue:
$sql = "SELECT a.group_nid, GREATEST(an.created, an.changed, an.comment) AS updated FROM {node} an INNER JOIN {og_ancestry} a ON a.nid = an.nid INNER JOIN {nodequeue_nodes} q ON q.nid = an.nid AND q.qid = %d";$joininfo = array(
'type' => 'outer',
'left' => array('table' => 'node', 'field' => 'nid'),
'right' => array('field' => 'group_nid'),
);
$query->add_subquery($sql, array($tier1_qid), $joininfo, 'tier1');
$query->orderby[] = 'tier1.updated '. $sort['sortorder'];
It's hard to do these types of queries without subqueries. You can do them, but it requires temporary tables and more code.
I'm not sure if the previous poster intentionally changed this from RTBC to Active, so I'm switching it back.
#30
Terrific! Thanks, Doug!
#31
I tried to apply this recently to the latest version of views, and it no longer applies.
#32
FYI, the hunk that fails seems extranous:
@@ -2143,4 +2159,4 @@// An implementation of hook_devel_caches() from devel.module. Must be in views.module so it always is included.
function views_devel_caches() {
return array('cache_views');
-}
\ No newline at end of file
+}
So, it didn't apply, but it was rather peripherial.
#33
I believe this patch is rolled and kept in the spirit of the orginal patch. Do please review Doug to make sure I didn't do something silly in views_cache.inc (which is where this patch fails).
This was rolled against: Views 5.x-1.x-dev (2008-Apr-18)
using: Views Fast Search 5.x-2.x-dev (2008-Apr-18)
And it works wonderfully.
#34
Confirmed working great, it removes some nasty query :)
However, the pager_query() is still nasty... anything we can do about this by a clever workaround to db_range... ?
This is what it produces for us on MothersClick.com (which is using this patch in a high production enivorment):
5402.48 1 pager_query SELECT count( DISTINCT(node.nid)) FROM mc_node node INNER JOIN (SELECT n.nid, SUM(4 * COALESCE((i.score * t.count), 0) + 2.5 * COALESCE((POW(2, GREATEST(n.created, n.changed) - 1208550244) * 6.43e-8), 0)) AS score FROM mc_node n LEFT JOIN mc_search_index i ON n.nid=i.sid LEFT JOIN mc_search_total t ON i.word=t.word WHERE i.fromsid=0 AND i.word IN ('mom') GROUP BY n.nid HAVING COUNT(*)=1) temp_vfs ON node.nid = temp_vfs.nid LEFT JOIN mc_community_content community_content ON node.nid = community_content.nid INNER JOIN shared_users users ON node.uid = users.uid LEFT JOIN mc_node_comment_statistics node_comment_statistics ON node.nid = node_comment_statistics.nid WHERE (node.status = '1') AND (node.type IN ('content_note')) AND (community_content.nid = node.nid) AND (community_content.group_nid = 332)
95.27 1 pager_query SELECT DISTINCT(node.nid), temp_vfs.score, node.title AS node_title, node.changed AS node_changed, users.name AS users_name, users.uid AS users_uid, node_comment_statistics.comment_count AS node_comment_statistics_comment_count, node_comment_statistics.last_comment_timestamp AS node_comment_statistics_last_comment_timestamp FROM mc_node node INNER JOIN (SELECT n.nid, SUM(4 * COALESCE((i.score * t.count), 0) + 2.5 * COALESCE((POW(2, GREATEST(n.created, n.changed) - 1208550244) * 6.43e-8), 0)) AS score FROM mc_node n LEFT JOIN mc_search_index i ON n.nid=i.sid LEFT JOIN mc_search_total t ON i.word=t.word WHERE i.fromsid=0 AND i.word IN ('mom') GROUP BY n.nid HAVING COUNT(*)=1) temp_vfs ON node.nid = temp_vfs.nid LEFT JOIN mc_community_content community_content ON node.nid = community_content.nid INNER JOIN shared_users users ON node.uid = users.uid LEFT JOIN mc_node_comment_statistics node_comment_statistics ON node.nid = node_comment_statistics.nid WHERE (node.status = '1') AND (node.type IN ('content_note')) AND (community_content.nid = node.nid) AND (community_content.group_nid = 332) ORDER BY node_comment_statistics_last_comment_timestamp DESC LIMIT 0, 20
#35
rerolled this patch to work with mysqli connection. Wasn't working when shifted to mysqli
#36
After talking to m3avrck, he reminded me that mysqli requires MySQL 4.1 or greater. Therefore, I rerolled the patch. Should be faster this way
#37
Scott, thanks for the re-rolls. I haven't looked at your changes close enough to comment, except:
I discovered today, that the order of where the SUBQUERY occurs, and on my most recent project, I need the SUBQUERY last instead of first. I'm uploading a new patch, but I haven't tested it with views fastsearch. If this breaks VFS, someone please comment here, and I'll find another solution for this. Thanks!
#38
FYI, I have used a few simple subqueries in my fields for OG and Views2 without issue. There is an example in comment.views.inc. search for views_handler_argument_comment_user_uid
#39
subscribing
#40
Hi Doug,
Not sure if im just "Doing it wrong" but it seems to break VFS
the error it outputs:
Fatal error: Call to undefined function views_get_view() in C:\Program Files\wamp\www\ihp\modules\views_fastsearch-5.x-2.0\views_fastsearch\views_fastsearch.module on line 620#41
Hi,
Thanks for all these patches but I haven't found one that works successfully with the last version of views 5.x-1.6.
Does anyone have a patch to support subqueries that works for the current views module?
Thanks,
James
#42
This would be helpful for #292835: Add Views Sort to sort by number of user's who signup for a node at least, and I'd be shocked if we didn't need to make use of this somewhere in project* once most of that is implemented with views. ;) For example, being able to sort projects by number of releases, to pull the latest release info while browsing projects, etc, etc. Time permitting I'll test this, but meanwhile, I just wanted to share my +1 on the concept and provide a link to another concrete use-case.
#43
@peashooter, The latest patch works on at least a half dozen of my projects, including the one I just worked on. When you say "you haven't found one that works successfully with the last version of views 5.x-1.6", what doesn't work? Does the patch not apply? Does it generate errors? Does it not work when you try to add a subquery in a views handler?
#44
@douggreen, when I try the patch in comment #37 to the latest views module (v 1.166.2.43) I get this output:
patching file views.module
Hunk #2 FAILED at 568.
Hunk #3 succeeded at 711 (offset 2 lines).
Hunk #4 succeeded at 932 (offset 2 lines).
Hunk #5 succeeded at 2171 (offset 2 lines).
1 out of 5 hunks FAILED -- saving rejects to file views.module.rej
patching file views_cache.inc
patching file views_query.inc
I'm assuming that means that it's not going to work.
Cheers for any help you might suggest.
James
#45
That patch works on the 5.x-1.6 version. For the latest dev version, use the attached patch.
#46
Sorry doug, my sincere apologies, you're right.
I don't know why it was playing up before.
Now I'm having trouble with your patch here (http://drupal.org/node/166244) but I'm pretty sure that's because og_views.inc has been modified since the last patch release.
Thanks for your help.
#47
Edit: Sorry for the previous hijack here...
Please have a look at the existing #230066: error after subquery patch issue.
#48
EDIT: Posted in an existing issue reporting "an error in your SQL syntax" after applying this #37 subquery patch.
As a sum-up of my case, the error seems triggered whenever I add any other filter to the VFS "Search: Fast Index" one (in a Views_FastSearch view, of course).
#49
One more changed, needed to replace the views_query_substitutions replacements in the subquery...
#50
Which 5.x version is this patch for? 1.x-dev or -1.6?
EDIT : #49 patch tested for 1.6. No effect on the #230066: error after subquery patch issue.