keith-turner commented on code in PR #4735:
URL: https://github.com/apache/accumulo/pull/4735#discussion_r1686763461
##########
core/src/main/java/org/apache/accumulo/core/data/Key.java:
##########
@@ -807,6 +807,20 @@ public ByteSequence getColumnVisibilityData() {
return new ArrayByteSequence(colVisibility);
}
+ /**
+ * Writes the column visibility into the given
<code>ArrayByteSequence</code>. This method gives
+ * users control over allocation of ArrayByteSequence objects by copying
into the passed in
+ * ArrayByteSequence.
+ *
+ * @param cv <code>ArrayByteSequence</code> object to copy into
+ * @return the <code>ArrayByteSequence</code> that was passed in
+ * @since 3.1.0
+ */
+ public ArrayByteSequence getColumnVisibilityData(ArrayByteSequence cv) {
Review Comment:
This could be a follow on PR, for consistency could add methods following
this pattern for row, family, and qualifier.
##########
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/VisibilityFilter.java:
##########
@@ -64,7 +66,7 @@ public synchronized SortedKeyValueIterator<Key,Value>
deepCopy(IteratorEnvironme
@Override
protected boolean accept(Key k, Value v) {
- ByteSequence testVis = k.getColumnVisibilityData();
+ k.getColumnVisibilityData(testVis);
Review Comment:
There may be a bug in these change where `defaultVisibility` is modified
because a previous call to this method could have done
`testVis=defaultVisibility`. So may end up modifying `defaultVisibility` when
calling ` k.getColumnVisibilityData(testVis)` here.
The general discussion of mutable vs immutable on this PR and seeing this
lead to opening the draft PR #4748.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
