gsmiller commented on code in PR #15823:
URL: https://github.com/apache/lucene/pull/15823#discussion_r3001448473
##########
lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java:
##########
@@ -174,6 +177,43 @@ public void addAll(Collection<T> elements) {
}
}
+ /**
+ * Adds all elements of the stream into the queue. This method should be
preferred over calling
+ * {@link #add(Object)} in loop if all elements are known in advance as it
builds queue faster.
+ *
+ * <p>If one needs to map or filter element in the iteration of elements in
this method, call this
+ * method with elements wrapped by {@link Stream#map(Function)} or {@link
+ * Stream#filter(Predicate)}, etc. In these cases, this method should be
preferred over calling
+ * {@link #addAll(Collection)}.
+ *
+ * <p>If one tries to add more objects than the maxSize passed in the
constructor, an {@link
+ * ArrayIndexOutOfBoundsException} is thrown. Which may result in parts of
elements added into the
Review Comment:
minor: Let's also mention that the heap invariant will always be maintained,
even on this error case?
##########
lucene/core/src/test/org/apache/lucene/util/TestPriorityQueue.java:
##########
@@ -159,6 +160,40 @@ public void testInsertWithOverflow() {
assertThat(pq.top(), equalTo(2));
}
+ public void testAddAllWithStream() {
+ PriorityQueue<Integer> pq = new IntegerQueue(6);
+ List<String> elements = new ArrayList<>();
+ for (String s : Arrays.asList("a", "b", "c", "d", "e", "f")) {
+ if (random().nextBoolean()) {
+ elements.addFirst(s);
+ } else {
+ elements.addLast(s);
+ }
+ }
+
+ pq.addAll(elements.stream().map(String::hashCode));
+ assertEquals("a".hashCode(), pq.top().intValue());
+ }
+
+ public void testAddAllWithStreamNotFitIntoQueue() {
+ PriorityQueue<Integer> pq = new IntegerQueue(5);
+ List<String> elements = new ArrayList<>();
+ for (String s : Arrays.asList("a", "b", "c", "d", "e", "f")) {
+ if (random().nextBoolean()) {
+ elements.addFirst(s);
+ } else {
+ elements.addLast(s);
+ }
+ }
+
+ assertThrows(
+ "Cannot add 6 elements to a queue with remaining capacity: 5",
+ ArrayIndexOutOfBoundsException.class,
+ () -> pq.addAll(elements.stream().map(String::hashCode)));
+ // Partly added.
+ assertEquals(5, pq.size());
Review Comment:
Can we also put some asserts in here to validate the created PriorityQueue
maintains the heap invariant (essentially test the fact that you're doing
heapify in the finally block correctly in case someone in the future tries to
change that)?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
