Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2113523005:
Crimson: Added SQL indexes (Closed)
DescriptionCrimson: Added SQL indexes
BUG=591565
R=sheyang@chromium.org
Committed: https://chromium.googlesource.com/infra/infra/+/b46e17c5310a9ca27e4eea76dfb3b352ae2c43d0
Patch Set 1 #
Total comments: 6
Depends on Patchset: Messages
Total messages: 15 (6 generated)
pgervais@chromium.org changed reviewers: + sergeyberezin@chromium.org, sheyang@chromium.org
sheyang: I'd like your opinion on this because you've spent time working with SQL. ptal
I don't know about the context, so just add some questions which may seem silly... https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... File go/src/infra/crimson/sql/schema.sql (right): https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... go/src/infra/crimson/sql/schema.sql:10: site varchar(20) NOT NULL, Is this a PK? https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... go/src/infra/crimson/sql/schema.sql:16: CREATE index ip_range_start_ip_idx ON ip_range(start_ip); The purpose of index start_ip and end_ip is for querying with where clause I guess. Would storing ip as varchar still work? And how about ipv6? https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... go/src/infra/crimson/sql/schema.sql:20: site varchar(20) NOT NULL, Is this a FK?
https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... File go/src/infra/crimson/sql/schema.sql (right): https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... go/src/infra/crimson/sql/schema.sql:10: site varchar(20) NOT NULL, On 2016/07/01 21:55:26, sheyang wrote: > Is this a PK? It will be part of it (in another CL). https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... go/src/infra/crimson/sql/schema.sql:16: CREATE index ip_range_start_ip_idx ON ip_range(start_ip); On 2016/07/01 21:55:26, sheyang wrote: > The purpose of index start_ip and end_ip is for querying > with where clause I guess. That's correct. > Would storing ip as varchar still work? And how about ipv6? IP and mac addresses are stored as hexadecimal strings, that's why it's 34 characters long (IPv6: 16bytes*2 + '0x' prefix). With this representation, lexicographical order sorts IPv6 and IPv4 addresses in two separate blocks. https://codereview.chromium.org/2113523005/diff/1/go/src/infra/crimson/sql/sc... go/src/infra/crimson/sql/schema.sql:20: site varchar(20) NOT NULL, On 2016/07/01 21:55:26, sheyang wrote: > Is this a FK? It looks like a foreign key but it's not. There is no SQL-friendly way to join the two tables. An IP address corresponds to an IP range but it's not a join...
Okay, LGTM!
The CQ bit was checked by pgervais@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Trusty 64 Tester on master.tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Trusty%...)
The CQ bit was checked by pgervais@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Infra Linux Precise 32 Tester on master.tryserver.infra (JOB_FAILED, https://build.chromium.org/p/tryserver.infra/builders/Infra%20Linux%20Precise...)
Description was changed from ========== Crimson: Added SQL indexes BUG=591565 ========== to ========== Crimson: Added SQL indexes BUG=591565 R=sheyang@chromium.org Committed: https://chromium.googlesource.com/infra/infra/+/b46e17c5310a9ca27e4eea76dfb3b... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as b46e17c5310a9ca27e4eea76dfb3b352ae2c43d0 (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
