[GitHub] [solr] magibney commented on a diff in pull request #1184: SOLR-16555: SolrIndexSearcher - FilterCache intersections/andNot should not clone bitsets repeatedly

[GitBox <gi...@apache.org>]

magibney commented on code in PR #1184:
URL: https://github.com/apache/solr/pull/1184#discussion_r1028520036


##########
solr/core/src/java/org/apache/solr/search/BitDocSet.java:
##########
@@ -194,19 +194,30 @@ public void addAllTo(FixedBitSet target) {
 
   @Override
   public DocSet andNot(DocSet other) {
-    FixedBitSet newbits = bits.clone();
+    return new BitDocSet(this.andNot(getFixedBitSetClone(), other));
+  }
+
+  /**
+   * Helper method for andNot that takes FixedBitSet and DocSet. This returns the resulting bits
+   * andNoted together.
+   *
+   * @param bits bits to operate on
+   * @param other The DocSet to compare to
+   * @return Resulting andNoted FixedBitSet
+   */
+  protected FixedBitSet andNot(FixedBitSet bits, DocSet other) {

Review Comment:
   nit: this could be a static method?
   
   Also, a couple of minor issues with javadoc wording:
   1. it could be clearer that this method modifies the docset provided as the first arg and returns that same object (making this a static method might help clarify that point).
   2. "andNoted together" implies that "andNot" is a symmetric operation, which of course it's not. Maybe something like "modifies the first arg input BitDocSet to remove all bits contained in the second DocSet arg -- equivalent to calling `a.andNot(b)`, but modifies the state of the base docset instead of returning a new docSet"?



-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org