Edgewall Software
Modify

Opened 14 years ago

Last modified 15 months ago

#2855 new enhancement

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

Reported by: dema@… Owned by:
Priority: normal Milestone: unscheduled
Component: general Version: 0.9.4
Severity: normal Keywords: query
Cc: haircut@… Branch:
Release Notes:
API Changes:

Description

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.

Attachments (5)

2855.diff (2.3 KB ) - added by mariotomo@… 8 years ago.
small patch implementing as described.
2855.2.diff (2.4 KB ) - added by mariotomo@… 8 years ago.
cast the text data being aggregated to integer or float
2855.3.diff (3.9 KB ) - added by mariotomo@… 8 years ago.
this patch contains also the tests for the new behaviour.
2855.4.diff (2.5 KB ) - added by mariotomo@… 8 years ago.
"backport" to 0.12dev (but without test cases)
2855-0.12dev.diff (4.0 KB ) - added by mariotomo@… 8 years ago.

Download all attachments as: .zip

Change History (32)

comment:1 by sid, 13 years ago

Keywords: query added

comment:2 by Christian Boos, 13 years ago

Milestone: 2.0

comment:3 by mariotomo@…, 8 years ago

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.

by mariotomo@…, 8 years ago

Attachment: 2855.diff added

small patch implementing as described.

comment:4 by mariotomo@…, 8 years ago

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?

comment:5 by Remy Blank, 8 years ago

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.

comment:6 by mariotomo@…, 8 years ago

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.

comment:7 by mariotomo@…, 8 years ago

nope, this patch does not work with custom columns.

comment:8 by mariotomo@…, 8 years ago

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…

comment:9 by AdrianFritz, 8 years ago

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

comment:10 by mariotomo@…, 8 years ago

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.

comment:11 by AdrianFritz, 8 years ago

Hi Mario, your understandig is correct.

comment:12 by AdrianFritz <adrian@…>, 8 years ago

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.

comment:13 by mariotomo@…, 8 years ago

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) :).

by mariotomo@…, 8 years ago

Attachment: 2855.2.diff added

cast the text data being aggregated to integer or float

by mariotomo@…, 8 years ago

Attachment: 2855.3.diff added

this patch contains also the tests for the new behaviour.

comment:14 by AdrianFritz, 8 years ago

I will test it, today, with a PostgreSQL.

comment:15 by mariotomo@…, 8 years ago

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

by mariotomo@…, 8 years ago

Attachment: 2855.4.diff added

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

in reply to:  15 ; comment:16 by AdrianFritz, 8 years ago

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).

comment:17 by AdrianFritz, 8 years ago

Tested on Trac 0.12 + PostgreSQL → worked perfectly

comment:18 by AdrianFritz, 8 years ago

Tested on Trac 0.12dev + PostgreSQL → worked perfectly

comment:19 by mariotomo@…, 8 years ago

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

by mariotomo@…, 8 years ago

Attachment: 2855-0.12dev.diff added

in reply to:  16 comment:20 by AdrianFritz, 8 years ago

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)

comment:21 by mariotomo@…, 8 years ago

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

comment:22 by Christian Boos, 8 years ago

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)

comment:23 by anonymous, 8 years ago

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

comment:24 by haircut@…, 7 years ago

Cc: haircut@… added

comment:25 by Ryan J Ollos, 4 years ago

Owner: Jonas Borgström removed

comment:26 by Peter Suter, 15 months ago

#10983 could maybe allow implementing this in a plugin.

comment:27 by figaro, 15 months ago

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.