ORDER BY column needs to be selected or grouped

RESOLVED FIXED in Bugzilla 2.20

Status

()

RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Tomas.Kopal, Assigned: Tomas.Kopal)

Tracking

2.19.2
Bugzilla 2.20
Dependency tree / graph
Bug Flags:
approval +
blocking2.20 +

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

14 years ago
For normal selects, Postgres requires columns, which are used in the ORDER BY
clause to be also in GROUP BY. For SELECT DISTINCT, columns used in ORDER BY
needs to be SELECTed.
Patch will follow
(Assignee)

Updated

14 years ago
Status: NEW → ASSIGNED
Flags: blocking2.20?
Target Milestone: --- → Bugzilla 2.20
(Assignee)

Comment 1

14 years ago
Posted patch V1 (obsolete) — Splinter Review
In the course of fixing this, I was actually able to get rid of one query in
editusers.cgi, yupeee :-).
Attachment #177832 - Flags: review?(mkanat)
Flags: blocking2.20? → blocking2.20+
>-    my $query = 'SELECT DISTINCT userid, login_name, realname, disabledtext ' .
>-                'FROM profiles';
>+    my $query = 'SELECT DISTINCT userid, login_name, realname, ' .
>+                'disabledtext, login_name FROM profiles';

I'm no Pg guy, but this seems strange to me... Do you really need to read
login_name twice?
(Assignee)

Comment 3

14 years ago
(In reply to comment #2)
> >-    my $query = 'SELECT DISTINCT userid, login_name, realname, disabledtext ' .
> >-                'FROM profiles';
> >+    my $query = 'SELECT DISTINCT userid, login_name, realname, ' .
> >+                'disabledtext, login_name FROM profiles';
> 
> I'm no Pg guy, but this seems strange to me... Do you really need to read
> login_name twice?

Ummmm, no? :-) Sorry, my oversight, I will fix that, thanks.
(Assignee)

Updated

14 years ago
Attachment #177832 - Flags: review?(mkanat)
(Assignee)

Comment 4

14 years ago
Posted patch V1.1 (obsolete) — Splinter Review
Fixed comment #2.
(Assignee)

Updated

14 years ago
Attachment #177832 - Attachment is obsolete: true
Attachment #179644 - Flags: review?(mkanat)
(Assignee)

Comment 5

14 years ago
Posted patch V1.2 (obsolete) — Splinter Review
Sorry for the spam, I was a bit trigger-happy. I removed the functional change,
but didn't remove change in formatting. Fixed.
Attachment #179644 - Attachment is obsolete: true
Attachment #179645 - Flags: review?(mkanat)
(Assignee)

Updated

14 years ago
Attachment #179644 - Flags: review?(mkanat)
Comment on attachment 179645 [details] [diff] [review]
V1.2

Hrm... this patch seems to be missing the actual SQL change for editusers.cgi,
and so now any actual changes to that file as a part of this bug aren't
necessary, right?
(Assignee)

Comment 7

14 years ago
(In reply to comment #6)
> (From update of attachment 179645 [details] [diff] [review] [edit])
> Hrm... this patch seems to be missing the actual SQL change for editusers.cgi,
> and so now any actual changes to that file as a part of this bug aren't
> necessary, right?
> 

Well, yes, I suppose. But I am sure the change in that file is still a valid
improvement. Should I move it to a different bug then?
(In reply to comment #7)
> Well, yes, I suppose. But I am sure the change in that file is still a valid
> improvement. Should I move it to a different bug then?

  Sure. :-) I always like code cleanup. 

  And then you can ask wurblzap for review. :-)
(Assignee)

Comment 9

14 years ago
Posted patch V1.3Splinter Review
Removed editusers.cgi part of the patch to bug 289062.
Attachment #179645 - Attachment is obsolete: true
Attachment #179660 - Flags: review?(mkanat)
(Assignee)

Updated

14 years ago
Attachment #179645 - Flags: review?(mkanat)
Comment on attachment 179660 [details] [diff] [review]
V1.3

Great! This is now obviously correct. :-)
Attachment #179660 - Flags: review?(mkanat) → review+
(Assignee)

Updated

14 years ago
Flags: approval?
Flags: approval? → approval+
Checking in summarize_time.cgi;
/cvsroot/mozilla/webtools/bugzilla/summarize_time.cgi,v  <-- 
summarize_time.cginew revision: 1.7; previous revision: 1.6
done
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v  <--  User.pm
new revision: 1.51; previous revision: 1.50
done
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Version: unspecified → 2.19.2

Updated

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