On Fri, Jul 1, 2016 at 3:24 PM, Mike Miller <michaelpmil...@gmail.com> wrote:
> What about an enum in the superclass (Filter.java) that contains all the > options specific to Filter? > public static enum FilterOptions { NEGATE } > And then skip any options in that enum in the constructor of TTLSet. > Someone would still have to include new options in the enum but at least it > would be more obvious. > That seems ok. Could also have a protected method, something like super.isValidOption(String). Not sure I like that method name. A method gives the super class more flexibility. > > On Fri, Jul 1, 2016 at 2:32 PM, Keith Turner <ke...@deenlo.com> wrote: > > > On Fri, Jul 1, 2016 at 1:29 PM, Mike Miller <michaelpmil...@gmail.com> > > wrote: > > > > > Hello Devs, > > > > > > For those of you I have yet to meet, my name is Mike Miller and I am > the > > > newbie who is going to be helping out on Accumulo development. I have > > been > > > working with Accumulo as a Java developer on an ingest and query > project > > > for the past year. I doubt I'll be able to fill the shoes of past > > > contributors but I hope that I can make a positive contribution to the > > > project. > > > > > > I have browsed the JIRA tickets labelled "newbie" and selected the > > > ACCUMULO-1604 <https://issues.apache.org/jira/browse/ACCUMULO-1604> > bug > > to > > > fix (more so as a learning exercise than anything). I just wanted to > make > > > sure I am reproducing the bug correctly and to propose a solution. > > > > > > I believe I reproduced the bug by getting the ColumnAgeOffFilter to > throw > > > an "IllegalArgumentException: bad TTL options" in the > > > o.a.a.core.iterators.user.FilterTest.test2a(). I did this by adding the > > > following line: > > > ColumnAgeOffFilter.setNegate(is, true); > > > This Exception is caused by: NumberFormatException: For input string > > "true" > > > > > > The comments in the ticket discuss adding "column:" prefix to options > for > > > the ColumnAgeOffFilter. Please correct me if I am wrong but I think it > > can > > > be fixed without having to change the syntax of the options. Wouldn't > > > adding a check to the loop in the constructor of TTLSet to ignore the > > > NEGATE option on any strings in objectStrings param prevent this error > > from > > > occurring? The negate option would still be seen by the parent, > without > > > any further changes. > > > > > > > > > That would fix it. It does not solve the case where new options are > added > > to the superclass in the future. However the option you proposed is > > infinitely better than doing nothing and I would be in favor of making > that > > change. > > > > Implementing the "column:" prefix in a backwards compatible way is very > > tricky, especially if considering the case where someone currently has an > > actual column name with that prefix. > > > > > > > > > > Looking forward to working with you folks! > > > -Mike M. > > > > > >