-
Notifications
You must be signed in to change notification settings - Fork 461
add DeepCopy to ColumnVisibility #5217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -125,6 +125,24 @@ public Node(int start, int end) { | |||||
this.end = end; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates a new node by performing a deep copy of an existing node object | ||||||
* | ||||||
* @param node Node object | ||||||
* @since 2.1.4 | ||||||
*/ | ||||||
public Node(Node node) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since Node is removed in newer versions of accumulo this constructor could be switched to private to avoid standardizing its usage
Suggested change
|
||||||
List<Node> childrenNew = | ||||||
node.children.isEmpty() ? EMPTY : new ArrayList<>(node.children.size()); | ||||||
for (Node child : node.children) { | ||||||
childrenNew.add(new Node(child)); | ||||||
} | ||||||
this.type = node.type; | ||||||
this.start = node.start; | ||||||
this.end = node.end; | ||||||
this.children = childrenNew; | ||||||
} | ||||||
|
||||||
public void add(Node child) { | ||||||
if (children == EMPTY) { | ||||||
children = new ArrayList<>(); | ||||||
|
@@ -505,6 +523,19 @@ public ColumnVisibility(byte[] expression) { | |||||
validate(expression); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Creates a new column visibility by performing a deep copy of an existing column visibility | ||||||
* object | ||||||
* | ||||||
* @param visibility ColumnVisibility object | ||||||
* @since 2.1.4 | ||||||
*/ | ||||||
public ColumnVisibility(ColumnVisibility visibility) { | ||||||
jalphonso marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
byte[] incomingExpression = visibility.expression; | ||||||
this.expression = Arrays.copyOf(incomingExpression, incomingExpression.length); | ||||||
this.node = new Node(visibility.node); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a comment in #5293 that is applicable here also. I think it's possible that another thread could be mutating the parse tree (visibility.node) while this thread is trying to make a copy of the ColumnVisibility because the parse tree is accessible and mutable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am experimenting w/ adding an immutable parse tree to accumulo-access as a possible way to address this. |
||||||
} | ||||||
|
||||||
@Override | ||||||
public String toString() { | ||||||
return "[" + new String(expression, UTF_8) + "]"; | ||||||
|
Uh oh!
There was an error while loading. Please reload this page.