289602 - Ordering by "Deadline" doesn't work.

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Terry Weissman]

If you bring up a buglist that contains the "deadline" column, and then you
click on that "deadline" header to order your bugs by deadline, it crashes.

Turns out the problem is that the definition of the "deadline" column works out
to something that contains a comma in it.  And the list of things to order by is
separated by commas.  So when it goes to parse out the list of columns, the
comma in the middle of the deadline definition confuses things, and it blows up.

I tried to look for a quick fix, but I didn't see one.  Options I see include:

- Change it to separate columns by something other than commas (maybe
semicolons).  This is probably the easiest fix, but it would break existing
bookmarks.

- Change the definition of deadline to be something that doesn't have a comma in
it.  I don't have any great ideas for what that would be.

- Change the %columns data structure in buglist.cgi so that the string you put
out in the URL does not have to be the same string that you feed to SQL.  This
is probably a somewhat large change.

Comment 1

[Max Kanat-Alexander]

Arrgh. We could also put it in %special_order_by in Search.pm, if we can even
get it to get that far.

This could also be solved by doing all the ORDER BY processing within Search.pm
instead of doing ANY of it in buglist.

Comment 2

[Dave Miller [:justdave]]

I like the last of those options best...  we should be using the short name from
fielddefs in anything that goes to a URL, and translate it back to the SQL
fragment on the back end when processing it.

Comment 3

[Marc Schumann [:Wurblzap]]

See also bug 302326.

Comment 4

[Frédéric Buclin]

*** Bug 312186 has been marked as a duplicate of this bug. ***

Comment 5

[Marc Schumann [:Wurblzap]]

Bug 302599 seems to me to be somewhat related.

Comment 6

[Frédéric Buclin]

For the record, the error message which is returned:

https://localhost/cvsbugzilla/buglist.cgi?bug_id=352&field-1-0-0=bug_id
&query_format=advanced&type-1-0-0=anyexact&value-1-0-0=352
&order=DATE_FORMAT%28bugs.deadline%2C%20%27%25Y-%25m-%25d%27%29%20AS%20deadline,bugs.bug_id&query_based_on=Security%20Bugs

 The custom sort order specified in your form submission contains an invalid column name DATE_FORMAT(bugs.deadline.

Comment 7

[Max Kanat-Alexander]

This should be fixed in 2.20, but I won't block the 2.20.1 release on it. However, I don't think we should release 2.22 until this is fixed.

Comment 8

[Frédéric Buclin]

Note that this error is generated by buglist.cgi, line 771:

   else {
       ThrowCodeError("invalid_column_name_form", $vars);
   }

in the /^Last Changed$/ && do { ... } block.

joel, any idea?

Comment 9

[Frédéric Buclin]

(In reply to comment #8)
> in the /^Last Changed$/ && do { ... } block.

err... the file and the line are correct, but it's not in this block.

Comment 10

[Frédéric Buclin]

OK, I found the reason, buglist.cgi list 756:

            # A custom list of columns.  Make sure each column is valid.
            foreach my $fragment (split(/,/, $order)) {

The order list is a comma separated list, but the deadline contains a comma and so is splitted:

TO_CHAR(bugs.deadline, 'YYYY-MM-DD') AS deadline 

Consequently, "TO_CHAR(bugs.deadline" alone doesn't match any column name and is rejected.

Note that this is the unique column name which contains a comma.

Comment 11

[Marc Schumann [:Wurblzap]]

*** Bug 319993 has been marked as a duplicate of this bug. ***

Comment 12

[Fred Brunner]

My bug 31993 was against 2.20 released, i.e. yes it is still in 2.20.

Comment 13

[Frédéric Buclin]

I think we are doing things the wrong way.

$cgi->param('order') should not contain SQL fragments, i.e. the column names of DefineColumn(), because they are DB dependent and so cannot be used by installations using different DBs. We should use column IDs instead. This is what bug 302326 is trying to do and this is the right fix IMO.

Comment 14

[Frédéric Buclin]

Fixed by blocker!