SQL syntax error when using the search_keywords filter or argument

Oh wow, you work on the dark side of things :) Thank you!

As written below, it would be not a bad idea to have some actual test coverage

1. +++ b/core/modules/node/src/NodeViewsData.php
   @@ -380,8 +380,9 @@ public function getViewsData() {
    
            $data['node_search_dataset']['table']['join'] = array(
   -          'node_search_index' => array(
   +          'node_field_data' => array(
                'left_field' => 'sid',
   ...
                'field' => 'sid',
   

   You want to have the node_search_dataset handlers available on node views, so this change makes sense!

2. +++ b/core/modules/search/src/Tests/Plugin/views/SearchArgumentTest.php
   @@ -0,0 +1,79 @@
   +class SearchArgumentTest extends PluginTestBase {
   

   Its great to have some test!

3. +++ b/core/modules/search/src/Tests/Plugin/views/SearchArgumentTest.php
   @@ -0,0 +1,79 @@
   +   */
   +  public function testSearchFilter() {
   +    $view = Views::getView('test_node_view');
   ...
   +
   

   Do you think we want to have some actual test that the query results in a result we expect it to be?

Hm... I don't understand the NodeViewsData part of this patch in the context of the existing stuff in NodeViewsData. NodeViewsData is currently doing the following:
- Setting up an auto-join between node_search_index (an alias of search_index) to node_field_data.
- Setting up an auto-join between node_search_total and node_search_index
- Setting up an auto-join between node_search_dataset (an alias of search_dataset) to node_search_index
==> the patch modifies this join as follows:

        $data['node_search_dataset']['table']['join'] = array(
-          'node_search_index' => array(
+          'node_field_data' => array(
             'left_field' => 'sid',
+            'left_table' => 'node_search_index',
             'field' => 'sid',
             'table' => 'search_dataset',
             'extra' => 'node_search_index.type = node_search_dataset.type AND node_search_index.langcode = node_search_dataset.langcode',

The Search Keywords filter/argument are set up on the "node_search_index" table. So, I see why the modifications in the filter/argument plugins are needed:

       if (!$this->searchQuery->simple()) {
-        $search_dataset = $this->query->addTable('search_dataset');
+        $search_dataset = $this->query->addTable('node_search_dataset');

because the table to be added is definitely node_search_dataset, not search_dataset. But it should be fine for it to join to the node_search_index table as it was, because this is the table that is already part of the view if you are using the keywords filter?

I also do not understand why the SearchIntegrationTest in views/src/Tests didn't catch this? Oh, yeah, it's because of the keywords being entered. So, maybe we should add to that test as well, instead of or in addition to the tests here? Things we should probably be testing:
- And/Or/Multiple:
a)pizza OR sandwich
b) pizza sandwich OR cola
c) code>cola good
- Phrases:
d) "cola is good"
To do these, we should add another node to the test with title "cola is good" to the "pizza" and "sandwich" nodes in the current test.

Then:
a ==> returns pizza, sandwich nodes
b ==> returns all three
c) ==> returns cola
d) ==> returns cola

OK, here's a patch. Something like this anyway...

Hm on #9... perhaps there is another bug we need to investigate then, or maybe the test is failing in some other way. I don't have time to look into it today, sorry. What I would do to investigate is try a view with a search query/filter and try an OR query there and see if it works right. Perhaps the test has a bug; perhaps the code. Unsure which.

Regarding #8, I thought Views could do a join from table B to table C even if table A is the base table of the View? It must know how to do that in some cases, but maybe it has to be a relationship rather than an auto-join for that to work. Anyway. Onwards!

OK. I looked into this some more.

The logic in the Search filter/argument plugins for Views was flawed. It was basically saying "If this is a simple query, do it my way instead of how Search does it by default". But the queries didn't actually work, so anything but one keyword wasn't really working right. So I took this out of these two plugins: now it just defers to the SearchQuery class to figure out what the query should be, no matter what.

Then I was getting duplicates in the results. This turned out to be a problem in how the search score was added to the views query. So I fixed that, and also updated the documentation on the Views Sql query class for addField so hopefully no one else will need to make that mistake.

Then the test was also not right. It was basically double-url-encoding the argument page URLs. So I fixed that. Also added to the test so that it makes sure there is only 1 result for each node and no dups.

So... now it is working. Attached is a patch, containing the code fixes and my test additions. There is also an interdiff, containing changes from both my test patch in #6 and the code patch in #2. The tests from #2 are not included... wasn't sure we needed them? If we do they are easy to add back, but I think the new test coverage is sufficient. I also made a patch with just my test additions so we can look again and verify the tests fail without the code changes.

Whew!

Hm. The test does:

+    $this->drupalGet('test-arg/pizza OR sandwich');

and the correct search results are returned. So it was testing the argument... and the word "or" is not part of any of the nodes.

Are you sure about what you were testing, and are you sure you had the code patch applied?

Let me take a look...

Hm. On my test site from yesterday, which is where I developed the patch in #12, in the Views preview for the Arg page on that test view, when I do
pizza OR cola
I get this WHERE clause in the view:

 (node_search_dataset.data LIKE '% pizza %' ESCAPE '\\') OR 
 (node_search_dataset.data LIKE '% cola %' ESCAPE '\\') ))

This is correct (and working)... not sure where you're getting the other one?

I also got this when I edited the Filter page's search keywords filter and put "pizza OR cola" in that field (without the quotes).

Committed bc23d01 and pushed to 8.0.x. Thanks!

+++ b/core/modules/views/src/Tests/SearchIntegrationTest.php
@@ -7,6 +7,8 @@
+use Drupal\Component\Utility\String;

@@ -58,17 +66,56 @@ public function testSearchIntegration() {
+    $message = String::format('Link with label %label found once.', array('%label' => $label));

This is deprecated - need to use SafeMarkup::format() - fixed on commit.