-
Notifications
You must be signed in to change notification settings - Fork 63
Adds possibility to search for commenter-email #18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
THis commit enables us to search for bugs that have been commented using a certain email-address
include/query.php
Outdated
| } | ||
|
|
||
| if ($commented_by != '') { | ||
| $query .= 'LEFT JOIN bugdb_comments c ON bugdb.id = c.bug'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe there is a space missing.
$query .= 'LEFT JOIN bugdb_votes v ON bugdb.id = v.bug';
$query .= 'LEFT JOIN bugdb_comments c ON bugdb.id = c.bug';
leads to
... bugdb.id = v.bugLEFT JOIN ...
without space. (I haven't checked larger context)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a copy of the SQL in line 67. Either that one doesn't work as well or the space at the end of the SQL-parts eliminates that issue.. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being a copy is not a reason the line is correct. Looking at a bit more context: The $query in earlier lines ends with a newline as whitespace, later additions always add a preceding space. Thus all other concatenations have some whitespace in between, this specific case won't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like you're right and I copied exactly the one line not adding a whitespace… 🤦♂️
Thanks for spotting!
As @johannes spotted the query didn't add a whitespace so that it might get added right to the preceding one without ans whitespace.
|
Easy to miss :)
Unless somebody beats me to it I'll push later ... and then we have to see how bad this JOIN is, there are quite a few comments in the database in total and I assume it will do a full table scan.
johannes
…On July 24, 2017 9:20:01 PM GMT+02:00, Andreas Heigl ***@***.***> wrote:
heiglandreas commented on this pull request.
> @@ -64,7 +65,11 @@
if (in_array($order_by, array('votes_count', 'avg_score'))) {
$query .= 'LEFT JOIN bugdb_votes v ON bugdb.id = v.bug';
- }
+ }
+
+ if ($commented_by != '') {
+ $query .= 'LEFT JOIN bugdb_comments c ON bugdb.id = c.bug';
Looks like you're right and I copied exactly the one line not adding a
whitespace… 🤦♂️
Thanks for spotting!
|
|
And there's no index on the email-field in the comment-table… Perhaps I should add an SQL-Patch with that? But that will leave the comment-table locked while the index is created… Not sure how long that will take… Or shall I add that as a separate PR? And anyway: How are patches added? |
|
Comment on behalf of johannes at php.net: I pushed it, let's see what breaks. I also added a simple MySQL status page to the admin section so we can estimate some effects. Should be live in an hour or so. |
|
So it is live, searching for all comments ever made by rasmus the request takes 3545 ms for 4157 comments, for all of mine (2681) it takes 2409ms. I ran a few other searches without these options and think it is acceptable for now. |
This PR enables us to search for bugs that have been commented using
a certain email-address.
It addresses #69576