----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18917/#review36575 -----------------------------------------------------------
The patch should make this feature optional. Preferably, make the visibility evaluation pluggable. At the very least, there's good reasons to disallow NOT terms, so this shouldn't be the default. I'd also like to see some additional tests, that read and write data to verify the feature end-to-end. This is changing some very important code that's been stable for some time, so more tests are always better. - Christopher Tubbs On March 7, 2014, 2:41 p.m., Joe Ferner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18917/ > ----------------------------------------------------------- > > (Updated March 7, 2014, 2:41 p.m.) > > > Review request for accumulo. > > > Repository: accumulo > > > Description > ------- > > This patch adds a NOT "!" operator to ColumnVisibility. > > The syntax is as follows: > !a > (!a)&(!b) > a&(!b) > a&(!(b|c)) > > Because of the nature of the current visibility parsing algorithm the > additional parentheses are required. > In the shell, the "Unable to render embedded object: File (" requires > escaping. This is due to how JLine parses the command and attempts to > substitute ") not found." with history. > > > Diffs > ----- > > core/src/main/java/org/apache/accumulo/core/security/ColumnVisibility.java > 75091d2 > > core/src/main/java/org/apache/accumulo/core/security/VisibilityEvaluator.java > 725b2c7 > > core/src/test/java/org/apache/accumulo/core/security/ColumnVisibilityTest.java > 7a6a80d > > core/src/test/java/org/apache/accumulo/core/security/VisibilityEvaluatorTest.java > ee4d2ee > > Diff: https://reviews.apache.org/r/18917/diff/ > > > Testing > ------- > > This patch includes unit tests for parsing, flattening, and evaluating the > not operator. > > > Thanks, > > Joe Ferner > >
