developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 11 years ago

Closed 10 years ago

#1225 closed bug (fixed)

bug with count + distinct clause in sqlite

Reported by: catsoup Owned by:
Priority: normal Milestone: Jelix 1.2RC3
Component: jelix:dao Version: trunk
Severity: normal Keywords:
Cc: Blocked By:
Blocking: Documentation needed: no
Hosting Provider: Php version:

Description

Because sqlite doesn't support COUNT(DISTINCT fieldname), we have 2 bugs:
in a dao method of the type

<method name="test" type="count" distinct="fieldname"/>

and with the countBy() method of the DAO factory

The common workaround seems to be that one: http://www.bernzilla.com/item.php?id=690

SELECT COUNT(fieldname) FROM (SELECT DISTINCT fieldname FROM table); 

Change History (6)

comment:1 Changed 11 years ago by catsoup

I was going to look for a patch, but, I wonder if it worth making a patch because it is not really a bug of dao, but a lack of of SQL support from sqlite.
So I think this bug is invalid ?

comment:2 Changed 11 years ago by catsoup

  • review set to review?

Finally, I worked on it,
Now countBy() works with a distinct parameter and sqlite
Plus I added support for multiple distinct on countBy and on type count dao methods. Need to provide distinct property like this "prop1,prop2" (without space) because I didn't made a trim, but I think I should..

Tests passes, but there is no tests using count, so I think I didn't broke anything, but need to improve the patch, maybe make some tests with count ?

here is my patch: http://bitbucket.org/catsoup/jelix-trunk-patches/src/tip/ticket1225.patch

comment:3 Changed 10 years ago by laurentj

  • Milestone set to Jelix 1.2

I'll take a look on your patch

comment:4 Changed 10 years ago by laurentj

  • review changed from review? to review-

first, I don't want this redefinition of buildUserMethods, it duplicates too many code. So I splitted it into several methods so now it easier to redefine only some parts http://bitbucket.org/jelix/jelix-trunk/changeset/a4b0d27af04f

Second, I don't want to add supports of multiple distinct fields, because it's too late, I just want to fix bugs for the 1.2, and because there isn't any tests for your new code...

If you have time to create a new patch that fixes only the bug...

comment:5 Changed 10 years ago by catsoup

  • review changed from review- to review?

Following changes you made in jDaoGenerator, I rewritted the patch, and now it only fix the bug for count with distinct concerning sqlite, both on a count dao method and on the countBy method: http://bitbucket.org/catsoup/jelix-trunk-patches/src/5f9e3d1cdb29/ticket1225.patch

comment:6 Changed 10 years ago by laurentj

  • Resolution set to fixed
  • review changed from review? to review+
  • Status changed from new to closed

Pushed, with a little modification: the database type can be retrieved from the dbms property instead of from the profile, so you don't have to deal with pdo configuration.

http://bitbucket.org/jelix/jelix-trunk/changeset/3fb7ccb3fdfb

thanks for your patch.

Note: See TracTickets for help on using tickets.