Bracket-encapsulated field names for static queries in core/modules/{every module}

Thank you @Megha_kundar for working on this. Your patch looks good, only there is some work to be done:

1. +++ b/core/modules/search/src/SearchIndex.php
   @@ -298,7 +298,7 @@ public function updateWordWeights(array $words) {
   -      $result = $this->replica->query("SELECT t.word AS realword, i.word FROM {search_total} t LEFT JOIN {search_index} i ON t.word = i.word WHERE i.word IS NULL");
   +      $result = $this->replica->query("SELECT [t.word] AS realword, [i.word] FROM {search_total} t LEFT JOIN {search_index} i ON t.word = i.word WHERE [i.word] IS NULL");
   

   Field names like t.word should be bracket encapsulated as [t].[word]. Also you forgot to bracket encapsulate the fields: "i.word" and "t.word" in the left join part of the query.

2. +++ b/core/modules/search/src/SearchIndex.php
   index 3226fde67a..9e015b9d2f 100644
   --- a/core/modules/search/tests/src/Functional/SearchNodeUpdateAndDeletionTest.php
   
   +++ b/core/modules/search/tests/src/Functional/SearchNodeUpdateAndDeletionTest.php
   index 9eb111e124..ad88ab7518 100644
   --- a/core/modules/system/tests/src/Functional/Database/TemporaryQueryTest.php
   
   +++ b/core/modules/system/tests/src/Functional/Database/TemporaryQueryTest.php
   index d51b224430..340486fd5e 100644
   --- a/core/modules/taxonomy/tests/src/Functional/TermIndexTest.php
   

   The change to the query in these files fall outside this issue. From the issue summary: "Static queries can be found by doing a search for "->query(" and "->queryRange(" in the directory "core/modules/{every module}" and its sub-directories, but not in the directory "core/modules/{every module}/tests"."

3. +++ b/core/modules/tracker/tracker.module
   @@ -212,7 +212,7 @@ function _tracker_add($nid, $uid, $changed) {
   +  $node = $connection->query('SELECT [nid], [status], [uid], [changed] FROM {node_field_data} WHERE [nid] = :nid AND [default_langcode] = 1 ORDER BY changed DESC, status DESC', [':nid' => $nid])->fetchObject();
   

   You forgot to bracket encapsulate the fields: "changed" and "status" in the order by part of the query.

4. There are still a lot of queries that have field names that need to be bracket encapsulated. Like in "core/modules/aggregator/src/FeedStorage.php" and "core/modules/ban/src/BanIpManager.php".

Review of the patch from comment #8. Patch looks better.

1. +++ b/core/modules/aggregator/src/FeedStorage.php
   @@ -16,7 +16,7 @@ class FeedStorage extends SqlContentEntityStorage implements FeedStorageInterfac
   +    return $this->database->query('SELECT [fid] FROM {' . $this->getBaseTable() . '} WHERE [queued] = 0 AND [checked + refresh] < :time AND [refresh] <> :never', [
   

   This is wrong [checked + refresh]. It should be: [checked] + [refresh].

2. +++ b/core/modules/comment/src/CommentStatistics.php
   @@ -154,7 +154,7 @@ public function create(FieldableEntityInterface $entity, $fields) {
   +    return $this->database->query('SELECT MAX(comment_count) FROM {comment_entity_statistics} WHERE [entity_type] = :entity_type', [':entity_type' => $entity_type])->fetchField();
   

   Can we change MAX(comment_count) to MAX([comment_count]).

3. +++ b/core/modules/comment/src/Plugin/views/field/NodeNewComments.php
   @@ -155,9 +155,9 @@ public function preRender(&$values) {
   +      $result = $this->database->query("SELECT [n].[nid], COUNT(c.cid) as num_comments FROM {node} n INNER JOIN {comment_field_data} c ON [n].[nid] = [c].[entity_id] AND [c].[entity_type] = 'node' AND [c].[default_langcode] = 1
   

   Can we change COUNT(c.cid) to COUNT([c].[cid]). Same for "h.timestamp".

4. +++ b/core/modules/dblog/src/Controller/DbLogController.php
   @@ -248,7 +248,7 @@ public function overview() {
   +    $dblog = $this->database->query('SELECT [w].[*], [u].[uid] FROM {watchdog} w LEFT JOIN {users} u ON [u].[uid] = [w].[uid] WHERE [w].[wid] = :id', [':id' => $event_id])->fetchObject();
   

   The char "*" does not to have brackets.

@Megha_kundar: Could you add an interdiff.txt with you patch. It makes reviewing the changes you made in the patch a lot easier.

Review of the patch from comment #9. Patch looks better.

My remark #10.3 also applies to your patch.

I am missing a query that need bracket encapsulation in "core/modules/node/src/Plugin/views/argument/Vid.php".

@S_Bhandari: Could you add an interdiff.txt with you patch. It makes reviewing the changes you made in the patch a lot easier.

@Megha_kundar: Could you take the patch from comment #9 and work from that. I am now reviewing both you individual patch work. It would be a lot easier for me when we work together on the same patch. I am choosing his patch, because you patch is missing a couple of queries.

Almost ready for the view from @alexpott.

1. +++ b/core/modules/comment/src/Plugin/views/field/NodeNewComments.php
   @@ -155,9 +155,9 @@ public function preRender(&$values) {
   +        AND [c].[changed] > GREATEST(COALESCE(h.timestamp, :timestamp1), :timestamp2) AND [c].[status] = :status GROUP BY [n].[nid]", [
   

   Can we change GREATEST(COALESCE(h.timestamp, to GREATEST(COALESCE([h].[timestamp],.

2. I found an other query that needs bracket encapsulation in "core/modules/node/src/Plugin/Action/AssignOwnerNode.php".

The patch is almost ready for a review by @alexpott. Just 2 remarks left:

1. +++ b/core/modules/history/history.module
   @@ -81,7 +81,7 @@ function history_read_multiple($nids) {
   +  $result = \Drupal::database()->query('SELECT [nid], timestamp FROM {history} WHERE [uid] = :uid AND [nid] IN ( :nids[] )', [
   
   +++ b/core/modules/locale/locale.module
   @@ -799,7 +799,7 @@ function locale_translation_get_file_history() {
   +    $result = \Drupal::database()->query('SELECT [project], [langcode], [filename], [version], [uri], timestamp, [last_checked] FROM {locale_file}');
   

   Can we add brackets to the field "timestamp".

2. From the interdiif.txt file:

   +++ a/core/modules/node/src/Plugin/views/argument/Vid.php
   @@ -68,7 +68,7 @@
   +    $results = $this->database->query('SELECT nr.vid, nr.nid, npr.title FROM {node_revision} nr WHERE nr.vid IN ( :vids[] )', [':vids[]' => $this->value])->fetchAllAssoc('vid', PDO::FETCH_ASSOC);
   -    $results = $this->database->query('SELECT [nr].[vid], [nr].[nid], [npr].[title] FROM {node_revision} nr WHERE [nr].[vid] IN ( :vids[] )', [':vids[]' => $this->value])->fetchAllAssoc('vid', PDO::FETCH_ASSOC);
   

   Why was this change removed? Can you put it back in.

After talking about this issue with @alexpott on Slack. He wanted to have all queries that are, according to him in the "critical path", to have the field be bracket encapsulated. All the other queries should be changed to dynamic queries. The queries in the current patch have all fields been bracket encapsulated. It is now up to @alexpott to determine which queries are in the "critical path" and which are not. For that am I adding the tag "Needs framework manager review" and changing the status to RTBC.

Thank you for your review @dww.

For #24.1.A and #24.1.D: This is something we do not do at the moment, but maybe that is something we start doing.

For #24.1.B and #24.4: Already fixed in #3159982: AS keyword should be capitalised in SQL queries.

For #24.1.C: We are adding quotes to field names. The part "n." is part of the field name. I have also no problem with doing "n.[nid]" as you are suggesting.

For #24.2 and #24.3: Could be seen as out of scope, but I have no problem in doing that in this issue.

+++ b/core/modules/dblog/src/Controller/DbLogController.php
@@ -248,7 +248,7 @@ public function overview() {
+    $dblog = $this->database->query('SELECT w.*, [u].[uid] FROM {watchdog} w LEFT JOIN {users} u ON [u].[uid] = [w].[uid] WHERE [w].[wid] = :id', [':id' => $event_id])->fetchObject();

+++ b/core/modules/forum/src/ForumIndexStorage.php
@@ -32,7 +32,7 @@ public function __construct(Connection $database) {
+    return $this->database->queryRange("SELECT [f].[tid] FROM {forum} f INNER JOIN {node} n ON [f].[vid] = [n].[vid] WHERE [n].[nid] = :nid ORDER BY [f].[vid] DESC", 0, 1, [':nid' => $node->id()])->fetchField();

@@ -92,14 +92,14 @@ public function update(NodeInterface $node) {
+    $count = $this->database->query("SELECT COUNT([cid]) FROM {comment_field_data} c INNER JOIN {forum_index} i ON [c].[entity_id] = [i].[nid] WHERE [c].[entity_id] = :nid AND [c].[field_name] = 'comment_forum' AND [c].[entity_type] = 'node' AND [c].[status] = :status AND [c].[default_langcode] = 1", [

+++ b/core/modules/node/src/Plugin/views/argument/Vid.php
@@ -68,7 +68,7 @@ public static function create(ContainerInterface $container, array $configuratio
+    $results = $this->database->query('SELECT [nr].[vid], [nr].[nid], [npr].[title] FROM {node_revision} nr WHERE [nr].[vid] IN ( :vids[] )', [':vids[]' => $this->value])->fetchAllAssoc('vid', PDO::FETCH_ASSOC);

+++ b/core/modules/user/src/Authentication/Provider/Cookie.php
@@ -71,14 +71,14 @@ protected function getUserFromSession(SessionInterface $session) {
+        ->query('SELECT * FROM {users_field_data} u WHERE [u].[uid] = :uid AND [u].[default_langcode] = 1', [':uid' => $uid])

Could we not add square brackets to alias names. Do WHERE u.[uid] = :uid instead of WHERE [u].[uid] = :uid. All the other changes look good. Points #24.2 and #24.3 are still open.

Postponed on #3189880: Use square brackets syntax in sql queries.

This issue is fixed in #3189880: Use square brackets syntax in sql queries. People in this issue got commit credits in the other issue.