Skip to content

Commit deaf62f

Browse files
committed
Bug#35046616 XDevAPI: Collection.modify().set("$",...) does not work.
Workaround to allow this update command using the same behaviour as the replaceOne(). Change-Id: Ic4210ed71fb223ea0695402edff2ce5bbc86247f
1 parent 085f771 commit deaf62f

File tree

2 files changed

+91
-7
lines changed

2 files changed

+91
-7
lines changed

common/op_impl.h

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2020, Oracle and/or its affiliates.
2+
* Copyright (c) 2015, 2023, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0, as
@@ -2209,6 +2209,8 @@ class Op_collection_add
22092209
JSON_INSERT(<json>, '$._id', <id>)
22102210
22112211
where <json> and <id> are given as constructor parameters.
2212+
2213+
<id> can either be a string or an expression which generates the <id>
22122214
*/
22132215

22142216
struct Insert_id
@@ -2219,8 +2221,14 @@ struct Insert_id
22192221
typedef cdk::string string;
22202222

22212223
const cdk::Expression &m_doc;
2224+
const cdk::Expression *m_id_exr = nullptr;
22222225
std::string m_id;
22232226

2227+
Insert_id(const cdk::Expression &doc, const cdk::Expression &id_expr)
2228+
: m_doc(doc){
2229+
m_id_exr = &id_expr;
2230+
}
2231+
22242232
Insert_id(const cdk::Expression &doc, const std::string &id)
22252233
: m_doc(doc), m_id(id)
22262234
{}
@@ -2248,7 +2256,11 @@ struct Insert_id
22482256
sprc->list_begin();
22492257
m_doc.process_if(sprc->list_el()); // the document to modify
22502258
sprc->list_el()->scalar()->val()->str("$._id");
2251-
sprc->list_el()->scalar()->val()->str(m_id);
2259+
if (m_id_exr) {
2260+
m_id_exr->process_if(sprc->list_el());
2261+
} else {
2262+
sprc->list_el()->scalar()->val()->str(m_id);
2263+
}
22522264
sprc->list_end();
22532265
}
22542266

@@ -2407,6 +2419,22 @@ class Op_collection_remove
24072419
}
24082420
};
24092421

2422+
/*
2423+
Represents an expression which generates _id.
2424+
In this case, will use the _id column value.
2425+
*/
2426+
struct Extract_id : cdk::Expression {
2427+
struct : cdk::api::Column_ref {
2428+
const cdk::api::Table_ref *table() const override { return nullptr; }
2429+
const cdk::string name() const override { return "_id"; }
2430+
} m_id;
2431+
2432+
Extract_id() {}
2433+
2434+
void process(cdk::Expression::Processor &prc) const override {
2435+
safe_prc(prc)->scalar()->ref(m_id, nullptr);
2436+
}
2437+
};
24102438

24112439
/*
24122440
Implementation of collection CRUD modify operation (Collection_modify_if
@@ -2467,7 +2495,16 @@ class Op_collection_modify
24672495
if (m_expr)
24682496
return m_expr->process(prc);
24692497

2470-
Value::Access::process(parser::Parser_mode::DOCUMENT, m_val, prc);
2498+
if(m_field == "$")
2499+
{
2500+
Value_expr doc(m_val, parser::Parser_mode::DOCUMENT);
2501+
2502+
Extract_id expr_id;
2503+
Insert_id id(doc, expr_id);
2504+
id.process(prc);
2505+
} else {
2506+
Value::Access::process(parser::Parser_mode::DOCUMENT, m_val, prc);
2507+
}
24712508
}
24722509
};
24732510

@@ -2494,8 +2531,7 @@ class Op_collection_modify
24942531
return new Op_collection_modify(*this);
24952532
}
24962533

2497-
cdk::Reply* do_send_command() override
2498-
{
2534+
cdk::Reply *do_send_command() override {
24992535
// Do nothing if no update specifications were added
25002536

25012537
if (m_update.empty())
@@ -2629,7 +2665,6 @@ class Op_collection_replace
26292665
add_operation(SET, "$", *this);
26302666
add_param("id", id);
26312667
}
2632-
26332668
};
26342669

26352670

devapi/tests/bugs-t.cc

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2020, Oracle and/or its affiliates.
2+
* Copyright (c) 2017, 2023, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0, as
@@ -963,4 +963,53 @@ TEST_F(Bugs, Bug35000027)
963963
}
964964
EXPECT_EQ(nr_docs, add_task.execute().getAffectedItemsCount());
965965
EXPECT_EQ(nr_docs, add_tast_2.execute().getAffectedItemsCount());
966+
}
967+
968+
TEST_F(Bugs, Bug35046616) {
969+
Schema sch = get_sess().createSchema("test", true);
970+
Collection coll = sch.createCollection("c1", true);
971+
972+
cout << "Inserting documents..." << endl;
973+
974+
coll.remove("true").execute();
975+
auto ids = {"1", "MYID1", "MYID2"};
976+
for (auto _id : ids) {
977+
coll.addOrReplaceOne(_id, R"({ "name": "foo", "age": 1 })");
978+
}
979+
980+
auto find_id = coll.find("_id = :id");
981+
for(auto _id : ids)
982+
{
983+
EXPECT_EQ(std::string("foo"), find_id.bind("id", _id)
984+
.execute()
985+
.fetchOne()["name"]
986+
.get<std::string>());
987+
}
988+
989+
cout << "Updating document..." << endl;
990+
991+
auto modify = coll.modify("_id = :id")
992+
.set("$", R"({ "_id": "DISCARDED", "name": "bar"})");
993+
for (auto _id : ids) {
994+
modify.bind("id", _id).execute();
995+
}
996+
997+
for (auto _id : ids) {
998+
EXPECT_EQ(std::string("bar"), find_id.bind("id", _id)
999+
.execute()
1000+
.fetchOne()["name"]
1001+
.get<std::string>());
1002+
}
1003+
1004+
// Changing all documents of the collection setting name to baz
1005+
coll.modify("true")
1006+
.set("$", R"({ "_id": "DISCARDED", "name": "baz"})")
1007+
.execute();
1008+
1009+
for (auto _id : ids) {
1010+
EXPECT_EQ(std::string("baz"), find_id.bind("id", _id)
1011+
.execute()
1012+
.fetchOne()["name"]
1013+
.get<std::string>());
1014+
}
9661015
}

0 commit comments

Comments
 (0)