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.

CommentFileSizeAuthor
#9 trip_search.module_diff.txt9.57 KBmindless
#4 trip_search.module_patch_0.txt12.84 KBmindless
#3 trip_search.module_patch.txt12.77 KBmindless
#1 trip_search.module_fulltext_patch.txt2.67 KBmindless

Comments

mindless’s picture

mindless commented
Version: 4.6.x-1.x-dev » master
Status: Active » Needs review
StatusFileSize
new trip_search.module_fulltext_patch.txt2.67 KB

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..

mindless’s picture

mindless commented

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.

mindless’s picture

mindless commented
StatusFileSize
new trip_search.module_patch.txt12.77 KB

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.

mindless’s picture

mindless commented
StatusFileSize
new trip_search.module_patch_0.txt12.84 KB

..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).

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

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.

mindless’s picture

mindless commented

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 :-)

joel_guesclin’s picture

joel_guesclin commented

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...

mindless’s picture

mindless commented

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.

mindless’s picture

mindless commented
Status: Needs review » Needs work
StatusFileSize
new trip_search.module_diff.txt9.57 KB

my work-in-progress, if you'd like to take a look.

joel_guesclin’s picture

joel_guesclin commented
Assigned: Unassigned » joel_guesclin

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.

mindless’s picture

mindless commented
Assigned: joel_guesclin » mindless
Status: Needs work » Fixed

new code using a fulltext index on comments table now in DRUPAL-4-7 branch.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)