Add aggregation functions (SUM, MAX, AVG) of ticket columns to the custom query options

[dema@…]

Use case: I added a custom field "estimated effort" to my project tickets that we usually like to see summed up on listings.

It would be great to have, probably below the "group by" option, the ability to aggregate columns by using common SQL aggregation functions such as min, max, sum, avg, etc.

[mariotomo@…]

or maybe easier, add the possibility to use one aggregation function in a similar way as count can already be used: from wiki:TracQuery

[[TicketQuery(version=0.6|0.7&resolution=duplicate, count)]] returning the count of the matching tickets,

it would be nice if we could use for example sum(<fieldname>), like this: [[TicketQuery(version=0.6|0.7&resolution=duplicate, sum(hours))]]

then the ones of us using (I can't remember what is the name of the plugin) defining the fields hours and totalhours could have an overview of the planned work.

even in this limited form, it sounds like a useful addition! and it can't be that difficult, can it? I'm going to have a look at the sources.

[mariotomo@…]

small patch implementing as described.

[mariotomo@…]

I've tested the patch on a vanilla installation, added two tickets, created a page with the query [[TicketQuery(status!=closed, sum(id))]] got 3

created a new ticket refresh test page got 6

closed ticket 1 refresh test page got 5

it looks good to me, do you need a unit-test case to accept the patch?

[Remy Blank]

This looks very nice indeed. Unit tests are not mandatory, but give extra karma points :)

One question: does this also work correctly with custom ticket fields? Those are text fields, so I'm not sure if they are cast to a numerical type prior to aggregation, and they are stored in a separate table.

[mariotomo@…]

oh, that is in fact also our case, I was not aware of the fact that they are in a separate table. I will either have to add custom fields to the (as said) vanilla installation, or apply the patch on our trac server and see what happens.

I think the first option is better but I do not have any experience with installing plugins to trac, so it's probably going to cost me a bit more effort.

[mariotomo@…]

nope, this patch does not work with custom columns.

[mariotomo@…]

without modifying the patch, but with a slightly clumsier query:

[[TicketQuery(status!=closed,format=sum(totalhours),col=totalhours)]] [[TicketQuery(status!=closed,format=sum(estimatedhours),col=estimatedhours)]]

it should be possible to alter the query at an early stage…

[AdrianFritz]

Nice work! Just for reference, there is a plugin (​th:SumFieldsPlugin) which allows to sum custom fields. It uses JQuery.

[mariotomo@…]

hi Adrian, thanks for the link. if I understand it correctly, that plugin adds a row with totals to query tables, and it must be configured (in trac.ini) for each column you want to sum up.

[AdrianFritz]

Hi Mario, your understandig is correct.

[AdrianFritz <adrian@…>]

P/S: I pointed the plug-in because could serve you as a reference (specifically to deal with custom fields). Your approach (selecting at query time which colums to sum), I think is more interesting because you don't need go into trac.ini to configure.

[mariotomo@…]

Hi Adrian, as rblank pointed out, custom fields are TEXT, so he was rightfully asking what happens when aggregating text values with a numeric function…

sqlite3 does not have a problem with it, but that was expected as sqlite says it has typed values, not typed columns.

using this data:

id	fk
1	1
2	2
3	1
4	1

on oracle select fk, count(*), sum('4') from SAMPLE group by fk gives

fk	count(*)	sum('4')
1	3	12
2	1	4

but postgres with the same query I am informed I "might need to add explicit type casts".

SELECT fk, count(*), sum(CAST ('4' AS integer)) from SAMPLE group by fk;

works on both Oracle and PostgreSQL. so I'm going to alter the patch and I am looking forward for feedback (or seeing it applied to the sources in the repository) :).

[mariotomo@…]

cast the text data being aggregated to integer or float

[mariotomo@…]

this patch contains also the tests for the new behaviour.

[AdrianFritz]

I will test it, today, with a PostgreSQL.

[mariotomo@…]

ping :) (did you have the chance to test the patch?)

[mariotomo@…]

"backport" to 0.12dev (but without test cases)

[AdrianFritz]

Replying to mariotomo@…:

ping :) (did you have the chance to test the patch?)

I'm sorry, I'm late because trouble with 0.13dev on current set-up (trying to keep both 0.12 and 0.13 on same server). Advancing anyway.

Test on 0.12 will be easier on our current environment (I saw you also did a backport).

[AdrianFritz]

Tested on Trac 0.12 + PostgreSQL → worked perfectly

[AdrianFritz]

Tested on Trac 0.12dev + PostgreSQL → worked perfectly

[mariotomo@…]

I just checked: the three test cases I had written for 0.13dev can also be used on 0.12dev.

[AdrianFritz]

Replying to AdrianFritz:

Replying to mariotomo@…:

ping :) (did you have the chance to test the patch?)

I'm sorry, I'm late because trouble with 0.13dev on current set-up (trying to keep both 0.12 and 0.13 on same server). Advancing anyway.

Test on 0.12 will be easier on our current environment (I saw you also did a backport).

Ping to myself: to keep 0.12.0, 0.12.dev and 0.13dev on same environment → Virtualbox

• trouble connecting PostgreSQL with 0.13dev (anyway not related with this patch)

[mariotomo@…]

ping :) we have been using this addition on a patched 0.12 for a couple of months, worked fine here. We recently moved the trac server to an other server and we would like to keep the installation as clean as possible, that is, no local patches. what news are there? regards, MF

[Christian Boos]

Thanks for the patch, and it's a good thing you've added unit tests for the feature! However we'll integrate this on trunk only, as 0.12-stable will not get new features at this point.

As such, you'll need to adapt to the new SQL API (for cnt, in env.db_query("SELECT ..., see TracDev/DatabaseApi#Trac0.13API).

Other minor points:

• instead of: if ... not in ['AVG', ...], prefer: if ... in ('AVG', ...) and no else clause (i.e. the caller will get None for an unknown aggregate function)
• when the caller gets None, it should render a value that can't be mistaken for a legitimate value (e.g. -- instead of 0) and with a tooltip saying something like _("unknown aggregate function %(name)s", name=function_name) (it's more important to make the error visible to the user rather than in the log… an admin can't do anything about that wrong user input)
• [a-z0-9_] in the _aggregator regexp could be just \w (as we're in a non-unicode case insensitive regexp)

[anonymous]

hi here, any news? I looked into the sources for 0.13 and did not yet find the aggregate function.

[Peter Suter]

#10983 could maybe allow implementing this in a plugin.

[figaro]

The business logic in the following plugins may be useful in this regard:

• ​th:SumFieldsPlugin
• ​th:ValuePropagationPlugin