diff --git a/changelog/unreleased/SOLR-18239-fix-npe-size-estimator.yml b/changelog/unreleased/SOLR-18239-fix-npe-size-estimator.yml new file mode 100644 index 000000000000..05b9993b4527 --- /dev/null +++ b/changelog/unreleased/SOLR-18239-fix-npe-size-estimator.yml @@ -0,0 +1,7 @@ +title: Fixed NPE in size estimator for null valued fields +type: fixed +authors: + - name: Jalaz Kumar +links: + - name: SOLR-18239 + url: https://issues.apache.org/jira/browse/SOLR-18239 diff --git a/solr/core/src/java/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactory.java index e7a58d517bba..7b97d04fc843 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactory.java @@ -170,6 +170,7 @@ static long estimate(Object obj) { } private static long primitiveEstimate(Object obj, long def) { + if (obj == null) return def; Class clazz = obj.getClass(); if (clazz.isPrimitive()) { return primitiveSizes.get(clazz); diff --git a/solr/core/src/test/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactoryTest.java b/solr/core/src/test/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactoryTest.java index 513b0589157f..167aaf145682 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactoryTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/IgnoreLargeDocumentProcessorFactoryTest.java @@ -86,6 +86,7 @@ public AddUpdateCommand getUpdate(int sizeBytes) { @Test public void testEstimateObjectSize() { + assertEquals(estimate(null), 0); assertEquals(estimate("abc"), 6); assertEquals(estimate("abcdefgh"), 16); List keys = List.of("int", "long", "double", "float", "str"); @@ -99,7 +100,8 @@ public void testEstimateObjectSize() { map.put("double", 12.0); map.put("float", 5.0f); map.put("str", "duck"); - assertEquals(estimate(map), 50); + map.put("short", null); + assertEquals(estimate(map), 60); SolrInputDocument document = new SolrInputDocument(); for (Map.Entry entry : map.entrySet()) { @@ -134,7 +136,8 @@ public void testEstimateObjectSizeWithSingleChild() { mapWChild.put("double", 12.0); mapWChild.put("float", 5.0f); mapWChild.put("str", "duck"); - assertEquals(estimate(mapWChild), 50); + mapWChild.put("short", null); + assertEquals(estimate(mapWChild), 60); Map childMap = new HashMap<>(mapWChild); SolrInputDocument document = new SolrInputDocument(); @@ -175,7 +178,8 @@ public void testEstimateObjectSizeWithChildList() { mapWChild.put("double", 12.0); mapWChild.put("float", 5.0f); mapWChild.put("str", "duck"); - assertEquals(estimate(mapWChild), 50); + mapWChild.put("short", null); + assertEquals(estimate(mapWChild), 60); Map childMap = new HashMap<>(mapWChild); SolrInputDocument document = new SolrInputDocument(); diff --git a/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessor.java b/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessor.java index 3d10b3beb51a..54ad07e3d243 100644 --- a/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessor.java +++ b/solr/modules/cross-dc/src/java/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessor.java @@ -516,6 +516,7 @@ static long estimate(Object obj) { } private static long primitiveEstimate(Object obj, long def) { + if (obj == null) return def; Class clazz = obj.getClass(); if (clazz.isPrimitive()) { return primitiveSizes.get(clazz); diff --git a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessorTest.java b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessorTest.java index b8e01a62efa5..77b3adf7a494 100644 --- a/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessorTest.java +++ b/solr/modules/cross-dc/src/test/org/apache/solr/crossdc/update/processor/MirroringUpdateProcessorTest.java @@ -16,6 +16,7 @@ */ package org.apache.solr.crossdc.update.processor; +import static org.apache.solr.crossdc.update.processor.MirroringUpdateProcessor.ObjectSizeEstimator.estimate; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doAnswer; @@ -27,6 +28,8 @@ import io.opentelemetry.api.common.Attributes; import java.io.IOException; +import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; @@ -570,6 +573,40 @@ public void testProcessDBQResults() throws Exception { assertEquals(1, counters.get("submittedDeleteByQuery").get()); } + @Test + public void testEstimateObjectSize() { + assertEquals(estimate(null), 0); + assertEquals(estimate("abc"), 6); + assertEquals(estimate("abcdefgh"), 16); + List keys = List.of("int", "long", "double", "float", "str"); + assertEquals(estimate(keys), 42); + List values = List.of(12, 5L, 12.0, 5.0, "duck"); + assertEquals(estimate(values), 8); + + Map map = new HashMap<>(); + map.put("int", 12); + map.put("long", 5L); + map.put("double", 12.0); + map.put("float", 5.0f); + map.put("str", "duck"); + map.put("short", null); + assertEquals(estimate(map), 60); + + SolrInputDocument document = new SolrInputDocument(); + for (Map.Entry entry : map.entrySet()) { + document.addField(entry.getKey(), entry.getValue()); + } + assertEquals(MirroringUpdateProcessor.ObjectSizeEstimator.estimate(document), estimate(map)); + + SolrInputDocument childDocument = new SolrInputDocument(); + for (Map.Entry entry : map.entrySet()) { + childDocument.addField(entry.getKey(), entry.getValue()); + } + document.addChildDocument(childDocument); + assertEquals( + MirroringUpdateProcessor.ObjectSizeEstimator.estimate(document), estimate(map) * 2); + } + @Test public void testFinish() throws IOException { processor.finish();