accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Keith Turner <ke...@deenlo.com>
Subject Re: ACCUMULO-1604 Bug Fix
Date Fri, 01 Jul 2016 19:28:15 GMT
On Fri, Jul 1, 2016 at 3:24 PM, Mike Miller <michaelpmiller@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 <keith@deenlo.com> wrote:
>
> > On Fri, Jul 1, 2016 at 1:29 PM, Mike Miller <michaelpmiller@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.
> > >
> >
>

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