thomasmueller commented on code in PR #550:
URL: https://github.com/apache/jackrabbit-oak/pull/550#discussion_r857640876


##########
oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/hybrid/LuceneJournalPropertyBuilder.java:
##########
@@ -93,6 +93,9 @@ public void addSerializedProperty(@Nullable String json) {
                     }
                     if (sizeWithinLimits()) {
                         indexedNodes.put(path, reader.readString());
+                    } else {
+                        // return if max limit reached for builder to avoid 
overflow exception
+                        return;

Review Comment:
   I can see why the old code is broken: if the size is exceeded, we don't call 
readString(), but continue and try to read a comma...
   
   But I also don't like the new code: we wouldn't detect any errors in the 
Json format afterwards... Which I would expect we do.
   
   Instead of return, what about:
   
   ```
   String x = reader.readString()
   if (sizeWithinLimits()) {
       indexedNodes.put(path, x);
   }
   ```
   
   Sure, one could argue it is more efficient to return, but I think the side 
effect of _not_ parsing the rest in this case is worse than the performance hit 
(which is unlikely to be measurable).



-- 
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: dev-unsubscr...@jackrabbit.apache.org

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

Reply via email to