diff options
author | Tor Egge <Tor.Egge@oath.com> | 2018-08-15 12:14:30 +0000 |
---|---|---|
committer | Tor Egge <Tor.Egge@oath.com> | 2018-08-15 12:14:30 +0000 |
commit | 72d56adf3f4b89480659be393d0178dbdc18ce84 (patch) | |
tree | 2c6ffe07cd65b63936f479339d16efcaa449dd84 /document | |
parent | 23204afafb060c5fca6b7e1d632af394e72eb234 (diff) |
Reverse iteration over selection result in an attempt to
avoid stale array indexes being used as part of
applying a remove field path update.
This change assumes that variables stored in result list
have increasing values.
Add unit test for removal of multiple array elements
using remove field path update.
Diffstat (limited to 'document')
-rw-r--r-- | document/src/tests/fieldpathupdatetestcase.cpp | 28 | ||||
-rw-r--r-- | document/src/vespa/document/select/resultlist.h | 3 | ||||
-rw-r--r-- | document/src/vespa/document/update/fieldpathupdate.cpp | 2 |
3 files changed, 32 insertions, 1 deletions
diff --git a/document/src/tests/fieldpathupdatetestcase.cpp b/document/src/tests/fieldpathupdatetestcase.cpp index f5db5831912..35b851ca895 100644 --- a/document/src/tests/fieldpathupdatetestcase.cpp +++ b/document/src/tests/fieldpathupdatetestcase.cpp @@ -38,6 +38,7 @@ struct FieldPathUpdateTestCase : public CppUnit::TestFixture { void testRemoveField(); void testApplyRemoveEntireListField(); void testApplyRemoveMultiList(); + void testApplyRemoveMultiList2(); void testApplyRemoveMultiWset(); void testApplyAssignSingle(); void testApplyAssignMath(); @@ -79,6 +80,7 @@ struct FieldPathUpdateTestCase : public CppUnit::TestFixture { CPPUNIT_TEST(testRemoveField); CPPUNIT_TEST(testApplyRemoveEntireListField); CPPUNIT_TEST(testApplyRemoveMultiList); + CPPUNIT_TEST(testApplyRemoveMultiList2); CPPUNIT_TEST(testApplyRemoveMultiWset); CPPUNIT_TEST(testApplyAssignSingle); CPPUNIT_TEST(testApplyAssignMath); @@ -420,6 +422,32 @@ FieldPathUpdateTestCase::testApplyRemoveMultiList() } void +FieldPathUpdateTestCase::testApplyRemoveMultiList2() +{ + Document::UP doc(new Document(_foobar_type, DocumentId("doc:things:thangs"))); + doc->setRepo(*_repo); + CPPUNIT_ASSERT(doc->hasValue("strarray") == false); + { + ArrayFieldValue strArray(doc->getType().getField("strarray").getDataType()); + strArray.add(StringFieldValue("remove val 1")); + strArray.add(StringFieldValue("remove val 1")); + strArray.add(StringFieldValue("hello hello")); + doc->setValue("strarray", strArray); + } + CPPUNIT_ASSERT(doc->hasValue("strarray")); + //doc->print(std::cerr, true, ""); + DocumentUpdate docUp(*_repo, _foobar_type, DocumentId("doc:barbar:foofoo")); + docUp.addFieldPathUpdate(FieldPathUpdate::CP( + new RemoveFieldPathUpdate("strarray[$x]", "foobar.strarray[$x] == \"remove val 1\""))); + docUp.applyTo(*doc); + { + std::unique_ptr<ArrayFieldValue> strArray = doc->getAs<ArrayFieldValue>(doc->getField("strarray")); + CPPUNIT_ASSERT_EQUAL(std::size_t(1), strArray->size()); + CPPUNIT_ASSERT_EQUAL(vespalib::string("hello hello"), (*strArray)[0].getAsString()); + } +} + +void FieldPathUpdateTestCase::testApplyRemoveEntireListField() { Document::UP doc(new Document(_foobar_type, DocumentId("doc:things:thangs"))); diff --git a/document/src/vespa/document/select/resultlist.h b/document/src/vespa/document/select/resultlist.h index f571106f95b..cd019828ce1 100644 --- a/document/src/vespa/document/select/resultlist.h +++ b/document/src/vespa/document/select/resultlist.h @@ -14,6 +14,7 @@ public: typedef std::vector<ResultPair> Results; typedef Results::iterator iterator; typedef Results::const_iterator const_iterator; + using const_reverse_iterator = Results::const_reverse_iterator; ResultList(); ResultList(ResultList &&) = default; @@ -45,6 +46,8 @@ public: const_iterator end() const { return _results.end(); } iterator begin() { return _results.begin(); } iterator end() { return _results.end(); } + const_reverse_iterator rbegin() const { return _results.rbegin(); } + const_reverse_iterator rend() const { return _results.rend(); } private: Results _results; diff --git a/document/src/vespa/document/update/fieldpathupdate.cpp b/document/src/vespa/document/update/fieldpathupdate.cpp index fa7a8b38aba..cef5dc21360 100644 --- a/document/src/vespa/document/update/fieldpathupdate.cpp +++ b/document/src/vespa/document/update/fieldpathupdate.cpp @@ -74,7 +74,7 @@ FieldPathUpdate::applyTo(Document& doc) const } else { std::unique_ptr<select::Node> whereClause = parseDocumentSelection(_originalWhereClause, *doc.getRepo()); select::ResultList results = whereClause->contains(doc); - for (select::ResultList::const_iterator i = results.begin(); i != results.end(); ++i) { + for (select::ResultList::const_reverse_iterator i = results.rbegin(); i != results.rend(); ++i) { LOG(spam, "vars = %s", handler->getVariables().toString().c_str()); if (*i->second == select::Result::True) { handler->setVariables(i->first); |