accumulo-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Keith Turner (JIRA)" <>
Subject [jira] [Commented] (ACCUMULO-956) Iterator to transform key parts
Date Mon, 14 Jan 2013 18:50:12 GMT


Keith Turner commented on ACCUMULO-956:

Code review notes for patch key-transforming-iterator_2.patch.

Notes for KeyTransformingIterator :

 * Javadoc should outline why TransformingIter does vis filtering.  Someone new to this may
wonder why this iterator is doing that, and why it can't be done by a follow on iterator.
 Should also document that you need to configure the untransformed auths on the scanner and
the transformed auths on this iterator.
 * javadoc for canSee() is wrong
 * transformKeys() is only called when super.hasTop() is true, but it handles the case when
super.hasTop() is false.  I was looking to see if this case could cause a NPE, but then realized
it would never occur. Not that anything needs to be done about this, just something I observed
 * For scans if the transformation creates an invalid colvis, this would be caught. For compactions
seems like it would not be caught. May consider checking that colvis is valid for compactions,
do not need to evaluate it, just ensure it parses. Could use a seperate cache for this.  If
no auths are passed for a scan, maybe the transformation is not checked?
 * May need to transform column families for seek. Could provide a hook like the following
for the user. Seek would call this hook.  There is a bug related to column qualifiers fetched
by the user that are not passed through seek, but filtered at a lower level.  Trying to find
the ticket for this.  Column families can still be handled though.

//this function is called by seek to transform column families for seek to parent iterator
  protected Collection<ByteSequence> columnFamilies transformColumnFamilies(Collection<ByteSequence>
    return colFams;

Notes for unit test :

 * Need a unit test that transforms to vis a user can see.   I think it only transforms to
vis a user can not see.
 * Need to test deep copy
 * need to test compaction/not scanning
 * Is there a test  for a range thats before all data and a range thats after all data?
 * Need to test all combinations of range for the following conditions.  Start/end key exist
inclusive/exclusive, start/end key not exist (between existing keys) exclusive/inclusive.
I think some of the case are tested, but not all.
 * Need to test seek that passes column families that are transformed.
All of the code looks really nice. For some reason isSetAfterPart() made me smile.

> Iterator to transform key parts
> -------------------------------
>                 Key: ACCUMULO-956
>                 URL:
>             Project: Accumulo
>          Issue Type: Improvement
>            Reporter: Brian Loss
>             Fix For: 1.5.0
>         Attachments: key_transforming_iterator-1.patch, key_transforming_iterator-2.patch,
> Iterators that transform parts of the key can be tricky if any transformation affects
sort ordering.  Implement an iterator that takes care of the tricky details that come with
modifying sort order (e.g., handling scan-time iterator reconstruction and the associated

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see:

View raw message