votingapi_update_8303() fails with MySql 8

This is a duplicate of #3143266: Error: Call to undefined method Drupal\Core\Entity\EntityDefinitionUpdateManager::applyUpdates() in votingapi_update_8301(). Use the patch there.

Reopening this issue as it is *not* a duplicate of Error: Call to undefined method Drupal\Core\Entity\EntityDefinitionUpdateManager::applyUpdates() in votingapi_update_8301().

The problem in this issue is that in votingapi_update_8303() when Drupal tries to execute the line

\Drupal::database()->schema()->changeField('votingapi_result', 'function', 'function', $schema['columns']['value']);

it calls Schema::changeField, which itself calls Schema::fieldExists - and because the votingapi_result table's field that we're trying to change is called 'function' (without the quotes), the query "SELECT function FROM votingapi_result;" fails (whereas the query "SELECT 'function' FROM votingapi_result;" - with the table name 'function' encapsulated in single quotes - returns a result.)

To debug, notice that if you change the fieldExists method from

    try {
      $this->connection->queryRange("SELECT $column FROM {" . $table . "}", 0, 1);
      return TRUE;

to

    try {
      if ($column == 'function') {
        $column = '"' . $column . '"';
      }
      $this->connection->queryRange("SELECT $column FROM {" . $table . "}", 0, 1);
      return TRUE;
    }

you'll see that mysql returns a result, which is all this method needs.

It could be that the way forward is to choose a different column name for the 'function' column in the votingapi_result table.

Sorry, you're right, not a duplicate.

Added the related issue where hook_update_8303() was developed and discussed.

So it seems the root cause of this is that function is a reserved keyword in SQL ? That issue has been resolved for D9 but the fix will not be backported to D8. Probably the best thing to do going forward is to rename the field but until then we will still need to fix the existing update functions.

Why didn't this error show up in #3083473: The Function field needs to be updated.? Or even before that since Voting API has been using function for a very long time. I'm wondering if this is related to DB version or even PHP version, as MySql 8 (for example) has gotten stricter and I've seen this sort of conflict in other modules (e.g. the Group module, which uses the keyword group as a column name which causes D8 sites to break completely if you try to upgrade to MySql 8.)

@kaipipek, @COBadger, what version of PHP/MySql (or other DB) are you using?

@TR as the fates would have it, after restarting my Mac the db updates sailed through without issue. To answer your question directly, here are the versions that I'm running:

php: 7.3.11
mysql: 8.0.19

In the MySql documentation where it lists key words and reserved words, https://dev.mysql.com/doc/refman/8.0/en/keywords.html, it says:

FUNCTION; became reserved in 8.0.1

and

Reserved words are permitted as identifiers if you quote them as described in Section 9.2, “Schema Object Names”:

So that explains why this wasn't seen in earlier versions of Voting API and probably explains why no one caught this when the update hooks were written in #3083473: The Function field needs to be updated..

Hopefully this is confined to a MySql 8 problem, in which case the fix is to quote 'function' as you described - we should be able to do that as a bridge then work to rename the column (which will require another update function).

Yes - same problem here with mysql8 - But th eright way would be to use something different in voting_api instead of patching Schema.php in Core - right ?

But the right way would be to use something different in voting_api instead of patching Schema.php in Core - right ?

Right. My comment was that the fix needed here is to quote the column name, as required by the SQL specification now that MySql has made 'function' (and others, like 'system') a reserved word. Hopefully we can do this inside Voting API, but that's not guaranteed - this might need a core patch to fix. Quoting of keywords is already mostly fixed in D9 (except for the issue with Schema, see related issue), but as far as I know this will not be backported to D8. I've linked some related issues - there's a lot to read.

In the case of the Group module, mentioned above, they have taken the position that if you have upgraded to MySql 8 with Drupal 8 you are just screwed and that you should just not do that - they recommend to upgrade to Drupal 9 FIRST, before upgrading to MySql 8. And yes I know that's not a good solution - one of my sites was brought down because of that and can't be fixed, I had to re-install from backups.

My advice would be just don't use MySql 8 with Drupal 8. And DEFINITELY don't even try this with Drupal 7. Of course, that advice doesn't help you if you've already upgraded to MySql 8, and you're not likely to see any of the warnings before you upgrade.

Is there a workaround available?

Is there a workaround available?

@SamTheMan: There is no workaround yet, other than don't use MySql 8. That's probably the best solution until core Drupal fixes the problems with MySql 8 in core, because Voting API is not the only module affected.

If someone would like to try to write a patch to rename the 'function' base field (AND provide a hook_update_N() so that everyone's site gets updated properly) then we can consider it here.

We're still going to have the problem that a hook_update_N() to rename the base field will have to run AFTER votingapi_update_8303(), so it really won't help existing sites unless you are running MySql 5. So again, even with a fix, you're going to have to back off to MySql 5 before you can apply the fix and only then upgrade to MySql 8. This is just another reason why this should get fixed in core ...

Easy simple temporary solution to run update:
Upload files from votingapi-8.x-3.0-beta1 to /modules/contrib and then run drush updb. Everything works.

Now you can wait a real fix.

Here is my first attempt to change the 'function' field to 'agg_function'. Seems to work fine for my case.

Attempt to fix updates and tests

This looks like a good start.

However, votingapi_update_8303() can't be changed. You need a new update hook, votingapi_update_8305(), to rename the field.

Hi @TR
Updated patch please review.

No, #21 is no good, you can't just change the name. We need both the unchanged votingapi_update_8303() AND a new and different votingapi_update_8305().

(I corrected my above post, because there already IS a votingapi_update_8303(). The point is, we need a NEW update function - the actual number will depend on whether other update functions get added before this gets committed.)

Hi @TR
Updated patch please review.

NO

You must never change existing update functions. For any reason. Period.

There is no fix yet - Drupal 8 core does not fully support MySql 8 and it appears there is nothing the Voting API can do to get around this problem without a fix in core Drupal.

I'm encountering the same error here.

Votingapi module: Version: 8.x-3.0-beta2

Drupal Core: 8.9.2

@TR is there a temporary fix that we could use to overcome this error? Does updating to Drupal 9 fix this issues?

A few things I think might work:

First, use a version of core that has #3155563: select query should quote aliases which are reserved words in MySQL included - this was just committed today, so you're going to need 9.1.x-dev or 9.0.x-dev or 8.9.x-dev.

I think that patch will allow Voting API to run with MySql 8 in Drupal 8.9 or Drupal 9.

But that still doesn't address the original problem where update function fails. I think that will be fixed when #3130579: Make Drupal\Core\Database\Schema work with reserved keywords for naming is finished, but that hasn't been done yet and it may not get backported to Drupal 8.

So I think the only way to avoid the update function failing is to update your version of Voting API FIRST, and run updatedb BEFORE you upgrade MySql from 5 to 8. If you do this, then you should be able to run Voting API in D8.9+ and D9+ without a problem (assuming you're also using #3155563: select query should quote aliases which are reserved words in MySQL like I said above).

If you've already installed MySql 8 and can't go back, then the only way to deal with this is to make the changes from hook_update_8303() and hook_update_8304() manually using the command line or phpMyAdmin or something of that nature. If you try that, please post here the list of changes you had to make - I'm sure that would be useful to many people.

I can confirm that downgrading to Mysql Version 5.7.23 fixes this issue.

For others who would like a quick fix for this issue I'll post the steps I used to fix this on my Mac: (Pre-requisites are that you revert to a version before running drush updb)

1) Download Dbngn Mac software: https://dbngin.com/#

2) Install on your Mac.

3) Install MySql Workbench for Mac (if you don't have it already)

4) Using Dbngn easily create a MySql 5 instance on port 3307

5) Create an sql dump from your MySql 8 instance from your site.

6) Import the sql dump from #5 instance to your MySql 5 instance.

7) change your settings.php file to point to the Database from MySql 5 instance. Also, change the following:

  'username' => 'root',
  'password' => '', 
  'port' => '3307',

8) Run drush cr

9) Run drush updb

Presto! it works... Now just sql dump this database and import it to your Mysql 8 Version and everything should be working fine (Don't forget to change back your settings.php DB credentials)

I was able to do the update 8303 after apply the patch 23 but then fails on the update 8304, I got this error

Update started: votingapi_update_8304
> [error] Argument 2 passed to Drupal\Core\Field\FieldStorageDefinitionListener::onFieldStorageDefinitionUpdate() must implement interface Drupal\Core\Field\FieldStorageDefinitionInterface, null given, called in /app/public/html/core/lib/Drupal/Core/Entity/EntityDefinitionUpdateManager.php on line 233
> [error] Update failed: votingapi_update_8304

Any ideia why?

I can confirm the suggestion by @TR that #3130579: Make Drupal\Core\Database\Schema work with reserved keywords for naming fixes this issue (on Drupal 9.2.21 and Mysql 8).