From c8bf11d20243c136900444f48627c4a158474436 Mon Sep 17 00:00:00 2001 From: Erick Erickson Date: Thu, 17 Apr 2014 22:46:38 +0000 Subject: [PATCH] add 'remove' as update option for atomically removing a value from a multivalued field git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1588385 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 + .../processor/DistributedUpdateProcessor.java | 104 +++++---- .../update/processor/AtomicUpdatesTest.java | 197 ++++++++++++++++++ 3 files changed, 266 insertions(+), 38 deletions(-) create mode 100644 solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index ee4d9c18c94..62db0ecf7ee 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -116,6 +116,9 @@ Other Changes * SOLR-5987: Add "collection" to UpdateParams. (Mark Miller, Greg Solovyev) +* SOLR-3862: Add remove" as update option for atomically removing a value + from a multivalued field (Jim Musli, Steven Bower, Alaknantha via Erick Erickson) + ================== 4.8.0 ================== Versions of Major Components diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java index 37c935e9aa1..be87d7ab124 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java @@ -905,49 +905,34 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { IndexSchema schema = cmd.getReq().getSchema(); for (SolrInputField sif : sdoc.values()) { - Object val = sif.getValue(); + Object val = sif.getValue(); if (val instanceof Map) { for (Entry entry : ((Map) val).entrySet()) { String key = entry.getKey(); Object fieldVal = entry.getValue(); boolean updateField = false; - if ("add".equals(key)) { - updateField = true; - oldDoc.addField( sif.getName(), fieldVal, sif.getBoost()); - } else if ("set".equals(key)) { - updateField = true; - oldDoc.setField(sif.getName(), fieldVal, sif.getBoost()); - } else if ("inc".equals(key)) { - updateField = true; - SolrInputField numericField = oldDoc.get(sif.getName()); - if (numericField == null) { - oldDoc.setField(sif.getName(), fieldVal, sif.getBoost()); - } else { - // TODO: fieldtype needs externalToObject? - String oldValS = numericField.getFirstValue().toString(); - SchemaField sf = schema.getField(sif.getName()); - BytesRef term = new BytesRef(); - sf.getType().readableToIndexed(oldValS, term); - Object oldVal = sf.getType().toObject(sf, term); - - String fieldValS = fieldVal.toString(); - Number result; - if (oldVal instanceof Long) { - result = ((Long) oldVal).longValue() + Long.parseLong(fieldValS); - } else if (oldVal instanceof Float) { - result = ((Float) oldVal).floatValue() + Float.parseFloat(fieldValS); - } else if (oldVal instanceof Double) { - result = ((Double) oldVal).doubleValue() + Double.parseDouble(fieldValS); - } else { - // int, short, byte - result = ((Integer) oldVal).intValue() + Integer.parseInt(fieldValS); - } - - oldDoc.setField(sif.getName(), result, sif.getBoost()); - } - + switch (key) { + case "add": + updateField = true; + oldDoc.addField(sif.getName(), fieldVal, sif.getBoost()); + break; + case "set": + updateField = true; + oldDoc.setField(sif.getName(), fieldVal, sif.getBoost()); + break; + case "remove": + updateField = true; + doRemove(oldDoc, sif, fieldVal); + break; + case "inc": + updateField = true; + doInc(oldDoc, schema, sif, fieldVal); + break; + default: + //Perhaps throw an error here instead? + log.warn("Unknown operation for the an atomic update, operation ignored: " + key); + break; } - // validate that the field being modified is not the id field. if (updateField && idField.getName().equals(sif.getName())) { throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid update of id field: " + sif); @@ -965,9 +950,52 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { return true; } + private void doInc(SolrInputDocument oldDoc, IndexSchema schema, SolrInputField sif, Object fieldVal) { + SolrInputField numericField = oldDoc.get(sif.getName()); + if (numericField == null) { + oldDoc.setField(sif.getName(), fieldVal, sif.getBoost()); + } else { + // TODO: fieldtype needs externalToObject? + String oldValS = numericField.getFirstValue().toString(); + SchemaField sf = schema.getField(sif.getName()); + BytesRef term = new BytesRef(); + sf.getType().readableToIndexed(oldValS, term); + Object oldVal = sf.getType().toObject(sf, term); + + String fieldValS = fieldVal.toString(); + Number result; + if (oldVal instanceof Long) { + result = ((Long) oldVal).longValue() + Long.parseLong(fieldValS); + } else if (oldVal instanceof Float) { + result = ((Float) oldVal).floatValue() + Float.parseFloat(fieldValS); + } else if (oldVal instanceof Double) { + result = ((Double) oldVal).doubleValue() + Double.parseDouble(fieldValS); + } else { + // int, short, byte + result = ((Integer) oldVal).intValue() + Integer.parseInt(fieldValS); + } + + oldDoc.setField(sif.getName(), result, sif.getBoost()); + } + } + + private void doRemove(SolrInputDocument oldDoc, SolrInputField sif, Object fieldVal) { + final String name = sif.getName(); + SolrInputField existingField = oldDoc.get(name); + if (existingField != null) { + final Collection original = existingField.getValues(); + if (fieldVal instanceof Collection) { + original.removeAll((Collection) fieldVal); + } else { + original.remove(fieldVal); + } + + oldDoc.setField(name, original); + + } + } - @Override public void processDelete(DeleteUpdateCommand cmd) throws IOException { updateCommand = cmd; diff --git a/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java b/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java new file mode 100644 index 00000000000..7f22031a190 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java @@ -0,0 +1,197 @@ +package org.apache.solr.update.processor; + +import com.google.common.collect.ImmutableMap; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrInputDocument; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +public class AtomicUpdatesTest extends SolrTestCaseJ4 { + + @BeforeClass + public static void beforeTests() throws Exception { + System.setProperty("enable.update.log", "true"); + initCore("solrconfig.xml", "schema.xml"); + } + + @Before + public void before() { + h.update("*:*"); + assertU(commit()); + } + @Test + public void testRemove() throws Exception { + SolrInputDocument doc; + + doc = new SolrInputDocument(); + doc.setField("id", "1"); + doc.setField("cat", new String[]{"aaa", "bbb", "ccc", "ccc", "ddd"}); + assertU(adoc(doc)); + + doc = new SolrInputDocument(); + doc.setField("id", "2"); + doc.setField("cat", new String[]{"aaa", "bbb", "bbb", "ccc", "ddd"}); + assertU(adoc(doc)); + + + doc = new SolrInputDocument(); + doc.setField("id", "20"); + doc.setField("cat", new String[]{"aaa", "ccc", "ddd"}); + assertU(adoc(doc)); + + doc = new SolrInputDocument(); + doc.setField("id", "21"); + doc.setField("cat", new String[]{"aaa", "bbb", "ddd"}); + assertU(adoc(doc)); + + + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '3']"); + + + doc = new SolrInputDocument(); + doc.setField("id", "1"); + List removeList = new ArrayList(); + removeList.add("bbb"); + removeList.add("ccc"); + doc.setField("cat", ImmutableMap.of("remove", removeList)); //behavior when hitting Solr through ZK + assertU(adoc(doc)); + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '2']"); + + doc = new SolrInputDocument(); + doc.setField("id", "21"); + removeList = new ArrayList(); + removeList.add("bbb"); + removeList.add("ccc"); + doc.setField("cat", ImmutableMap.of("remove", removeList)); //behavior when hitting Solr through ZK + assertU(adoc(doc)); + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']"); + + doc = new SolrInputDocument(); + doc.setField("id", "1"); + doc.setField("cat", ImmutableMap.of("remove", "aaa")); //behavior when hitting Solr directly + + assertU(adoc(doc)); + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '4']"); + assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '3']"); + } + + @Test + public void testAdd() throws Exception { + SolrInputDocument doc = new SolrInputDocument(); + doc.setField("id", "3"); + doc.setField("cat", new String[]{"aaa", "ccc"}); + assertU(adoc(doc)); + + doc = new SolrInputDocument(); + doc.setField("id", "4"); + doc.setField("cat", new String[]{"aaa", "ccc"}); + assertU(adoc(doc)); + + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']"); + + + doc = new SolrInputDocument(); + doc.setField("id", "3"); + doc.setField("cat", ImmutableMap.of("add", "bbb")); + assertU(adoc(doc)); + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']"); + } + + @Test + public void testSet() throws Exception { + SolrInputDocument doc; + + doc = new SolrInputDocument(); + doc.setField("id", "5"); + doc.setField("cat", new String[]{"aaa", "ccc"}); + assertU(adoc(doc)); + + doc = new SolrInputDocument(); + doc.setField("id", "6"); + doc.setField("cat", new String[]{"aaa", "ccc"}); + assertU(adoc(doc)); + + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']"); + assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '2']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']"); + assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '2']"); + + + doc = new SolrInputDocument(); + doc.setField("id", "5"); + doc.setField("cat", ImmutableMap.of("set", "bbb")); + assertU(adoc(doc)); + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '2']"); + assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '1']"); + assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']"); + } + + @Test + public void testInvalidOperation() { + SolrInputDocument doc; + + doc = new SolrInputDocument(); + doc.setField("id", "7"); + doc.setField("cat", new String[]{"aaa", "ccc"}); + assertU(adoc(doc)); + + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '1']"); + + doc = new SolrInputDocument(); + doc.setField("id", "7"); + doc.setField("cat", ImmutableMap.of("whatever", "bbb")); + assertU(adoc(doc)); + assertU(commit()); + + assertQ(req("q", "cat:*", "indent", "true"), "//result[@numFound = '1']"); + assertQ(req("q", "cat:aaa", "indent", "true"), "//result[@numFound = '1']"); + assertQ(req("q", "cat:bbb", "indent", "true"), "//result[@numFound = '0']"); + assertQ(req("q", "cat:ccc", "indent", "true"), "//result[@numFound = '1']"); + + } +}