Route document removes only to the recipient(s) handling the document type (as given in the document id).

For legacy document ids (without document type) we still use all the routes.

5 files changed, 36 insertions, 1 deletions

diff --git a/document/src/main/java/com/yahoo/document/select/rule/AttributeNode.java b/document/src/main/java/com/yahoo/document/select/rule/AttributeNode.java
index bbd244dd1dc..294ffafa7e7 100644
--- a/document/src/main/java/com/yahoo/document/select/rule/AttributeNode.java
+++ b/document/src/main/java/com/yahoo/document/select/rule/AttributeNode.java
@@ -133,6 +133,8 @@ public class AttributeNode implements ExpressionNode {
             return handler.values;
         } else if (value instanceof DocumentUpdate) {
             return Result.INVALID;
+        } else if (value instanceof DocumentRemove) {
+            return Result.INVALID;
         }
         return Result.FALSE;
     }
diff --git a/document/src/main/java/com/yahoo/document/select/rule/DocumentNode.java b/document/src/main/java/com/yahoo/document/select/rule/DocumentNode.java
index 242413c8d3e..d12e8d20fa9 100644
--- a/document/src/main/java/com/yahoo/document/select/rule/DocumentNode.java
+++ b/document/src/main/java/com/yahoo/document/select/rule/DocumentNode.java
@@ -43,6 +43,9 @@ public class DocumentNode implements ExpressionNode {
             doct = ((DocumentPut)op).getDocument().getDataType();
         } else if (op instanceof DocumentUpdate) {
             doct = ((DocumentUpdate)op).getDocumentType();
+        } else if (op instanceof DocumentRemove) {
+            DocumentRemove removeOp = (DocumentRemove)op;
+            return (removeOp.getId().getDocType().equals(type) ? op : Boolean.FALSE);
         } else {
             throw new IllegalStateException("Document class '" + op.getClass().getName() + "' is not supported.");
         }
diff --git a/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java b/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java
index 508bf7f0b18..5286cec4644 100644
--- a/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java
+++ b/document/src/test/java/com/yahoo/document/select/DocumentSelectorTestCase.java
@@ -250,6 +250,21 @@ public class DocumentSelectorTestCase extends junit.framework.TestCase {
         // TODO Fails: assertEquals(Result.TRUE, evaluate("test.hint + 1 > 13", upd));
     }
 
+    public void testDocumentRemove() throws ParseException {
+        assertEquals(Result.TRUE, evaluate("test", createRemove("id:ns:test::1")));
+        assertEquals(Result.FALSE, evaluate("test", createRemove("id:ns:null::1")));
+        assertEquals(Result.FALSE, evaluate("test", createRemove("userdoc:test:1234:1")));
+        assertEquals(Result.INVALID, evaluate("test.hint", createRemove("id:ns:test::1")));
+        assertEquals(Result.FALSE, evaluate("test.hint", createRemove("id:ns:null::1")));
+        assertEquals(Result.INVALID, evaluate("test.hint == 0", createRemove("id:ns:test::1")));
+        assertEquals(Result.INVALID, evaluate("test.anything", createRemove("id:ns:test::1")));
+        assertEquals(Result.INVALID, evaluate("test and test.hint == 0", createRemove("id:ns:test::1")));
+    }
+
+    private DocumentRemove createRemove(String docId) {
+        return new DocumentRemove(new DocumentId(docId));
+    }
+
     public void testInvalidLogic() throws ParseException {
         DocumentPut put = new DocumentPut(manager.getDocumentType("test"), new DocumentId("doc:scheme:"));
         DocumentUpdate upd = new DocumentUpdate(manager.getDocumentType("test"), new DocumentId("doc:scheme:"));
diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp
index db7b48cdc3a..410ac539ff4 100644
--- a/document/src/tests/documentselectparsertest.cpp
+++ b/document/src/tests/documentselectparsertest.cpp
@@ -37,6 +37,7 @@ class DocumentSelectParserTest : public CppUnit::TestFixture {
     CPPUNIT_TEST(testThatComplexFieldValuesHaveCorrectFieldNames);
     CPPUNIT_TEST(testBodyFieldDetection);
     CPPUNIT_TEST(testDocumentUpdates);
+    CPPUNIT_TEST(testDocumentIdsInRemoves);
     CPPUNIT_TEST(test_syntax_error_reporting);
     CPPUNIT_TEST(test_operator_precedence);
     CPPUNIT_TEST(test_token_used_as_ident_preserves_casing);
@@ -102,6 +103,7 @@ public:
     void testDocumentUpdates2();
     void testDocumentUpdates3();
     void testDocumentUpdates4();
+    void testDocumentIdsInRemoves();
     void test_syntax_error_reporting();
     void test_operator_precedence();
     void test_token_used_as_ident_preserves_casing();
@@ -1238,6 +1240,19 @@ void DocumentSelectParserTest::testDocumentUpdates4()
     PARSEI("-6 % 10 = -6", *_update[0], True);
 }
 
+void DocumentSelectParserTest::testDocumentIdsInRemoves()
+{
+    PARSE("testdoctype1", DocumentId("id:ns:testdoctype1::1"), True);
+    PARSE("testdoctype1", DocumentId("id:ns:null::1"), False);
+    PARSE("testdoctype1", DocumentId("userdoc:testdoctype1:1234:1"), False);
+    PARSE("testdoctype1.headerval", DocumentId("id:ns:testdoctype1::1"), Invalid);
+    // FIXME: Should ideally be False. As long as there always is an AND node with doctype in a selection expression
+    // we won't end up sending removes using the wrong route.
+    PARSE("testdoctype1.headerval", DocumentId("id:ns:null::1"), Invalid);
+    PARSE("testdoctype1.headerval == 0", DocumentId("id:ns:testdoctype1::1"), Invalid);
+    PARSE("testdoctype1 and testdoctype1.headerval == 0", DocumentId("id:ns:testdoctype1::1"), Invalid);
+}
+
 void DocumentSelectParserTest::testUtf8()
 {
     createDocs();
diff --git a/document/src/vespa/document/select/doctype.cpp b/document/src/vespa/document/select/doctype.cpp
index bbaba0ae7cf..4ecb388f60d 100644
--- a/document/src/vespa/document/select/doctype.cpp
+++ b/document/src/vespa/document/select/doctype.cpp
@@ -41,7 +41,7 @@ DocType::contains(const Context &context) const
                                        _doctype)));
     }
     if (context._docId != NULL) {
-        return ResultList(Result::False);
+        return ResultList(Result::get((context._docId->getDocType() == _doctype)));
     }
     const DocumentUpdate &upd(*context._docUpdate);
     return ResultList(Result::get(