Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 780028 - [Oracle] Oracle crashes if a column listed in ORDER BY appears twice in SELECT
: [Oracle] Oracle crashes if a column listed in ORDER BY appears twice in SELECT
: regression
Product: Bugzilla
Classification: Server Software
Component: Query/Bug List (show other bugs)
: 4.2.2
: All All
: -- critical (vote)
: Bugzilla 4.2
Assigned To: Frédéric Buclin
: default-qa
: 781549 783873 (view as bug list)
Depends on: 753688
  Show dependency treegraph
Reported: 2012-08-02 16:21 PDT by Frédéric Buclin
Modified: 2012-08-19 04:27 PDT (History)
5 users (show)
LpSolit: approval+
LpSolit: blocking4.4+
LpSolit: approval4.2+
LpSolit: blocking4.2.3+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

patch, v1 (1.15 KB, patch)
2012-08-02 18:08 PDT, Frédéric Buclin
glob: review+
Details | Diff | Splinter Review

Description Frédéric Buclin 2012-08-02 16:21:15 PDT
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.
Comment 1 Frédéric Buclin 2012-08-02 16:37:09 PDT
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?
Comment 2 Frédéric Buclin 2012-08-02 17:37:55 PDT
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.
Comment 3 Frédéric Buclin 2012-08-02 18:08:35 PDT
Created attachment 648586 [details] [diff] [review]
patch, v1

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.
Comment 4 Byron Jones ‹:glob› 2012-08-03 00:16:42 PDT
Comment on attachment 648586 [details] [diff] [review]
patch, v1

by inspection - i don't have oracle to test
Comment 5 Frédéric Buclin 2012-08-03 03:22:08 PDT
Committing to: bzr+ssh://
modified Bugzilla/
Committed revision 8328.

Committing to: bzr+ssh://
modified Bugzilla/
Committed revision 8112.
Comment 6 Frédéric Buclin 2012-08-09 08:46:41 PDT
*** Bug 781549 has been marked as a duplicate of this bug. ***
Comment 7 Frédéric Buclin 2012-08-19 04:27:07 PDT
*** Bug 783873 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.