RESOLVED FIXED 199227
commit api should not return 'AmbiguousRevisionPrefix' if there is an exact revision match 
https://bugs.webkit.org/show_bug.cgi?id=199227
Summary commit api should not return 'AmbiguousRevisionPrefix' if there is an exact r...
dewei_zhu
Reported 2019-06-26 12:54:37 PDT
commit api should not return 'AmbiguousRevisionPrefix' if there is an exact revision match
Attachments
Patch (4.73 KB, patch)
2019-06-26 12:57 PDT, dewei_zhu 		rniwa: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
dewei_zhu
Comment 1 2019-06-26 12:57:16 PDT
Created attachment 372941 [details] Patch
dewei_zhu
Comment 2 2019-06-26 12:57:42 PDT
<rdar://52140788>
Ryosuke Niwa
Comment 3 2019-06-26 13:19:46 PDT
Comment on attachment 372941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372941&action=review > Websites/perf.webkit.org/public/include/commit-log-fetcher.php:209 > + if ($rows[0]['commit_revision'] == $revision_prefix) > + return $rows[0]; There is no guarantee that the first one would be an exact match. It appears to me that we'd have to issue two queries one for the exact match or another one for like. Alternatively, you can generate a boolean based on exact equality of the revision and sort the results so that the exact match appear first. As is there is no guarantee this would work.
dewei_zhu
Comment 4 2019-06-26 13:57:27 PDT
Comment on attachment 372941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372941&action=review >> Websites/perf.webkit.org/public/include/commit-log-fetcher.php:209 >> + return $rows[0]; > > There is no guarantee that the first one would be an exact match. > It appears to me that we'd have to issue two queries one for the exact match or another one for like. > Alternatively, you can generate a boolean based on exact equality of the revision > and sort the results so that the exact match appear first. > As is there is no guarantee this would work. Ordering by commit_revision should grantee the exact match always be the first one. Per https://www.postgresql.org/docs/9.5/queries-order.html#AEN4629. The only case that I can think of is same revision but different lower vs upper cases.
Ryosuke Niwa
Comment 5 2019-06-26 14:37:28 PDT
Comment on attachment 372941 [details] Patch Ah, I missed that we were only checking prefixes.
Note You need to log in before you can comment on or make changes to this bug.