Bug 753688 comment 8: > 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.
Actually, solution a) doesn't work, because in tabular and graphical reports, you can have the same field twice for the x, y or z axis, and removing duplicated columns from the SQL query means we get only one field back instead of two. I tried, and report.cgi is unable to display any data anymore. Looks like we have to go with b), but I have no idea how to know which table a column belongs to. Any idea?
Assignee: LpSolit → query-and-buglist
Status: ASSIGNED → NEW
Solution b) won't work either, because some columns are the computation of several columns and so are not real columns and so cannot be prefixed. This leaves us with another solution: c) if duplicated columns are detected in _display_columns() (or maybe in _input_columns() only), use a different alias for them. But this probably involves a lot of complexity. /me hates Oracle.
OK, this patch is a good compromise between not altering duplicated columns passed by the caller, removing extra columns which are already present, and avoiding code complexity. buglist.cgi explicitly excludes duplicated columns, so we are sure this problem can never happen there as this patch now stops blindly adding extra columns even if they are already present. And report.cgi doesn't sort the data, so we are safe there too.
Assignee: query-and-buglist → LpSolit
Status: NEW → ASSIGNED
Attachment #648586 - Flags: review?(glob)
Comment on attachment 648586 [details] [diff] [review] patch, v1 r=glob by inspection - i don't have oracle to test
Attachment #648586 - Flags: review?(glob) → review+
Committing to: bzr+ssh://email@example.com/bugzilla/trunk/ modified Bugzilla/Search.pm Committed revision 8328. Committing to: bzr+ssh://firstname.lastname@example.org/bugzilla/4.2/ modified Bugzilla/Search.pm Committed revision 8112.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.