Feel free to use `git-am` to create a patch or just submit a pull-request against the master branch of apache/accumulo on Github.

Mike Miller wrote:
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<ke...@deenlo.com>  wrote:

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.


Reply via email to