accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jeff Kunkle" <kunkl...@gmail.com>
Subject Re: Review Request 18917: Add a NOT (!) operator to ColumnVisibility (ACCUMULO-2439)
Date Wed, 12 Mar 2014 00:24:38 GMT


> On March 7, 2014, 9:19 p.m., Christopher Tubbs wrote:
> > 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.

What are the good reasons to disallow NOT terms? I'm not saying there aren't, I'm just not
aware of them. I would greatly benefit from the NOT operator on a current project and hope
this patch has a chance of being included.


- Jeff


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18917/#review36575
-----------------------------------------------------------


On March 11, 2014, 3:51 p.m., Joe Ferner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18917/
> -----------------------------------------------------------
> 
> (Updated March 11, 2014, 3:51 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message