Closed
Bug 753688
Opened 12 years ago
Closed 12 years ago
Classification doesn't work as z-axis on reports
Categories
(Bugzilla :: Reporting/Charting, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 4.2
People
(Reporter: signalrauschen, Assigned: LpSolit)
References
Details
(Keywords: regression)
Attachments
(1 file, 2 obsolete files)
1.95 KB,
patch
|
glob
:
review+
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
Comment 2•12 years ago
|
||
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•12 years ago
|
Keywords: qawanted,
regression
Assignee | ||
Comment 3•12 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•12 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•12 years ago
|
Flags: blocking4.4+
Assignee | ||
Comment 5•12 years ago
|
||
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•12 years ago
|
Flags: approval4.2+
Flags: approval+
Updated•12 years ago
|
Attachment #638195 -
Flags: review?(dkl)
Assignee | ||
Updated•12 years ago
|
Attachment #638195 -
Flags: review?(bugzilla.1.wurblzap)
Assignee | ||
Comment 7•12 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
Closed: 12 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 8•12 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.
You need to log in
before you can comment on or make changes to this bug.
Description
•