Posted by mindless on October 27, 2005 at 4:56pm
3 followers
| Project: | SQL Search (Trip Search) |
| Version: | master |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | mindless |
| Status: | closed (fixed) |
Issue Summary
Near the top of node_trip_search it adds the MATCH clause to $query->wheres if mysql_full mode is in use. If comment module is present it adds subject/comment to $query->fields. When the query is put together you get all the fields OR'ed together (match if any specified field matches) but the MATCH is AND'ed in.. ie, even if a comment matches it won't return that topic unless the MATCH clause also evaluates to true. When fulltext is not in use you get node title/body in $query->fields so everything is properly OR'ed together.
Comments
#1
Here is my quick fix for this problem.. it now ORs the MATCH clause with the clauses generated by $query->fields. Patch is against current CVS. Hope this helps..
#2
Hm, I suppose rather than:
if ($query->fields || $query->match) {$query = trip_search_expand_query($query, $keys);
}
this would be more appropriate:
if ($query->fields) {$query = trip_search_expand_query($query, $keys);
}
else if ($query->match) {
$query->wheres[] = $query->match;
}
Though since with the current code $query->fields is never empty, this doesn't really matter.
#3
The previous patch created significant performance problems now that the LEFT JOIN on comments table must actually be processed. We now use a new fulltext index on comments table (subject,comment) and a separate query to check comments. New patch against cvs is attached. Note this patch also includes some fixes we use for proper escaping of quotes (though we run 4.6; hopefully I got everything right in this cvs patch). Note the "filter results by category" counts are still based only on node matches; comment matches not counted.
#4
..and one more update to the patch to fix another bug.. user:"user name" syntax (this syntax is generated by advanced search but then not parsed correctly).
#5
Thanks for the improvements, apologies that I haven't found a chance to test this. Would you be willing to fix any new bugs that the patch may raise? If so, please go ahead and apply it.
#6
Status: I applied the user:"user name" fix mentioned in comment #4. nedjo has already applied some of the other search-term-parsing fixes. These parts of the patch above are now in CVS:
@@ -307,7 +307,7 @@
@@ -554,6 +554,7 @@
@@ -700,18 +701,15 @@
Now the remaining parts of the patch file relate just to this bug report :-)
#7
I've taken on trip_search in 4.7 - but ran into the same problem with comments and for the moment have removed it altogether. Is there any interest in looking at this subject again? The code has changed a lot though...
#8
we've had to disable search.module due to performance problems, so yes we'd certainly be interested if trip_search worked on 4.7 and could search comments.
#9
my work-in-progress, if you'd like to take a look.
#10
Thanks - I am waiting for approval for a new version which will have changed a fair bit once again, in order to overcome XSS vulnerability. Once that is available, if you would like to have a look again at including comments I would certainly be interested.
#11
new code using a fulltext index on comments table now in DRUPAL-4-7 branch.
#12