msfroh commented on PR #13028:
URL: https://github.com/apache/lucene/pull/13028#issuecomment-1904540802
Hey -- I was just telling a colleague about this change and he asked about
the synchronization on `initChildrenSiblings`. I talked through the logic in
`computeChildrenSiblings` and realized that we very likely have another bug:
```
for (int i = first; i < length; i++) {
int parent = parents.get(i);
// The existing youngest child of the parent is the next older sibling
of i.
// note that parents[i] is always < i, so the right-hand-side of
// the following line is already set when we get here
siblings.set(i, children.get(parent));
// The new youngest child of the parent is i.
children.set(parent, i);
}
```
Since `first` is the index of the first new element, and `parent < i`
always, we can guarantee that at least one `parent` is an "old" ordinal, likely
shared with the old `TaxonomyIndexArray` (unless the parent is also in the last
chunk). If we add a new top-level category, then `parent` will be `0`, which is
always going to be from one of our old, supposedly "immutable" chunks.
I think we need to deep-copy any `children` chunks for parents of the
newly-added ordinals.
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
