Classification doesn't work as z-axis on reports

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
Reporting/Charting
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: signalrauschen, Assigned: Frédéric Buclin)

Tracking

({regression})

4.2.1
Bugzilla 4.2
regression
Bug Flags:
approval +
blocking4.4 +
approval4.2 +
blocking4.2.2 +

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 622658 [details]
attachment1 [details] [diff] [review].jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725

Steps to reproduce:

A report query that worked in Bugzilla 3.x does not work in Bugzilla 4.2+ any more.

I tried to generate a tabular report with the following axis parameters (see attachment 1 [details] [diff] [review]):
- Vertical axis: Status
- Horizontal axis: Severity
- Tables: Classification

The URL looks like this:
https://bugzilla.xxx.de/report.cgi?x_axis_field=bug_severity&y_axis_field=bug_status&z_axis_field=classification&query_format=report-table&short_desc_type=allwordssubstr&short_desc=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&bug_status=RESOLVED&bug_status=VERIFIED&bug_status=CLOSED&longdesc_type=allwordssubstr&longdesc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&deadlinefrom=&deadlineto=&bug_id=&bug_id_type=anyexact&emailtype1=substring&email1=&emailtype2=substring&email2=&emailtype3=substring&email3=&chfieldvalue=&chfieldfrom=&chfieldto=Now&j_top=AND&f1=noop&o1=noop&v1=&format=table&action=wrap


In Bugzilla 3.x this resulted in a tabular report showing a table with the following structure for each classification:

             blocker | citical | major | minor | normal | trivial | enh | Total
NEW
ASSIGNED
REOPENED
DONE
...



Actual results:

In Bugzilla 4.2+ this report query resulted in an empty page (see attachment 2 [details] [diff] [review]). No table has been generated.


Expected results:

This report query should result in a tabular report showing a table with the following structure for each classification:

             blocker | citical | major | minor | normal | trivial | enh | Total
NEW
ASSIGNED
REOPENED
DONE
...
(Reporter)

Comment 1

5 years ago
Created attachment 622659 [details]
attachment2 [details] [diff] [review].jpg
I can confirm on https://landfill.bugzilla.org/bugzilla-tip/. If the classification is used as the z-axis of a report, the result is empty.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Generated tabular report is empty → Classification doesn't work as z-axis on reports
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Updated

5 years ago
Keywords: qawanted, regression
(Assignee)

Comment 3

5 years ago
There is something really wrong, also with other fields as z-axis. With JS turned on, all tables have the same data. With JS turned off, it seems to be fine. With the classification as z-axis, JS turned on or off doesn't help.
Flags: blocking4.2.2+
Keywords: qawanted
(Assignee)

Comment 4

5 years ago
The reason why reports now fail with the classification field is that Search.pm now *prepends* extra columns to the column list passed by the caller instead of appending them as it used to do in Bugzilla 4.0 and older. The consequence is that the caller no longer gets back what it passed in the 'fields' argument and data are in a pseudo-random order. AFAIK, there is no reason to prepend extra columns for the SELECT part of the SQL query. The order is only important for the FROM part.

This is a regression due to bug 577807 and/or bug 578278.
Severity: normal → major
(Assignee)

Updated

5 years ago
Flags: blocking4.4+
(Assignee)

Comment 5

5 years ago
Created attachment 638195 [details] [diff] [review]
patch, v1

AFAIK, we don't need to select columns involved in JOINs, and so _display_columns() doesn't include columns listed in %COLUMN_DEPENDS. Fields are now returned in the order defined by the caller, with extra columns at the end (which the caller can ignore).

Let me know if this regresses something.
Assignee: charting → LpSolit
Attachment #622658 - Attachment is obsolete: true
Attachment #622659 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #638195 - Flags: review?(glob)
Attachment #638195 - Flags: review?(dkl)
Attachment #638195 - Flags: review?(bugzilla.1.wurblzap)
Comment on attachment 638195 [details] [diff] [review]
patch, v1

Review of attachment 638195 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

this looks good, and i can't break it.

::: Bugzilla/Search.pm
@@ +852,5 @@
> +
> +    # Do not alter the list specified here at all, even if they are duplicated.
> +    # Those are passed by the caller, and the caller expects to get them back
> +    # in the exact same order.
> +    $self->{display_columns} = [$self->_input_columns, $self->_extra_columns];

do:

$self->{display_columns} ||= [$self->_input_columns, $self->_extra_columns];

and drop the early return

@@ +878,5 @@
>  # will go into the SELECT clause.
>  sub _sql_select {
>      my ($self) = @_;
>      my @sql_fields;
> +    foreach my $column ($self->_display_columns) {

as far as i can tell it should be safe to 'uniq' display_columns here
Attachment #638195 - Flags: review?(glob) → review+
(Assignee)

Updated

5 years ago
Flags: approval4.2+
Flags: approval+

Updated

5 years ago
Attachment #638195 - Flags: review?(dkl)
(Assignee)

Updated

5 years ago
Attachment #638195 - Flags: review?(bugzilla.1.wurblzap)
(Assignee)

Comment 7

5 years ago
(In reply to Byron Jones ‹:glob› from comment #6)
> do:
> 
> $self->{display_columns} ||= [$self->_input_columns, $self->_extra_columns];
> 
> and drop the early return

Good idea.


> as far as i can tell it should be safe to 'uniq' display_columns here

I prefer not to, as the caller may pass the same column twice.


Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 8296.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/Search.pm
Committed revision 8103.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 8

5 years ago
(In reply to Byron Jones ‹:glob› from comment #6)
> as far as i can tell it should be safe to 'uniq' display_columns here

I'm doing some tests with Oracle right now, and I see that Oracle crashes if the SQL query contains a column in ORDER BY which appears twice in SELECT. So for instance

  "SELECT bug_id, assigned_to, assigned_to FROM bugs ORDER BY assigned_to"

crashes in Oracle (because it complains that the assigned_to column in ORDER BY is ambiguous), but it works correctly in MySQL and PostgreSQL. If I explicitly specify the table name in ORDER BY, i.e. "ORDER BY bugs.assigned_to", then Oracle is happy.

So to fix this problem, we have 2 solutions:
a) remove duplicated columns in _display_columns, but this may confuse the caller which expects some list order. So if the caller wants "bug_id, assigned_to, assigned_to", we would return "bug_id, assigned_to" only, which is unexpected.
b) prefix all columns in ORDER BY with the table they belong to, but how do we get this information? This doesn't seem to be a viable solution, unless someone knows how to do this.
(Assignee)

Updated

5 years ago
Blocks: 780028
You need to log in before you can comment on or make changes to this bug.