I have set up a view with a context search filter (search terms). So putting just 1 keyword as argument, the view gives me results. But if there are more words, there are always no results. I am 100% sure that this view must return results. When I use same search terms in "search/node/key1 key2..." the search gives me the results. There are no filters or other methods which exclude my wanted result items.

Here is the SQL statement with 1 keyword:

SELECT node.nid AS nid, node.title AS node_title, node.created AS node_created, SUM(search_index.score * search_total.count) AS score, 'node' AS field_data_field_largeimages_node_entity_type
FROM
{node} node
LEFT JOIN {search_index} search_index ON node.nid = search_index.sid
LEFT JOIN {search_total} search_total ON search_index.word = search_total.word
WHERE ((( (search_index.type = 'node') AND( (search_index.word = 'example') )))AND(( (node.status = '1') AND (node.type IN  ('article')) AND (node.created >= 1338764465-3456000) )))
GROUP BY search_index.sid, score, nid, field_data_field_largeimages_node_entity_type, node_title, node_created
HAVING (( (COUNT(*) >= '1') ))
ORDER BY node_created DESC

which returns results.

This is a statement with 2 keywords, returning nothing but should:

SELECT node.nid AS nid, node.title AS node_title, node.created AS node_created, SUM(search_index.score * search_total.count) AS score, 'node' AS field_data_field_largeimages_node_entity_type
FROM
{node} node
LEFT JOIN {search_index} search_index ON node.nid = search_index.sid
LEFT JOIN {search_total} search_total ON search_index.word = search_total.word
WHERE ((( (search_index.type = 'node') AND( (search_index.word = 'example1') OR (search_index.word = 'example2') )))AND(( (node.status = '1') AND (node.type IN  ('article')) AND (node.created >= 1338764592-3456000) )))
GROUP BY search_index.sid, score, nid, field_data_field_largeimages_node_entity_type, node_title, node_created
HAVING (( (COUNT(*) >= '2') ))
ORDER BY node_created DESC

Attached a view export.

Thanks for your help.

Files: 
CommentFileSizeAuthor
#30 search_multiple_terms-1615438-30.patch1.83 KBrsmylski
PASSED: [[SimpleTest]]: [MySQL] 1,627 pass(es).
[ View ]
#28 search_multiple_terms-1615438-28.patch829 bytesrsmylski
PASSED: [[SimpleTest]]: [MySQL] 1,603 pass(es).
[ View ]
#13 search-multiple-terms-1615438-12.patch1.03 KBawolfey
PASSED: [[SimpleTest]]: [MySQL] 1,603 pass(es).
[ View ]
view_search.txt50.79 KBmaxle

Comments

I just tested with stable 7.x-3.3 and there it works fine. So I just leave this issue as active that this problem won't be ignored in the dev branch. I tested this on another D7.14 installation and the problem also occurs there with the dev version from June 1st.

I think the problem is somewhere in the source code, not in the SQL statement.

Unfortunately I cannot work with 7.x-3.3 because this version breaks a lot of my views, which are correctly working (except this problem of course) in the dev version. So I'd be very thankful if a maintainer could look into this.

Thank you for your help.

Update: There could be some results when using more than 1 word, but specific node types are just not being recognised. Example: Some nodes of type A are returned in result set, but there are no results of type B - but there should, and this works in 7.x-3.3.

I'd be very thankful if someone cares about it and would give some feedback. The problem is still there in the current dev version. I hope it won't be present in the upcoming stable releases.

Version:7.x-3.x-dev» 7.x-3.5

So the problem is now in the current stable release.

Short summary:
When using more than 1 word for argument, I get no results in the view, but on previous version (7.x-3.3) it works just fine.
The SQL statement is the same in 7.x-3.5 and 7.x-3.3, so there must be something wrong in the source code of the Views module.

I have noticed that the SQL statement created from the module is slightly different from 7.x-3.5 to 7.x-3.3:

In 7.x-3.3, where it worked fine, the "GROUP BY score" is missing. In 7.x-3.5 this new GROUP statement appears. I don't know what this is for.

Title:Search terms: no results when using 2+ wordsSearch terms: nodes dont appear when using multiple arguments
Priority:Normal» Major

I really need help on this. I can't figure out where the origin of this problem is in the source code.

I have this same issue with views 7.x-3.5

Further testing shows a regression was introduced in 7.x-3.4. Multiple search terms works as intended with views 7.x-3.3

On a completely fresh Drupal installation, just installing Views 7.x-3.5 and Ctools, and creating some nodes, there it works fine. Only on my updated sites (from 7.x-3.3 to latest dev and stable) the problem occurs. I hope the bug is reproducable for the maintainers.
I searched for days the source code and have given up so far, no success :/. I need a tip where the trigger of this problem could be.

Please help.

I had the same problem with views 7.x-3.5, my exposed view search for my blog would work if one word is used, but if two words are used the views search does not work - no results.

I have gone back to 7.x-3.3 and it works fine again.

+1 Same error here
1 word all results expected (11)
2 words 9 results of 11
4 words 0 results of 11

Noticing the same issue.

Tracked it down to views_plugin_query_default.inc and the following code

if ($this->has_aggregate && (!empty($this->groupby) || !empty($non_aggregates))) {
$groupby = array_unique(array_merge($this->groupby, $non_aggregates));
foreach ($groupby as $field) {
$query->groupBy($field);
}
if (!empty($this->having) && $condition = $this->build_condition('having')) {
$query->havingCondition($condition);
}
}

For whatever reason, this code is adding an extra GroupBy for every field in the query that doesn't use an aggregate function. By doing this, the HAVING (COUNT(*) ...) added by views_handler_filter_search.inc is nullified because the results are no longer grouped solely by search_index.sid which is needed to determine how many word matches there are.

With the following change, to not add any extra fields to GroupBy if we have one already, I have it working for me(sorry, not good with patches).

if ($this->has_aggregate && (!empty($this->groupby) || !empty($non_aggregates))) {
  // don't mess with the original groupby if it was set
  if (!empty($this->groupby)) {
    $groupby = $this->groupby;
  }
  // use the non aggregates to create our groupby
  else {
    $groupby = $non_aggregates;
  }
  foreach ($groupby as $field) {
    $query->groupBy($field);
  }
  if ($condition = $this->build_condition('having')) {
    $query->havingCondition($condition);
  }
}

I didn't want to drop the $non_aggregates altogether, but I can't see why they'd be used anyway. No matter what sort of query you are running, adding extra fields to groupby will always add the chance the results will not be grouped properly.

Confirmed.

Ran into the same problem and also tracked it down to the - apparently superfluous - GROUP BY columns added by the query plugin itself (and therefore unresponsive to any hook-hacking), before I found this thread.

These additionally merged in columns:
$groupby = array_unique(array_merge($this->groupby, $non_aggregates));
seem to nullify the HAVING-clause and thus cause valid search results to disappear. And I cannot imagine what it should be good for... Getting rid of it in the above manner seems to fix the problem immediately.

Confirmed here as well, but different symptoms. The fix in #10 solved my issue.

I was having the problem where duplicate results were showing up for each matched result. I tracked it down to the new grouping by "score" added in 3.4 or 3.5.

score is expressed as SUM(search_index.score * search_total.count) in the query - this causes there to be a unique score field for each hit in the search index causing several duplicates to show up depending on the number of hits.

I also can't figure out why this was added, or what it tries to solve. Has anyone found the issue where this was introduced? I've been searching around, and can't find it.

Version:7.x-3.5» 7.x-3.x-dev
Status:Active» Needs review
StatusFileSize
new1.03 KB
PASSED: [[SimpleTest]]: [MySQL] 1,603 pass(es).
[ View ]

This works for me. Here's a patch for the changes in #10.

The problem is why the code exists? There must be a reason that someone added an extra GroupBy.
I'm worried about if patch from #13 will be commited, that this will cause another problem somewhere, because some little feature needs this extra GroupBy..

Hmm, let's see... The code in the current version (which is cleary wrong - and I would consider the missing search results for multiple terms a major bug, which should IMHO take priority over some hypothetical exotic minor feature "somewhere out there"...) groups by $this->groupby and $non_aggregates, while the patch groups either by $this->groupby or $non_aggregates. So the change will only matter in cases when $this->groupby as well as $non_aggregates are non-empty (the original condition).

A $this->groupby seems to be used mainly by the views/modules/search stuff (e.g. the said grouping by search_index.sid in views/modules/search/views_handler_argument_search.inc l. 94). The other cases I encountered (grepping for 'add_groupby()') were the $this->distinct feature (in views_plugin_query_default::query()) and the summary function (views/handlers/views_handler_argument.inc l. 886).

The former (in my experience) does not work in every case anyway (and if it does, it should take priority over everything else, if I understand that feature right...). And the latter does not seem to use the code in question...

As on the other hand $non_aggregates only become relevant, if "use_aggregation" is selected, the sole case in which problems could arise (exotic modules' features again not taken into account), would be when using a search_index based search with aggregation.

Since I never did that (and would consider it a rather untypical use of the search functionality), I tried to construct a view that uses the search index and aggregation, and noticed (for the first time!) that, as soon as you select "use aggregation", the field "search score" is no longer available (under "Fields -> add field") in the views UI. So the designers clearly seem to have tried to prevent that "pathological" case.

(It is nevertheless possible to construct the view by first adding the "search.score" field and the "search terms" exposed filters (and switch on aggregation afterwards) - but then the aggregation is spoiled anyway (since "GROUP BY search_index.sid" is added in any case), and again the patched version seems to be the only one that yields correct results for multiple search terms.)

For me the patch seems to work perfectly.

Status:Needs review» Needs work

Patch from #13 needs to be cleaned up due to Drupal's coding standards.

Is there a Views maintainer who can give further feedback on this? Thanks in advance.

salbertz, thank you very much for your opinion, but I'd like to have a Views maintainer taking a look over this, furthermore we need a maintainer who will commit the proper patch.

Sure. Would it be useful to change the issue status to catch a maintainer's attention - to "needs review" or "patch" or something like that? (I guess you know the conventions better than me to judge what would be the most appropriate.)

The issue does not seem to be too popular (maybe not too many using complex search condition in views exposed filters), though I'd consider it a rather heavy bug and would like to see it fixed in the nearer future...

Status:Needs work» Needs review

Hauptm, can you please identify the standards errors you found? It looks fine to to the coder module. You could make an argument that the comments should be proper sentence case, but I'm not seeing anything else. In any case, whoever commits can adjust that before committing.

I've been using this in production on an enterprise-level site since it was posted. Can anyone else confirm that this is RTBC? That will get the maintainers' attention.

Thanks.

You could make an argument that the comments should be proper sentence case, but I'm not seeing anything else.

Yes, this should be everything which has to be cleanded up on this.

I also confirm that patch from #13 solves my posted issue.

though I'd consider it a rather heavy bug and would like to see it fixed in the nearer future...

Agreed! Another reason could be that this bug is not immediately apparent but a critical one for production sites :-/

Many thanks to r_smylski, salbertz and awolfey for helping me out on this one, nice work!

The reason for the GROUP BY is most likely due to differences in MySQL and PostgreSQL as well as the different SQL standards. If you remove the functionality then Views will likely break when used on a PostgreSQL based site.

The functionality (of grouping by $non_aggregates) is not removed at all in the general case (as I tried to illustrate in #15) - only if "search terms" (or something else) provides a $this->group_by of its own, this takes precedence (in the patched version) over the grouping by $non-aggregates, instead of being combined with it (which still simply does not seem to make sense to me).

But I don't have any experience with PostgreSQL - if you (or anyone else) have: is it really imaginable, that the query produced by the old version ("GROUP BY search_index.sid, col1, col2, col3....." - including virtually any column used by the view, since one wouldn't typically use any aggregate function in a search) can be correct - or even more - necessary for PostgrSQL?

The issue is that mysql is spoiling us. Like mentioned in #20, true SQL requires that every column mentioned in the SELECT expression, that isn't an aggregate, needs to be listed in the GROUP BY. This is done so that data isn't lost from grouping things that might not be equal together.

I'm not sure if would be entirely possible, but I think the only real way to fix this would be to look closer at how the search module does keyword searching now in D7, since it would be working across all DBs, and see if that can be implemented for views.

You're right, thanks for the correction. You may be right about mysql as well...

Ok, so the $group_by is correct in strict SQL. Merely from the perspective of views' search mechanism (which in this case seems to be mysql-centric itself!) it is incorrect - since that mechanism exactly wants to accomplish, what PostgreSQL seems to prohibit, namely to aggregate by count(*) at any price (even at the price that, in your words, distinctive "data is lost") - to have the condition count(*) >= $num_search_terms met.

Then the only proper solution would indeed be a totally alternative approach, by reformulating the search condition (in views/modules/search/views_handler_filter_search.inc - I hope it can be done there, without involving core search issues).

I think I got it now. We were on the wrong track all the time - thanks to r_smylski for clarifying that (and reminding us of the dangers of MySQL-centrism ;-)

It seems, not all the additional GROUP BYs added by views_plugin_query_default::query() (that PostgreSQL needs) are detrimental to the search condition. It seems to be mainly the fact that "score" is added to $non_aggregates, that spoiles the HAVING-clause.

views/modules/search/views_handler_filter_search.inc seems to be absolutely correct in this respect: it accurately adds the information that "score" is an aggregate function in line 134:

$this->search_score = $this->query->add_field('', "SUM($search_index.score * $search_total.count)", 'score', array('aggregate' => TRUE));

What seems to go wrong, is that this information is utterly ignored by views_plugin_query_default::compile_fields() (in line 1208) and the field is added to $non-aggregates indifferently, as soon as it is found to be a function (i.e. to have no table):

// This is a formula, using no tables.
elseif (empty($field['table'])) {
   $non_aggregates[] = $fieldname;
   ...

If I change the code to check whether the field was marked as "aggregate" beforehand, everything seems to work fine:

elseif (empty($field['table'])) {
   if (empty($field['aggregate']) || $field['aggregate'] != true) {
      $non_aggregates[] = $fieldname;
   }

(Sorry, no experience with providing and testing patches so far. Anyway, maybe someone could verify that this could be an alternative solution, and that it works for other practical cases as well.)

Status:Needs review» Needs work

Changing to proper status, assuming #13 won't provide a global compatible solution.

#24 fixed it for me. I had nodes listed multiple times when using more than 1 word. Now it works as it should...

Just tested the fix from #24, works for me too.

StatusFileSize
new829 bytes
PASSED: [[SimpleTest]]: [MySQL] 1,603 pass(es).
[ View ]

Taking a stab at creating a patch for the fix. Here it is.

Status:Needs work» Needs review

StatusFileSize
new1.83 KB
PASSED: [[SimpleTest]]: [MySQL] 1,627 pass(es).
[ View ]

After looking over that suggested code, I don't believe it to be the best course of action. The problem isn't with views_plugin_query_default.inc, it is actually with how add_field() is being called from views_handler_filter_search.inc. There is code to catch if a field is actually a function, but in this case, it isn't being set up as one, and that's why there is the issue.

I've create a new patch, that adds the appropriate info to add 'score' as a field that is a function properly, and works without the change to views_plugin_query_default.inc.

There are 2 files in which the improper call was made, so the patch addresses both.

I would prefer the patch #28 above #30 for several reasons:

1) Not every function is an aggregate function. To my understanding the crucial distinction to decide whether a field should be added to $non_aggregates or not, should primarily depend on the 'aggregate'-flag (at least as far as I understand the inherent logic of the design - what else should that flag be good for?) and not on the assumption that every field containing a function will declare it explicitly.

2) patch #30 could - theoretically at least - have side effects for custom modifications via hooks in applications (if these modifications depend on the old field declaration). For example in my own application I wanted to "blur" the score a bit, to enable the secondary 'ORDER BY'-field to take over sort precedence if the score of two items is similar. To achieve that I modified the query in hook_views_query_alter():

        $view->query->fields['score'] = array(
          'field' => 'ROUND(SUM(search_index.score * search_total.count) * 100)',
          'alias' => 'score',
          'aggregate' => true,
       );

Whether that is very elegant or not - this simple approach is a somewhat natural thing to do, if one wants to modify the sort order. Anyone who has done similar things will not benefit from the fix (although in this case the change would not break anything), until he also adds the explicit 'function'-declaration:

        $view->query->fields['score'] = array(
          'field' => 'SUM(search_index.score * search_total.count * 100)',
          'alias' => 'score',
          'aggregate' => true,
          'function' => 'ROUND',
       );

3) This should also illustrate that the resulting logic is - at least - somewhat counter-intuitve. In order to prevent my field from being added to $non_aggregates and keep my custom scoring function, I have to declare the outer function 'ROUND' (which not even is an aggregate function), while the explicit setting "'aggregate' => true" is de facto irrelevant (and could have been omitted without making any difference). Anyone who does not know about the 'implicit logic' (and has not examined the source code) will fail if he tries to provide an aggregate function by simply writing it down in the "field"-attribute (like the previous author of the original code also did).

Even if the "function"-declaration may be better practice - according to the original logic, the above 'field-only' approach should be permitted as well - at least this is how I would interpret the comment in the (if this assumption is correct, still faulty!) code in compile_fields()

   // This is a formula, using no tables.
   elseif (empty($field['table'])) {
       $non_aggregates[] = $fieldname;
       ....

4) Last not least, the above location is where the bug became manifest: the line '$non_aggregates[] = $fieldname;' was apparently added in views-3.4 (maybe in conjunction with the "pure distinct" feature - probably not without reason, but unfortunately without any comment). But that this elseif-branch on the one hand seems to be explicitly intended for "formulas", but on the other ignores the aggregate-flag completely, still seems like an error to me.

So, if anything, than IMHO both patches should be combined. The fix #28 is "minimally invasive" (I can not imagine any side effects, besides fixing the bug introduced with 3.4) and will guarantee that queries altered via hook will work as expected, whereas the code in #30 (alone) will not help (or might even cause minor trouble) for applications with customized queries like in the example above - but may be cleaner from a general view and thus provide a better base for the future.

There might still be a "better" solution by examining and perhaps rewriting some of the 'if-else'-logic in views_plugin_query_default::compile_fields() or adding comments why something is done in this or that order, as some of the comments suggest that the section needs cleanup and/or commenting. But in the short term at least patch #28 should (and IMHO could without any dangers) be committed to fix the bug for now.

Patch at #28 solved my problem.

Thank you everybody.

If you don't feel like patching or hacking views then there's another way around this issue ...

<?php
/**
  * Implements hook_views_query_alter().
  */
function my_module_views_query_alter(&$view, &$query) {
  if (
$view->name == 'MY_SEARCH_VIEW') {
   
$query->fields['score']['table'] = 'foo';
  }
}
?>

Ok so there's no table 'foo' - and it's pretty disgusting (I'm probably on merlinofchaos's hit list now) - but from what I can see, because the score field is already an aggregate, views ignores the table field anyway - however because the table field exists, the group by loop respects it as an aggregate field and ignores it in the grouping.

When I perform this query alter, and analyze the views query output there's no mention of the table 'foo' anywhere - so there shouldn't be any SQL errors.

I must confess, that I didn't read the whole thread, but I experienced the same problem and I had to do something like this here to show any results, differing from the solution in #33:

<?php
/**
  * Implements hook_views_query_alter().
  */
function my_module_views_query_alter(&$view, &$query) {
  if (isset(
$query->fields['score'])) {
    unset(
$query->fields['score']);
  }
}
?>

Thanks Stefan. This workaround (#34) was just what I needed until a full fix makes its way into core.

Stefan, thank you very much, this saved me some headache!

Can you still sort by search relevancy using the solution post in 33? I think if you unset score it completely removes it from the view but I might be wrong.

@interactivejunky From your comment I think you're actually referring to #34 (and not your own post!), which you're right doesn't allow the sort by score. I'm doing a mix of #33 & #34 (as below) so as not to need to use the view name. Sorting still seems to work fine then. However, I noticed you can't display the score field doing this - displaying it always shows up as 0.

<?php
<?php
/**
  * Implements hook_views_query_alter().
  */
function my_module_views_query_alter(&$view, &$query) {
  if (isset(
$query->fields['score'])) {
   
$query->fields['score']['table'] = 'foo';
  }
}
?>

?>

So is patch from #30 a good one or not? Does it work with all databases? Can it be committed?

I think it's time to get this issue solved.
r_smylski, thanks for your patch.

As a temporary hack, NOT A SOLUTION, and an alternative to #34:

<?php
function my_module_query_alter(QueryAlterableInterface $query) {
  if (isset(
$query->alterTags['views']) && isset($query->alterTags['views_search_view_id'])) {
   
//Get a list of all 'group by' in the query
   
$fields =& $query->getGroupBy();
    unset(
$fields['score']);
  }
}
?>

This just removes score from the GROUP BY part of the SQL but leaves it in the SELECT. I guess this won't work for PostgreSQL. (I'm just getting up to speed on this issue)

Adapt the IF statement and set 'views_search_view_id' to the correct ID for your view.

I can still only affirm what I wrote in #24: 'score' simply must not be added to the 'GROUP BY' clause, but views_plugin_query_default::compile_fields() does exactly that since views-3.4. And that's an error that should be corrected, period. Which is very simple to do and which is exactly what r_smylski's patch #28 does (just tested it again with views-3.6, still works great).

Patch #30 works as well and is considered 'more correct' by r_smylski, while I hold the opposite view (for several reasons pointed out in #31). #28 depends on the field attribute 'aggregate', #30 on 'function' - so it seems to be a matter of personal preference. In my view #28 simply corrects the error introduced in 3.4 (after all, everything worked fine before that, didn't it?) and therefore should be considered the most natural and least problematic approach.

But of cause either solution is better than messing around with hook_views_query_alter and discarding the score functionality (including sorting) altogether – that can't be a serious option.

I'm good with the patch from #28 being used, as it fixes the changed functionality, and I'd love to see this finally put to bed. Plus, it would catch any other instances of the same usage by any other plugin or contrib that might use it as a basis for its functionality.

Works fine for me.

I'm not sure if #28 is a good solution, but I can also confirm I had the problems described here and #28 fixed it for me.

#34 is working for me as well, I'm using this workaround only on my view which is actually using search terms:

<?php
function my_module_views_query_alter(&$view, &$query) {
  if (
$view->name == 'my_search_view' && isset($query->fields['score'])) {
    unset(
$query->fields['score']);
  }
}
?>

Thanks for your help, guys. I dont' know if this is a good solution for a fix in the module, since this maybe can break sites using PostgreSQL or other dbms. But at least my nodes appear now.

When updating to views-3.4 we had a case where aggregate were used which caused a faulty query (too many group by's).
#28 was a good fix

I think we definetly need a Views maintainer who really knows what he's doing on this problem. Both #28 and #34 seem to fix this, but nobody knows exactly why and even worse nobody following this issue knows if these fixes could make other problems then. I hope a maintainer will look over this soon.

Just ran into this on a completely new site. Searching for 1 word will return results. Searching for more than one word returns no results. Seems like quite a major bug! I'm wary about the patches above since nobody seems sure what's going on exactly.

If I had to choose I would be really going with #30 given that it don't interfere with other views. If this workaround fork for people which just have a single argument as well, I think this would be ready to fly.

Thanks for your feedback, Daniel. #30 is working for me as well as #28. If you think #30 is the better fix than #28, then let's go for it. But I agree with salbertz in his statement from #31 that there might be new upcoming problems caused by custom extensions which were made between Views 3.4 and the current version. :/

Issue summary:View changes

This is a Viess module issue.

I'm still using the solution #24/#28 with the current views version (3.7) and can confirm that it still works great.

As repeated in #41: If you look at the code block cited in #24 (views-3.4 to present) and compare it to earlier versions, you will find that the present logic is simply flawed, while the "old logic" seemed perfectly reasonable. I can't see any reason for the change in 3.4 (and there is no comment to justify it) - the patch simply reverts to the "old logic", which still seems intuitive and correct - and worked perfectly.

If the problematic change introduced in 3.4 was really that vital to anything, the author should at least have left a comment, what it should be good for. Given the absence of such a hint, I still consider it some unintentional (or unfortunate - maybe the author wanted to 'clean up' a little bit... everyone of us knows that urge and its pitfalls, especially in conjunction with complex conditional branching) 'collateral damage'.

I tried both #28 and #30 and both seem to work pretty well.

I am going with #28 now, will report if I experience something strange.

Thanks!

Version:7.x-3.x-dev» 7.x-3.7

Problem still exists and patch in comment #30 still works!

Status:Needs review» Needs work

The last submitted patch, 30: search_multiple_terms-1615438-30.patch, failed testing.

The last submitted patch, 30: search_multiple_terms-1615438-30.patch, failed testing.

Status:Needs work» Needs review

Again: Please consider the patch #28 instead; as I stated in #51 and #41 it should be the better, simpler and safer solution, simply correcting an obvious mistake introduced in views-3.4.