Replace all calls to db_query, which is deprecated

reroll against 8.6.x

Maybe this patch should be chunked into the smaller patches? Like it was with #2924538: [META] Remove all usages of drupal_set_message and drupal_get_messages

This patch is too big, so I will attach smaller patches in the followups:

• #2939385: Replace all calls to db_query [core/includes]
• #2939386: Replace all calls to db_query [core/lib]
• #2939387: Replace all calls to db_query [core/test/../Database]
• #2939389: Replace all calls to db_query [core/test]
• #2939390: Replace all calls to db_query [core/modules/{a-d}]
• #2939392: Replace all calls to db_query [core/modules/{f-n}]
• #2939393: Replace all calls to db_query [core/modules/{p-s}]
• #2939394: Replace all calls to db_query [core/modules/{t-v}]

Updated issue summary

Returning to the usual task issue. Regarding https://www.drupal.org/core/scope#examples we should follow good scope.
So as @alexpott had mentioned in the followup issues, to reduce the complexity of maintaining huge patch, it should be scripted.

Initial replacements.

# Replace the occurrences in code.
find ./core/lib -type f -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/tests -type f -not -path "./core/tests/Drupal/KernelTests/Core/Database/*" -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/tests/Drupal/KernelTests/Core/Database -type f -not -path "./core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php" -exec sed -i -r 's/db_query\(/$this->connection->query\(/g;' {} \;
find ./core/modules -type f -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;

Working on scripting this patch. Here's what I have so far:

export find_string='db\_query[(]';
export replace_string='\\Drupal\:\:database[(][)]-\>query\(';
export regex='s/'$find_string'/'$replace_string'/g';
git grep -l 'db_query(' | xargs sed -i $regex;

It replaces all instances, which is unfortunate, because that replaces the `function db_query(` in database.inc as well. Once I figure out how to quickly revert that one change, I'll update this issue and close child issues.

EDIT: Whoops, looks like @voleger beat me to it (I worked on this last week, but just now getting around to posting it). His script looks more comprehensive, especially handling the object oriented test, etc.

Also, the script should avoid replacing function calls at tests for the function itself like Drupal\KernelTests\Core\Database\QueryTest

Next iteration.
Bulk replacements that cover all places where db_query function is used.

# Replace the occurrences in code.
find ./core/lib -type f -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/tests -type f -not -path "./core/tests/Drupal/KernelTests/Core/Database/*" -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/tests/Drupal/KernelTests/Core/Database -type f -not -path "./core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php" -not -path "./core/tests/Drupal/KernelTests/Core/Database/QueryTest.php" -exec sed -i -r 's/db_query\(/$this->connection->query\(/g;' {} \;
find ./core/modules -type f -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/tests/Drupal/KernelTests/Core/Database/LoggingTest.php -type f -exec sed -i -r "s/call_user_func_array\('db_query'/call_user_func_array\([\$this->connection, 'query']/g;" {} \;
find ./core/tests/Drupal/KernelTests/Core/Database/SchemaTest.php -type f -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/includes/form.inc -type f -exec sed -i -r 's/db_query\(/Database::getConnection\()->query\(/g;' {} \;
find ./core/scripts/ -type f -exec sed -i -r 's/db_query\(/\\Drupal::database\()->query\(/g;' {} \;
# Start using \Drupal::database() or inject service
find ./core/modules/tracker -type f -exec sed -i -r 's/Database::getConnection\(/\\Drupal::database\(/g;' {} \;

The next step should be to find out files where Database class not used in current namespace and then fix related errors.
That step also should be scripted.

Also I guess we shouldn't remove db_query function name from the next array in core/lib/Drupal/Core/Utility/Error.php:50 file:

      // The first element in the stack is the call, the second element gives us
      // the caller. We skip calls that occurred in one of the classes of the
      // database layer or in one of its global functions.
      $db_functions = ['db_query', 'db_query_range'];

Let's see which files require additional attention.
Here's scripted patch from #22 script.
No interdiff. Patch was scripted.

Try manually to add usages of required Database class in files

Reupload interdiff

Fixed comments

Need some review of #27 and ideas on how to fix the Migration and Logger tests.

I think better to get in db_delete & db_merge then patch will be smaller because in some places there will be $connection variable

Agreed
Postponed on:
#2848137: Replace all calls to db_update, which is deprecated
#2850033: Replace all calls to db_delete, which is deprecated
#2873684: Replace all calls to db_select, which is deprecated

There is a lot of calls to db_query('SELECT COUNT(*) ...');.

Find for example just those in test code with find . -type f -name '*Test.php' -exec grep -Ei "QUERY.*COUNT.*\(\*\)" {} \+.

We might convert those to $injected_database->select(...)->countQuery() and then the conversion here would be more digestible...

Opened #2994904: Convert query('SELECT ... FROM {xxx}') to select('xxx')->... in tests.

#2848137: Replace all calls to db_update, which is deprecated is fixed.

Still postponed on:
#2853118: Replace all calls to db_insert, which is deprecated
#2850033: Replace all calls to db_delete, which is deprecated
#2873684: Replace all calls to db_select, which is deprecated

#2850033: Replace all calls to db_delete, which is deprecated fixed

Postopend on #2873684: Replace all calls to db_select, which is deprecated

Also, make it postponed on #2994904: Convert query('SELECT ... FROM {xxx}') to select('xxx')->... in tests that will convert some query into select calls

db_select #2873684: Replace all calls to db_select, which is deprecated is gone

#3001216: Use the database.replica service where appropriate has gone

I do not think this is blocked on #2994904: Convert query('SELECT ... FROM {xxx}') to select('xxx')->... in tests - it would help for sure but can do without.

I agree those issues are not the blockers no more.

Scripting of the patch will not help because it also become outdated and it's complicated to script adding missing namespaces, loop construction situations, ::getConnection calls optimization, connections to the replica database.

Added rerolled patch.
Skipped D6 and D7 scripts, because they should be executed against related codebase version.
Skipped documentation updates for db_* functions, because they are deprecated and usually linked to deprecated db_* functions that are also deprecated.

Yep, as other conversions landed let's get this one next before "count" issue in bikeshed

I'm not sure about replacement in LoggingTest. Trying not pass test property in the callable array.

We need #2999962: Unify Database/Log::findCaller and Utility/Error::getLastCaller before we can address the LoggingTest issue.

As logger already works not properly, is it possible to fix an issue related to the LoggingTest in the followup issue? It will unblock a huge amount of replacements.

As discussed in #2994904: Convert query('SELECT ... FROM {xxx}') to select('xxx')->... in tests, we probably need to have separate issues to convert db_query usages on entity related tables to use the Entity Query API, since neither converting to Connection::query nor to Connection::select seems the right thing to do in these cases.

Since amounts replacements in this issue much more than replacements in #3012599: Replace all db calls to aggregator_feed and aggregator_item tables with Entity APIs and this issue already blocked by #2999962: Unify Database/Log::findCaller and Utility/Error::getLastCaller, I suggest postponing this issue also on #3012599: Replace all db calls to aggregator_feed and aggregator_item tables with Entity APIs as there are no blockers.

Oops wrong issue

Fixed #3012599: Replace all db calls to aggregator_feed and aggregator_item tables with Entity APIs
Still postponed on #2999962: Unify Database/Log::findCaller and Utility/Error::getLastCaller

Just reroll. Still blocked by #2999962: Unify Database/Log::findCaller and Utility/Error::getLastCaller

IMHO, we should also decide on the fate of #2994904: Convert query('SELECT ... FROM {xxx}') to select('xxx')->... in tests before unpostponing - with that in, this patch would be 100kb+ smaller and much easier to review.

@voleger: I think the #2999962: Unify Database/Log::findCaller and Utility/Error::getLastCaller is tricky to resolve, is this really a hard blocker, can't we just keep those duplicated pieces of code and update them separately?

Blocker for #3049380: Complete deprecation of _db_get_target() function

I'm still not sure about blocking issue and it just feels hard to review for me.

Why don't we just fix it like this? Took me a while to understand it, but the trick as much as I understand it really is to find the last call of the backtrace that's still *inside* the database system and then report the next, but the line/file is actually on this part.

We already check for the db_* function not being the next, so as far as I see, we just need to change the namespace check to the same. That means it behave consistently, whether or not the call goes through db_query(). Also added explicit tests for that.

Nice. +1 for the fix.
Cleanup of unused use definitions. Updated deprecation messages to fix CS issues.
Fixed typo and comments CS issue.

Looks good!

+++ b/core/tests/Drupal/KernelTests/Core/Database/LoggingTest.php
@@ -13,19 +13,30 @@ class LoggingTest extends DatabaseTestBase {
+    db_query('SELECT name FROM {test} WHERE age > :age', [':age' => 25])->fetchCol();

But why not use the $this->connection->query?

This line will test the logging of procedural call of the db_query function as we introduce the fix for the logging service. BTW this test in the legacy group, so that line will be removed from drupal:9.0.0.

Thanks @voleger!

Still applies and test is still green, looks like another random test fail in MediaLibraryTest?

All looks good so setting back to RTBC! Tests failure is not related (see #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest).

Updated IS

See #2901792: Disable all animations in Javascript testing

Needs a re-roll.

Two conflicts in EntityApiTest. Adding a patch diff to show that there are only context changes compared to the last patch and setting back to RTBC.

Rerolled
Updated changes in db_query function definition. See #3000931: Connection::query() does not support 'target' option

And actual rerolled patch

adding review credit for mondrake for co-ordination and management tasks

Committed 275c864 and pushed to 8.8.

🎉