Problem/Motivation

Static queries should have the field names bracket-encapsulated. In that way static queries cannot fail on a database, because a field name is a reserved keyword for the given database. For more info: https://www.drupal.org/node/2986894.

Proposed resolution

Bracket-encapsulated field names for all static queries located in the directory "core/tests/Drupal/KernelTests/Core/Database" and its sub-directories. Static queries can be found by doing a search for "->query(" and "->queryRange(" in the directory "core/tests/Drupal/KernelTests/Core/Database" and its sub-directories.

What needs to be changed:

From:
$connection->query('SELECT nid FROM {node} WHERE nid = :id', [':id' => 1']);

To:
$connection->query('SELECT [nid] FROM {node} WHERE [nid] = :id', [':id' => 1']);

Remaining tasks

TBD

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

TBD

CommentFileSizeAuthor
#9 interdiff_5-9.txt4.4 KBDeepak Goyal
#9 3152390-9.patch54.06 KBDeepak Goyal
#5 interdiff_3-5.txt1.4 KBmunish.kumar
#5 3152390-5.patch50.8 KBmunish.kumar
#3 3152390-3.patch51.38 KBmunish.kumar
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Unassigned » munish.kumar
munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Active » Needs review
FileSize
3152390-3.patch51.38 KB

Please review the patch as I have made changes in the core/tests/Drupal/KernelTests/Core/Database to make static queries field names bracket-encapsulated.

Status: Needs review » Needs work

The last submitted patch, 3: 3152390-3.patch, failed testing. View results

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
FileSize
3152390-5.patch50.8 KB
interdiff_3-5.txt1.4 KB

Fixed the test cases issue.

munish.kumar’s picture

munish.kumar CreditAttribution: munish.kumar as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Assigned: munish.kumar » Unassigned
Status: Needs work » Needs review
daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work

Patch looks good.

  1. +++ b/core/tests/Drupal/KernelTests/Core/Database/DeleteTruncateTest.php
@@ -22,7 +22,7 @@ class DeleteTruncateTest extends DatabaseTestBase {
+    $pid_to_delete = $this->connection->query("SELECT * FROM {test_task} WHERE [task] = 'sleep' ORDER BY tid")->fetchField();

    The field "tid" also needs square brackets. 

  2. +++ b/core/tests/Drupal/KernelTests/Core/Database/BasicSyntaxTest.php
@@ -37,7 +37,7 @@ public function testConcatLiterals() {
+      'SELECT CONCAT(:a1, CONCAT([job], CONCAT(:a2, CONCAT(age, :a3)))) FROM {test} WHERE [age] = :age', [

    The field "age" also needs square brackets.

  3. There are fields that need square brackets in Drupal\KernelTests\Core\Database\BasicSyntaxTest::testConcatWsFields().
  4. There are fields that need square brackets in Drupal\KernelTests\Core\Database\SchemaTest::testSchema().
  5. There are fields that need square brackets in Drupal\KernelTests\Core\Database\SelectTest::testRandomOrder().
Deepak Goyal’s picture

Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Unassigned » Deepak Goyal
Deepak Goyal’s picture

Deepak Goyal CreditAttribution: Deepak Goyal at Srijan | A Material+ Company for Drupal India Association commented
Assigned: Deepak Goyal » Unassigned
Status: Needs work » Needs review
FileSize
3152390-9.patch54.06 KB
interdiff_5-9.txt4.4 KB

Hi @daffie
Made changes as you suggested please review.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Reviewed & tested by the community

All field names have now square brackets around them.
All code changes look good to me.
For me it is RTBC.

Thank you for working on this: @munish.kumar and @deepak_goyal.

daffie’s picture

daffie CreditAttribution: daffie commented
Category: Task » Bug report
Issue tags: +Bug Smash Initiative

The parent issue is a "bug report", therefore this issue is one too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Title: Bracket-encapsulated field names for static queries in core/tests/Drupal/KernelTests/Core/Database » [backport] Bracket-encapsulated field names for static queries in core/tests/Drupal/KernelTests/Core/Database
Version: 9.1.x-dev » 9.0.x-dev

Committed dbd9350 and pushed to 9.1.x. Thanks!

Will backport to 9.0.x once the branch is open again.

  • alexpott committed dbd9350 on 9.1.x 
    Issue #3152390 by munish.kumar, Deepak Goyal, daffie: Bracket-...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Title: [backport] Bracket-encapsulated field names for static queries in core/tests/Drupal/KernelTests/Core/Database » Bracket-encapsulated field names for static queries in core/tests/Drupal/KernelTests/Core/Database
Status: Reviewed & tested by the community » Fixed

Cherry-picked to 9.0.x

  • alexpott committed 749984b on 9.0.x 
    Issue #3152390 by munish.kumar, Deepak Goyal, daffie: Bracket-...

Status: Fixed » Closed (fixed)

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