Todd Lipcon has posted comments on this change.

Change subject: improvements to /table

Patch Set 1:

Commit Message:

Line 29: 5. The ALL_CAPS enum names for the encoding and compression types
hrm, I actually prefer the ALL_CAPS, since anyone using the API (or SQL) would 
have to use the actual enum name, and now you're making them convert back, no?
File src/kudu/master/

Line 126:   if (a.second == RaftPeerPB::LEADER) {
hrm, I have a hard time verifying in my head whether this maintains a proper 
total order (it probably does, but I've made mistakes in this sort of function 
before, and had it cause a crash on some STL implementations which require 
"consistent" comparators).

how about something like:

int RoleToSortIndex(RaftPeerPB::Role r) {
  switch (r) {
    case LEADER: return 0;
    default: return 1 + static_cast<int>(r);

and then compare RoleToSortIndex(a) < RoleToSortIndex(b)?

Line 259:   *output << "<h3>Partition schema & range bounds</h3>";

Line 265:                           
need to HTML Escape this (otherwise someone could make a range bound with a 
<script> tag)
File src/kudu/server/

PS1, Line 136:   *output << "  'kudu.table_name' = '" << table_name << "',\n";
             :   *output << "  'kudu.master_addresses' = '" << master_addresses 
<< "',\n";
             :   *output << "  'kudu.key_columns' = '" << 
JoinElements(key_columns, ", ") << "'\n";
now that I look at this code again, I think these need HTML escapes

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib6724348b1cd199c4d651c1282f1eadb58226bea
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <>
Gerrit-HasComments: Yes

Reply via email to