SQL query with COUNT(DISTINCT

abautu - February 17, 2007 - 19:21
Project:Drupal
Version:5.1
Component:taxonomy.module
Category:feature request
Priority:normal
Assigned:abautu
Status:duplicate
Description

Hello,

I'm working on a SQLite layer for Drupal as a hobby. It's very functional, but I have the following problem: your module uses some queries with COUNT(DISTINCT(...)). I'm not sure is this syntax is SQL standard, but it can by rewritten using a GROUP BY clause (in a 100% standard way). For instance:
line 1241: 'SELECT COUNT(DISTINCT(n.nid)) FROM {node} n '. $joins .' WHERE n.status = 1 '. $wheres; could be replaced with 'SELECT COUNT(*) FROM {node} n '. $joins .' WHERE n.status = 1 '. $wheres . ' GROUP BY n.nid';
and similar in line 1231.

You may notice that the queries I'm suggesting are shorter, cleaner (you may cleary see that you are counting all the lines) and maybe faster (because of possible GROUP BY optimizations).

Best wishes,
Andrei.

#1

abautu - February 17, 2007 - 22:11
Assigned to:Anonymous» abautu
Status:active» patch (reviewed & tested by the community)

I learned how to use a CVS, how Drupal CVS works, how patches are made and I thought to give it a try. I've tested the patch on my installation.

AttachmentSize
taxonomy_35.patch1.68 KB

#2

Crell - February 17, 2007 - 23:14

1) Do not post duplicate issues to different issue queues.
2) Do not set your own patch to "ready to be committed".
3) New features go against 6.x, not against the stable version.
4) Do check the issue queue before posting, as there is already an effort for SQLite support: http://drupal.org/node/67349

Thank you.

#3

abautu - February 18, 2007 - 08:05

Thank you for your reply, I'll tackle the problems you mentioned one by one.

1) Sorry, I did not know the same person is handling all the modules. If it counts, I did post specific messages for each module (it was not the same message).

2) a. Then why is it available ? b. Which one should I use?

3) What about 5.x-dev? Should I post for that? I'm not even sure if this is a feature request, since it's not something new and it fixes some problems (I've read in a post that Drupal tends to use SQL compatible queries).

4) I did. That port is just a bit more than simple search and replace. I've done in two days a much more extensive port (including user defined functions required by statistics and forum modules, UTF-8 support, unqualified table names, automatic translation of MySQL queries including production of CREATE INDEX queries where available). With the for patches I've submitted, all Drupal core modules are fully functional with SQLite 2.8 and above (maybe even lower).

Take care,
Andrei.

#4

Crell - February 18, 2007 - 08:51
Status:patch (reviewed & tested by the community)» duplicate

Drupal Core, unless you're really getting into heavy API rewrites, should be viewed as a single entity. There's no one specific person responsible for the whole thing but the community as a whole cares for it. That said, this is a feature request and all feature requests should go against the current development version; once a stable version is released, it takes no patches except bug/security fixes.

When submitting a patch, it is never ready to commit until it has been reviewed by at least one, preferably several other people to see if it's bug-free, useful, well-written, well-documented, etc. Setting your own patch RTBC roughly translates as "I'm just so damned good that you don't need to check me". No matter how good you may be, no one is that good. :-) (And it's available on the drop down because the drop down is not contextually-restricted.)

If you're interested in seeing SQLite support in Drupal 6 (which I am all for), then trying to start a second effort to see it happen will only make it that much less likely because you're duplicating effort. SQLite support is already "almost ready" in the other patch. Focus on getting that fully ready first and committed. If you then have other optimizations you believe can be made, do so incrementally after the SQLite patch itself is in. Of course, what you've posted here is just a small SQL tweak, not full SQLite support, so there's no evidence that you've done any of what you claim. (I'm not saying you haven't, just that "show me the code" is worth far more than "tell me about the code".)

#5

abautu - February 18, 2007 - 10:51

Thank you for advices. I'll try to keep them in mind. I apologize if I seemed rude, I was just curious. If you'd like my SQLite port, drop me an email. I'll try to get it sync with the other port, but I was hoping to see SQLite support in Drupal 5.2 or so :( It's really not a big deal, just a fix of mine. Meanwhile, I've also writen a script that translates an MySQL database dump into SQLite compatible queries.

Bye,
Andrei.

#6

Crell - February 18, 2007 - 17:16

Well it will definitely not happen in 5.2, because 5.2 will be a bug/security fix for Drupal 5 only. Drupal 5.x is feature-frozen for all eternity. Drupal 6-dev, though, is still gleefully accepting new features and will be until 1 June, I believe.

#7

Gloria B - March 18, 2008 - 16:43

So, how did it go? Did you get it to work properly?

Gloria, Web Developer currently working on the herbs for hypothyroidism project.

 
 

Drupal is a registered trademark of Dries Buytaert.