Posted by dawehner on January 6, 2013 at 12:06am
18 followers
Jump to:
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | database system |
| Category: | task |
| Priority: | normal |
| Assigned: | Crell |
| Status: | closed (duplicate) |
Issue Summary
Based on some twitter-discussions and issues like #1817672: Use db_query() instead of db_select() where appropriate #1805996: [META] Views in Drupal Core
I'm asking now when should we use db_select and when db_query.
In Drupal 7 the policy seemed to be: use db_query only when it's performance critical code.
Based on some comments some people seems to think that in Drupal 8 you should use db_query unless:
- You have a dynamic query like a view
- You need some kind of access mechanism like node_access
- You want to do fancyness like pagerDecorators.
Comments
#1
SQL strings are the evil. Use them when must but never else. Simple enough?
#2
#3
There's surely an existing issue about this somewhere, or several. I remember trying to update the handbook page about this when I first started looking at D7...
#4
chx: Can you please elaborate more why SQL strings are evil? I'm sure you have good reasons, but it would be best if you put them in the thread for further discussion...
Some positives I can think of for now:
Without concrete negatives as to the utility of db_select() in situations where a dynamic query, pagers, and access control are involved, I don't see why the policy would need to change in D8 or future versions...
#5
Cross-posted.
#6
*sigh*
The policy is, always has been, and always shall be, "db_select() is way slower than db_query(). Don't use it if you don't need it." Anyone who doesn't know that hasn't been paying attention for the past 4+ years since DBTNG went in, or is in denial, but either way they're simply wrong. Just how much slower is debatable, but the last time someone benchmarked it there was a surprisingly higher memory usage as well as a notable performance loss.
Which, if you think about what it does, makes total sense.
If you're using db_select() by default, you're doing it wrong. Period. End of story. kthxbye.
chx wants everyone to use db_select() because db_select() is easier to hack into something that will run on MongoDB or some other non-SQL (I refuse to use the buzzword "NoSQL") datastore. (I've discussed this with him before many times.) To be clear, I am 100% in support of the goal of making Drupal run sans-SQL. However, "make it easy to hack" is a fundamentally flawed approach.
Instead, especially with Drupal 8 the correct answer is... duh, dependency injection. We now have two different systems available in Drupal 8 for swapping out functionality: The Dependency Injection Container and Plugins. Use them. Please. Like, virtually all meaningful functionality should be in one or the other of those systems; the only things that should remain outside of one of those are glue code, and that glue code should not have SQL queries in it in the first place. (Yes, I include hooks in that; hooks, like events and controllers, should just be gluing together services offered by the DIC, or Plugins.) And those services and Plugins should take a DB object as an injected service rather than calling the wrapper functions in the first place.
That way, you don't need to try and mutate an SQL query into something that is not SQL. That's a losing battle. Rather, if you want to swap out a particular system that uses an SQL database for one that uses MongoDB, Cassandra, Redis, flat files, or something that's not been invented yet, you write one new class, tweak the DIC configuration, and poof. No more SQL.
Of course, the coding style that makes that easy also makes Unit Testing easy. It also makes development easier because it encourages divide and conquer. It also encourages flexibility. It also encourages writing for-reals APIs rather than random blobs of unstructured data, which in turn makes it possible to have real backward compatibility.
In short, well-written modern code makes the whole "use db_select() so it's more hackable" point irrelevant. If you have an SQL call, regardless of whether it's a literal query or a query builder, that is not in a swappable class, then in Drupal 8 I would consider that a bug. Right now, core still has a lot of examples of that bug, but we're slowly working at it. :-) But the solution to that bug is not "use the slower and harder to read version all the time." That's just bad news all around.
#7
Eh, that's hardly the reason. I strive very hard to separate that part of me from the core developer part, if it makes sense.
Let me ask this. Do you know who wrote a whole database layer to avoid writing INSERT queries?
#8
What I said on twitter: creating unalterable queries using a quirky language from the seventies doesn't look that good to me.
While db_select is slower, meh? Does that matter? Are we optimizing something meaningful? Usual questions, really.
#9
Finally, note that I can (and I do) work with SQL strings in the MongoDB driver as I want to, that's no reason...
#10
We've discussed this before, where it was Crell vs. DamZ: #835068: Document usage of static vs dynamic queries. Duplicate?
#11
Thank you!