I got the following error when using search api autocomplete with DB backend

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[42000]: Syntax error or access violation: 1463 Non-grouping field 'results' is used in HAVING clause: ...............

This is due to alias in having clause used in getAutocompleteSuggestions function in /search_api_db/src/Plugin/search_api/backend/Database.php file.

$db_query = $this->database->select($word_query, 't');
      $db_query->addExpression('COUNT(DISTINCT item_id)', 'results');
      $db_query->fields('t', array('word'))
        ->groupBy('word')
        ->having('results <= :max', array(':max' => $max_occurrences))
        ->orderBy('results', 'DESC')
        ->range(0, $limit);

I see the usage in the getFacets function as well in the above file where as in other places in the same file whole expression is used instead of the aliases. 

      if ($facet['min_count'] > 1) {
        $select->having('num >= :count', array(':count' => $facet['min_count']));
      }

According to this stackoverflow answer, http://dba.stackexchange.com/questions/50391/why-does-mysql-allow-having... , alias in having clause is enabled by default.

Why is there an error in D8 if it is default?

CommentFileSizeAuthor
#6 2783987-6--sql_standard_compliance.patch2.56 KBdrunken monkey
#6 2783987-6--sql_standard_compliance--tests_only.patch1.34 KBdrunken monkey
#3 having_error-2783987-3.patch1.17 KBzuhair_ak
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

zuhair_ak created an issue. See original summary.

zuhair_ak’s picture

zuhair_ak
He/Him
Primary language Malayalam
Location Dubai
CreditAttribution: zuhair_ak at Zyxware Technologies commented
Issue summary: View changes
zuhair_ak’s picture

zuhair_ak
He/Him
Primary language Malayalam
Location Dubai
CreditAttribution: zuhair_ak at Zyxware Technologies commented
FileSize
having_error-2783987-3.patch1.17 KB

I have added a simple patch replacing the aliases with the whole expression. Correct me if there is any error.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer and at Chapter Three commented

I'm wondering which DB you actually use. I had no problems with mysql personally, but this might be a problem on some more strict databases :(

zuhair_ak’s picture

zuhair_ak
He/Him
Primary language Malayalam
Location Dubai
CreditAttribution: zuhair_ak at Zyxware Technologies commented

I also have mysql db with default config. I thought D8 wasn't supporting alias in having.

This is from the same file which also has full expression - line 1963 onwards

        if (!$match_parts) {
          if ($mul_words) {
            $db_query->having('COUNT(DISTINCT t.word) >= ' . $var, array($var => $subs));
          }
          else {
            $db_query->having('COUNT(t.word) >= ' . $var, array($var => $subs));
          }
        }
        else {
          foreach ($keyword_hits as $alias) {
            $db_query->having("SUM($alias) >= 1");
          }
        }
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Status: Active » Needs review
FileSize
2783987-6--sql_standard_compliance--tests_only.patch1.34 KB
2783987-6--sql_standard_compliance.patch2.56 KB

Thanks a lot for reporting this and providing a patch!
Seems like exactly what we fixed back in #2305107: Fix standards-compliance of the generated SQL queries for Drupal 7. Never ported this since it seemed we already had fixed everything in D8 anyways (since that always uses strict mode), but apparently (and quite obvious, if you think about it) we forgot those places not covered by the tests – facets with min count greater than 1 (which we didn't test, for some reason) and autocompletion (which we can't really test, especially not before Autocomplete itself existed for D8).

The attached patch also adds a regression test for this, although it didn't really work for me (i.e., didn't fail without the fix) locally. But lets give it a try, and otherwise we at least have a test for higher min count.
(The "test_only" patch doubles as the interdiff, I didn't change the fix.)

The last submitted patch, 6: 2783987-6--sql_standard_compliance--tests_only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: 2783987-6--sql_standard_compliance.patch, failed testing.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented 
fail: [Browser] Line 495 of modules/search_api/src/Tests/IntegrationTest.php:
GET /checkout/core/install.php returned 0 (0 bytes).

looks like an unrelated fail, retesting.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
CreditAttribution: borisson_ as a volunteer and commented
Status: Needs work » Reviewed & tested by the community

Yep, it was unrelated, patch looks great.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
Status: Reviewed & tested by the community » Fixed

And the test seems to work, too, great (also on the other DBMSs)!
Thanks for reviewing! Committed.
And thanks a lot, again, to zuhair_ak for the patch!

(Still a bit troubling, that there are random fails again. :-/)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.