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
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:
- 3189880-use-square-brackets
changes, plain diff MR !187
Comments
Comment #3
mondrakeThis should cover tests for group 'Database'.
Comment #4
daffie commentedComment #5
mondrakeReplied in MR
Comment #6
daffie commentedUpdated the IS with a proposed solution.
Comment #7
daffie commentedComment #8
daffie commented@mondrake: What do you want to do with #3160271: Bracket-encapsulated field names for static queries in core/modules/{every module}?
Comment #9
mondrake#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
Comment #10
daffie commentedI will review more later.
Comment #11
daffie commentedJust found two files with queries that need square brackets added:
core/modules/node/src/Plugin/Action/AssignOwnerNode.phpandcore/modules/forum/src/ForumIndexStorage.php.Comment #13
Pooja Ganjage commentedComment #14
daffie commentedThere are still 15 unresolved threads, therefore back to needs work.
Comment #16
raman.b commentedComment #17
daffie commentedAll 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.
Comment #18
mondrakeThe 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.
Comment #19
daffie commentedComment #20
mondrakeMissing a few instances of
queryTemporary.Comment #21
mondrakeComment #22
daffie commented@mondrake: You where right, we forgot the temporary queries. I did a code base search and we got all instances.
Back to RTBC.
Comment #23
klausiThis 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.
Comment #24
mondrakeThe CR https://www.drupal.org/node/2986894 already covers this, although I agree it could be updated a little.
Comment #25
daffie commentedCreated an issue in the coding standards project. See: #3190494: Add a standard for "All identifiers should be quoted in database queries".
Back to RTBC.
Comment #26
alexpottI'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.
Comment #30
alexpottCrediting Megha_kundar, S_Bhandari, daffie, dww as per the issue summary.
Comment #31
alexpottCommitted d78e53b and pushed to 9.2.x. Thanks!
Comment #35
smustgrave commentedKnow 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.
Comment #36
mikelutzI 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.