Just a thought - this would be a perfect case for adding a set of unit
tests to ensure that we get the output expected for the corresponding
input, for all the nooks and crannies of JSON and SQL, as the code
continues to be refined / debugged / extended.

And the test cases could make for good examples, too.

Just saying.

2009/6/25 Scott McKellar <m...@swbell.net>:
>
> I found the same glitch a few weeks ago and fixed it.  When I test it in my 
> sandbox, using the test_json_query utility, I get the GROUP BY and HAVING 
> clauses in the right order.  It also works from srfsh.
>
> I don't know what version you're running, but you can probably grab the 
> current version of oils_cstore.c from trunk and rebuild without refreshing 
> anything else.  Don't forget to restart the C services so that you're running 
> the new version.
>
> Scott McKellar
>
> --- On Thu, 6/25/09, Jeff Godin <j...@tcnet.org> wrote:
>
>> From: Jeff Godin <j...@tcnet.org>
>> Subject: [OPEN-ILS-DEV] open-ils.cstore.json_query and HAVING clauses
>> To: "Evergreen Development Discussion List" 
>> <open-ils-dev@list.georgialibraries.org>
>> Date: Thursday, June 25, 2009, 8:35 AM
>> Greetings-
>>
>> First off, a huge thank you to Scott and whoever else was
>> involved in
>> the production of "JSON Queries: A Tutorial", found in
>> docs/TechRef/JSONTutorial.xml
>>
>> I think I've found a bug in cstore when using GROUP BY with
>> a HAVING
>> clause. After a discussion with Mike Rylander, this might
>> be less of a
>> bug and more a case of "incomplete support for HAVING
>> clauses".
>>
>> Using an example from the tutorial:
>>
>>     {
>>         "select": {
>>             "aou": [
>>
>>   "parent_ou", {
>>
>>       "column":"id",
>>
>>       "transform":"count",
>>
>>       "alias":"id_count",
>>
>>       "aggregate":"true"
>>
>>   }
>>             ]
>>         },
>>         "from":"aou",
>>         "having": {
>>             "id": {
>>
>>   ">" : {
>>
>>       "transform":"count",
>>
>>       "value":6
>>
>>   }
>>             }
>>         }
>>     }
>>
>> I execute this in srfsh like so:
>>
>> srfsh# request open-ils.cstore
>> open-ils.cstore.json_query.atomic {
>> "select": { "aou": [ "parent_ou", { "column":"id",
>> "transform":"count", "alias":"id_count", "aggregate":"true"
>> } ] },
>> "from":"aou", "having": { "id": { ">" : {
>> "transform":"count",
>> "value":1 } } } }
>>
>> And the result is:
>>
>> Received Exception:
>> Name: osrfMethodException
>> Status: Severe query error -- see error log for more
>> details
>> Status: 500
>> Received Exception:
>> Name: osrfMethodException
>> Status: An unknown server error occurred
>> Status: 404
>> ------------------------------------
>> Request Completed Successfully
>> Request Time in seconds: 0.000476
>> ------------------------------------
>>
>> Checking logs, it appears that the GROUP BY and HAVING
>> clauses are
>> reversed in the generated SQL:
>>
>> open-ils.cstore 2009-06-25 09:34:03 [ERR
>> :4398:oils_cstore.c:4213:]
>> open-ils.cstore: Error with query [SELECT
>> "aou".parent_ou AS
>> "parent_ou", count("aou".id ) AS "id_count" FROM
>> actor.org_unit AS
>> "aou"  HAVING count("aou".id ) >  1
>> GROUP BY  1;]
>>
>> I looked at oils_cstore.c and it seems like things are
>> being placed
>> into the buffer in the correct order, but it's very
>> possible that I'm
>> reading this wrong, or missing something:
>>
>>         string =
>> buffer_release(group_buf);
>>
>>         if ( *string && (
>> aggregate_found || (flags & SELECT_DISTINCT) ) ) {
>>
>> OSRF_BUFFER_ADD( sql_buf, " GROUP BY " );
>>
>> OSRF_BUFFER_ADD( sql_buf, string );
>>         }
>>
>>         free(string);
>>
>>         if( having_buf ) {
>>
>> string = buffer_release(having_buf);
>>
>>                 if
>> ( *string ) {
>>
>>         OSRF_BUFFER_ADD( sql_buf, "
>> HAVING " );
>>
>>         OSRF_BUFFER_ADD( sql_buf, string
>> );
>>                 }
>>
>>
>> free(string);
>>         }
>>
>> If I take the generated SQL and manually re-order the GROUP
>> BY and
>> HAVING clauses, the query works.
>>
>> Anyone with in-depth knowledge of open-ils.cstore care to
>> investigate
>> and let me know if this is a simple fix?
>>
>> Thanks!
>>
>> -jeff
>>
>



-- 
Dan Scott
Laurentian University

Reply via email to