Problem/Motivation

It would be useful to use the square brackets syntax to mark identifiers also in expressions. It's not a significant change for core since the Database API already escapes the identifiers with the relevant quote characters. For Oracle, viceversa, it's pretty fundamental since the DBMS uppercases all the identifiers if they are not quoted, and that makes it impossible to use unquoted lowercase identifiers in expressions when you have created db assets with quoted lowercase field names.

Proposed resolution

All field names and all alias names must be encapsulated with square brackets.

Remaining tasks

TBD

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

Commit credits

Could the people who worked on #3160271: Bracket-encapsulated field names for static queries in core/modules/{every module} also get commit credits for this issue.

Issue fork drupal-3189880

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Assigned: mondrake » Unassigned
Status: Active » Needs review

This should cover tests for group 'Database'.

mondrake’s picture

Replied in MR

daffie’s picture

Issue summary: View changes
Issue tags: +Contrib database driver

Updated the IS with a proposed solution.

daffie’s picture

Issue summary: View changes
daffie’s picture

mondrake’s picture

#8 sorry I did not realize about that issue when I opened this. I do not know, maybe let's postpone that on this one to cleanup if anything left then - it seems to be that one would be needing a reroll anyway

daffie’s picture

Status: Needs review » Needs work

I will review more later.

daffie’s picture

Just found two files with queries that need square brackets added: core/modules/node/src/Plugin/Action/AssignOwnerNode.php and core/modules/forum/src/ForumIndexStorage.php.

Pooja Ganjage made their first commit to this issue’s fork.

Pooja Ganjage’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Needs work

There are still 15 unresolved threads, therefore back to needs work.

raman.b made their first commit to this issue’s fork.

raman.b’s picture

Status: Needs work » Needs review
daffie’s picture

Title: Use square brackets syntax in expressions » Use square brackets syntax in sql queries
Status: Needs review » Reviewed & tested by the community

All my threads are resolved.
I did code bas searches for: "->query(", "->queryRange(", "->expression(", "->addExpression(", "->join(", "->leftJoin(", "->innerJoin(" and "->where(".
All code changes look good to me.
As a reviewer am I not allowed to mark threads as resolved.
For me is it RTBC.

mondrake’s picture

The CR for #2986452: Database reserved keywords need to be quoted as per the ANSI standard covers this already, although it could use some edit to mention rhis applies also to joins.

daffie’s picture

Issue summary: View changes
mondrake’s picture

Status: Reviewed & tested by the community » Needs work

Missing a few instances of queryTemporary.

mondrake’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Reviewed & tested by the community

@mondrake: You where right, we forgot the temporary queries. I did a code base search and we got all instances.

Back to RTBC.

klausi’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

This is quite a significant change how we write SQL queries in Drupal and I think this fails the documentation gate right now. Should we update https://www.drupal.org/docs/develop/standards/sql-coding-conventions before or after we merge this? It does not mention [] brackets at all. If we want to do this in the future then we need to clearly document this as best practice.

Do we need to create a Coding Standards issue for that at https://www.drupal.org/project/coding_standards or do we want to work on the proposed documentation page change here? We could prepare the update for the documentation in the issue summary.

We also need to prepare a change record for this new SQL Coding standard.

mondrake’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record

The CR https://www.drupal.org/node/2986894 already covers this, although I agree it could be updated a little.

daffie’s picture

Status: Needs review » Reviewed & tested by the community

Created an issue in the coding standards project. See: #3190494: Add a standard for "All identifiers should be quoted in database queries".
Back to RTBC.

alexpott’s picture

I've updated the documentation on https://www.drupal.org/docs/develop/standards/sql-coding-conventions#res... to mention square brackets. Much of the documentation on that page is woefully out-of-date even for Drupal 7 but it's now a touch better.

alexpott credited dww.

alexpott’s picture

Issue summary: View changes

Crediting Megha_kundar, S_Bhandari, daffie, dww as per the issue summary.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed d78e53b and pushed to 9.2.x. Thanks!

  • alexpott committed d78e53b on 9.2.x
    Issue #3189880 by mondrake, raman.b, Pooja Ganjage, daffie, Megha_kundar...

Status: Fixed » Closed (fixed)

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

smustgrave’s picture

Know this ticket has been closed but how do you use brackets for regex? Trying to write a custom OrderBy expression that works in sql but fails in Drupal. (REGEXP_SUBSTR($field, '[0-9]+') != '') gets converted to (REGEXP_SUBSTR($field, "0-9"+') != '') which doesn't work.

mikelutz’s picture

I banged my head on this for an hour this morning, just to figure out it was Drupal killing my brackets and not a problem with my regex...

The answer is to add the 'allow_square_brackets' option to the query

\Drupal::database()->select('table', 't', ['allow_square_brackets' => TRUE]);

Unfortunately, my regex was `'[0-9]{2,}'`, and I don't see a way to preserve squiggly brackets as those are used as table placeholders by the query builder. I'll get around it with `'[0-9][0-9]+` for today, but it's good to realize I can't use squiggly brackets in regex.