Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Content-Type: multipart/alternative;
 boundary="===============6646098028686760794=="
MIME-Version: 1.0
Subject: Re: Review Request 18917: Add a NOT (!) operator to ColumnVisibility
 (ACCUMULO-2439)
From: "Jeff Kunkle" <kunklejr@gmail.com>
To: "Christopher Tubbs" <ctubbsii@apache.org>,
 "Josh Elser" <josh.elser@gmail.com>, "Joe Ferner" <joe.m.ferner@gmail.com>,
 "Jeff Kunkle" <kunklejr@gmail.com>, "accumulo" <dev@accumulo.apache.org>
Date: Wed, 12 Mar 2014 01:28:43 -0000
Message-ID: <20140312012843.27073.80960@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Jeff Kunkle" <noreply@reviews.apache.org>
References: <20140307211907.2625.96928@reviews.apache.org>
In-Reply-To: <20140307211907.2625.96928@reviews.apache.org>
Reply-To: "Jeff Kunkle" <kunklejr@gmail.com>

--===============6646098028686760794==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit


> 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.
> 
> Jeff Kunkle wrote:
>     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.
> 
> Josh Elser wrote:
>     Jeff, I'd encourage you to read and participate in the discussion on the mailing lists as to the reasons behind this (http://mail-archives.apache.org/mod_mbox/accumulo-user/201403.mbox/%3C1394052507311-7949.post%40n5.nabble.com%3E). ReviewBoard isn't really the place to have this discussion (it's just not manageable).

Sorry. I posted here because someone commented on JIRA issue that it was easier to gather feedback here. 

https://issues.apache.org/jira/browse/ACCUMULO-2439?focusedCommentId=13924236&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13924236


- 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
> 
>


--===============6646098028686760794==--