accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Miller <michaelpmil...@gmail.com>
Subject Re: ACCUMULO-1604 Bug Fix
Date Tue, 05 Jul 2016 15:49:40 GMT
OK since the problem is really with the ColumnAgeOffFilter, I will go with
the simple check in the constructor and not make any changes to Filter.  Is
there a preferred technique of code submission for non-committers?

On Fri, Jul 1, 2016 at 3:28 PM, Keith Turner <keith@deenlo.com> wrote:

> 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