WT-3228 Remove with overwrite shouldn't return WT_NOTFOUND (#3339)

* Table cursors with overwrite configured wrongly treat not-found as an error, return success instead.

* The LSM code clears WT_CURSTD_KEY_SET on unsuccessful searches, which breaks table cursors with indices doing searches on the set of cursors in order to delete old index keys, because there's no key set when it's time to do the update.

3 files changed, 59 insertions, 26 deletions

diff --git a/src/cursor/cur_table.c b/src/cursor/cur_table.c
index ef2c0ac5163..3b72bb0730f 100644
--- a/src/cursor/cur_table.c
+++ b/src/cursor/cur_table.c
@@ -625,13 +625,25 @@ __curtable_remove(WT_CURSOR *cursor)
 	/* Find the old record so it can be removed from indices */
 	if (ctable->table->nindices > 0) {
 		APPLY_CG(ctable, search);
+		if (ret == WT_NOTFOUND)
+			goto notfound;
 		WT_ERR(ret);
 		WT_ERR(__apply_idx(ctable, offsetof(WT_CURSOR, remove), false));
 	}
 
 	APPLY_CG(ctable, remove);
+	if (ret == WT_NOTFOUND)
+		goto notfound;
 	WT_ERR(ret);
 
+notfound:
+	/*
+	 * If the cursor is configured to overwrite and the record is not found,
+	 * that is exactly what we want.
+	 */
+	if (ret == WT_NOTFOUND && F_ISSET(primary, WT_CURSTD_OVERWRITE))
+		ret = 0;
+
 	/*
 	 * If the cursor was positioned, it stays positioned with a key but no
 	 * no value, otherwise, there's no position, key or value. This isn't
diff --git a/src/lsm/lsm_cursor.c b/src/lsm/lsm_cursor.c
index 2a34240de46..3f0b6df8eb0 100644
--- a/src/lsm/lsm_cursor.c
+++ b/src/lsm/lsm_cursor.c
@@ -1250,10 +1250,10 @@ __clsm_lookup(WT_CURSOR_LSM *clsm, WT_ITEM *value)
 	WT_ERR(WT_NOTFOUND);
 
 done:
-err:	F_CLR(cursor, WT_CURSTD_KEY_SET | WT_CURSTD_VALUE_SET);
-	if (ret == 0) {
-		clsm->current = c;
+err:	if (ret == 0) {
+		F_CLR(cursor, WT_CURSTD_KEY_SET | WT_CURSTD_VALUE_SET);
 		F_SET(cursor, WT_CURSTD_KEY_INT);
+		clsm->current = c;
 		if (value == &cursor->value)
 			F_SET(cursor, WT_CURSTD_VALUE_INT);
 	} else if (c != NULL)
diff --git a/test/suite/test_overwrite.py b/test/suite/test_overwrite.py
index 4739abaa578..c894de99bd0 100644
--- a/test/suite/test_overwrite.py
+++ b/test/suite/test_overwrite.py
@@ -27,32 +27,47 @@
 # OTHER DEALINGS IN THE SOFTWARE.
 
 import wiredtiger, wttest
-from wtdataset import SimpleDataSet
+from wtdataset import SimpleDataSet, SimpleIndexDataSet
+from wtdataset import SimpleLSMDataSet, ComplexDataSet, ComplexLSMDataSet
 from wtscenario import make_scenarios
 
 # test_overwrite.py
 #    cursor overwrite configuration method
 class test_overwrite(wttest.WiredTigerTestCase):
     name = 'overwrite'
-    scenarios = make_scenarios([
-        ('file-r', dict(type='file:', keyfmt='r', dataset=SimpleDataSet)),
-        ('file-S', dict(type='file:', keyfmt='S', dataset=SimpleDataSet)),
-        ('lsm-S', dict(type='lsm:', keyfmt='S', dataset=SimpleDataSet)),
-        ('table-r', dict(type='table:', keyfmt='r', dataset=SimpleDataSet)),
-        ('table-S', dict(type='table:', keyfmt='S', dataset=SimpleDataSet)),
-    ])
+    keyfmt = [
+        ('integer', dict(keyfmt='i')),
+        ('recno', dict(keyfmt='r')),
+        ('string', dict(keyfmt='S')),
+    ]
+    types = [
+        ('file', dict(uri='file:', ds=SimpleDataSet)),
+        ('lsm', dict(uri='lsm:', ds=SimpleDataSet)),
+        ('table-complex', dict(uri='table:', ds=ComplexDataSet)),
+        ('table-complex-lsm', dict(uri='table:', ds=ComplexLSMDataSet)),
+        ('table-index', dict(uri='table:', ds=SimpleIndexDataSet)),
+        ('table-simple', dict(uri='table:', ds=SimpleDataSet)),
+        ('table-simple-lsm', dict(uri='table:', ds=SimpleLSMDataSet)),
+    ]
+    scenarios = make_scenarios(types, keyfmt)
+    def skip(self):
+        return self.keyfmt == 'r' and \
+            (self.ds.is_lsm() or self.uri == 'lsm')
 
     # Confirm a cursor configured with/without overwrite correctly handles
     # non-existent records during insert, remove and update operations.
     def test_overwrite_insert(self):
-        uri = self.type + self.name
-        ds = self.dataset(self, uri, 100, key_format=self.keyfmt)
+        if self.skip():
+            return
+
+        uri = self.uri + self.name
+        ds = self.ds(self, uri, 100, key_format=self.keyfmt)
         ds.populate()
 
         # Insert of an existing record with overwrite off fails.
         cursor = self.session.open_cursor(uri, None, "overwrite=false")
         cursor.set_key(ds.key(5))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1000))
         self.assertRaises(wiredtiger.WiredTigerError, lambda: cursor.insert())
 
         # One additional test for the insert method: duplicate the cursor with
@@ -63,30 +78,33 @@ class test_overwrite(wttest.WiredTigerTestCase):
         cursor = self.session.open_cursor(uri, None, "overwrite=false")
         cursor.set_key(ds.key(5))
         dupc = self.session.open_cursor(None, cursor, "overwrite=true")
-        dupc.set_value('XXXXXXXXXX')
+        dupc.set_value(ds.value(1001))
         self.assertEquals(dupc.insert(), 0)
 
         # Insert of an existing record with overwrite on succeeds.
         cursor = self.session.open_cursor(uri, None)
         cursor.set_key(ds.key(6))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1002))
         self.assertEquals(cursor.insert(), 0)
 
         # Insert of a non-existent record with overwrite off succeeds.
         cursor = self.session.open_cursor(uri, None, "overwrite=false")
         cursor.set_key(ds.key(200))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1003))
         self.assertEquals(cursor.insert(), 0)
 
         # Insert of a non-existent record with overwrite on succeeds.
         cursor = self.session.open_cursor(uri, None)
         cursor.set_key(ds.key(201))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1004))
         self.assertEquals(cursor.insert(), 0)
 
     def test_overwrite_remove(self):
-        uri = self.type + self.name
-        ds = self.dataset(self, uri, 100, key_format=self.keyfmt)
+        if self.skip():
+            return
+
+        uri = self.uri + self.name
+        ds = self.ds(self, uri, 100, key_format=self.keyfmt)
         ds.populate()
 
         # Remove of an existing record with overwrite off succeeds.
@@ -110,32 +128,35 @@ class test_overwrite(wttest.WiredTigerTestCase):
         self.assertEquals(cursor.remove(), 0)
 
     def test_overwrite_update(self):
-        uri = self.type + self.name
-        ds = self.dataset(self, uri, 100, key_format=self.keyfmt)
+        if self.skip():
+            return
+
+        uri = self.uri + self.name
+        ds = self.ds(self, uri, 100, key_format=self.keyfmt)
         ds.populate()
 
         # Update of an existing record with overwrite off succeeds.
         cursor = self.session.open_cursor(uri, None, "overwrite=false")
         cursor.set_key(ds.key(5))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1005))
         self.assertEquals(cursor.update(), 0)
 
         # Update of an existing record with overwrite on succeeds.
         cursor = self.session.open_cursor(uri, None)
         cursor.set_key(ds.key(6))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1006))
         self.assertEquals(cursor.update(), 0)
 
         # Update of a non-existent record with overwrite off fails.
         cursor = self.session.open_cursor(uri, None, "overwrite=false")
         cursor.set_key(ds.key(200))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1007))
         self.assertEquals(cursor.update(), wiredtiger.WT_NOTFOUND)
 
         # Update of a non-existent record with overwrite on succeeds.
         cursor = self.session.open_cursor(uri, None)
         cursor.set_key(ds.key(201))
-        cursor.set_value('XXXXXXXXXX')
+        cursor.set_value(ds.value(1008))
         self.assertEquals(cursor.update(), 0)
 
 if __name__ == '__main__':