Skip to content

Commit 1931896

Browse files
committed
Bug#35046616 XDevAPI: Collection.modify().set("$",...) does not work.
If modify() receives string, it will be reported as string. To set whole document, user should pass DbDoc() object on modify("$",DbDoc()); Change-Id: I68a656a6e0ec0ba1b3a21869ba0156a9547a65a8
1 parent 7f97206 commit 1931896

File tree

9 files changed

+201
-156
lines changed

9 files changed

+201
-156
lines changed

common/op_impl.h

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2490,18 +2490,26 @@ class Op_collection_modify
24902490
: m_op(op), m_field(field), m_val(val)
24912491
{}
24922492

2493-
void process(Processor &prc) const
2494-
{
2495-
if (m_expr)
2496-
return m_expr->process(prc);
2497-
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);
2493+
void process(Processor &prc) const {
2494+
if (m_expr) return m_expr->process(prc);
2495+
2496+
// We send document given by a JSON string as an SQL expression
2497+
// `JSON_INSERT(val, '$', '{}')` to force server to interpret it as JSON
2498+
// value. Otherwise it would be interpreted as a literal string value,
2499+
// not a document.
2500+
if (Value::JSON == m_val.get_type()) {
2501+
struct : cdk::api::Object_ref {
2502+
const cdk::api::Schema_ref *schema() const override {
2503+
return nullptr;
2504+
}
2505+
const cdk::string name() const override { return "JSON_INSERT"; }
2506+
} json_insert;
2507+
auto json_set_args = safe_prc(prc.scalar()->call(json_insert));
2508+
json_set_args->list_begin();
2509+
json_set_args->list_el()->scalar()->val()->str(m_val.get_string());
2510+
json_set_args->list_el()->scalar()->val()->str("$");
2511+
json_set_args->list_el()->scalar()->val()->str("{}");
2512+
json_set_args->list_end();
25052513
} else {
25062514
Value::Access::process(parser::Parser_mode::DOCUMENT, m_val, prc);
25072515
}

devapi/crud.cc

Lines changed: 1 addition & 1 deletion
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

devapi/document.cc

Lines changed: 14 additions & 1 deletion
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
@@ -47,6 +47,19 @@
4747
using namespace ::mysqlx;
4848
using std::endl;
4949

50+
// Value::get specialization to allow convertion to common::Value type
51+
// --------------------
52+
53+
// We need to export this template instantiation
54+
template PUBLIC_API common::Value Value::get<common::Value>() const;
55+
56+
template <>
57+
common::Value Value::get<common::Value>() const {
58+
if (getType() == DOCUMENT) {
59+
return common::Value::Access::mk_json(m_doc.get_json());
60+
}
61+
return *this;
62+
}
5063

5164
// DbDoc implementation
5265
// --------------------

devapi/tests/bugs-t.cc

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -972,9 +972,13 @@ TEST_F(Bugs, Bug35046616) {
972972
cout << "Inserting documents..." << endl;
973973

974974
coll.remove("true").execute();
975+
975976
auto ids = {"1", "MYID1", "MYID2"};
977+
976978
for (auto _id : ids) {
977-
coll.addOrReplaceOne(_id, R"({ "name": "foo", "age": 1 })");
979+
coll.add(std::string(R"({ "_id": ")") + _id +
980+
R"(", "name": "foo", "age": 1 })")
981+
.execute();
978982
}
979983

980984
auto find_id = coll.find("_id = :id");
@@ -989,12 +993,10 @@ TEST_F(Bugs, Bug35046616) {
989993
cout << "Updating document..." << endl;
990994

991995
auto modify = coll.modify("_id = :id")
992-
.set("$", R"({ "_id": "DISCARDED", "name": "bar"})");
996+
.set("$",expr( R"({ "_id": "DISCARDED", "name": "bar"})"));
993997
for (auto _id : ids) {
994998
modify.bind("id", _id).execute();
995-
}
996999

997-
for (auto _id : ids) {
9981000
EXPECT_EQ(std::string("bar"), find_id.bind("id", _id)
9991001
.execute()
10001002
.fetchOne()["name"]
@@ -1003,7 +1005,7 @@ TEST_F(Bugs, Bug35046616) {
10031005

10041006
// Changing all documents of the collection setting name to baz
10051007
coll.modify("true")
1006-
.set("$", R"({ "_id": "DISCARDED", "name": "baz"})")
1008+
.set("$", DbDoc(R"({ "_id": "DISCARDED", "name": "baz"})"))
10071009
.execute();
10081010

10091011
for (auto _id : ids) {
@@ -1012,4 +1014,29 @@ TEST_F(Bugs, Bug35046616) {
10121014
.fetchOne()["name"]
10131015
.get<std::string>());
10141016
}
1017+
1018+
// Changing all documents of the collection setting name to a document
1019+
// {first: last:}
1020+
coll.modify("true")
1021+
.set("$.name", DbDoc(R"({"first": "foo", "last": "bar"})"))
1022+
.execute();
1023+
1024+
for (auto _id : ids) {
1025+
auto doc = find_id.bind("id", _id).execute().fetchOne();
1026+
EXPECT_EQ(Value::DOCUMENT, doc["name"].getType());
1027+
EXPECT_EQ(std::string("foo"), doc["name"]["first"].get<std::string>());
1028+
}
1029+
1030+
// Changing all documents of the collection setting name to a string with a
1031+
// JSON. JSON should not be processed.
1032+
coll.modify("true")
1033+
.set("$.name", R"({"first": "foo", "last": "bar"})")
1034+
.execute();
1035+
1036+
for (auto _id : ids) {
1037+
auto doc = find_id.bind("id", _id).execute().fetchOne();
1038+
EXPECT_EQ(Value::STRING, doc["name"].getType());
1039+
EXPECT_EQ(std::string(R"({"first": "foo", "last": "bar"})"),
1040+
doc["name"].get<std::string>());
1041+
}
10151042
}

include/mysqlx/devapi/collection_crud.h

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
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
@@ -82,6 +82,8 @@ class Session;
8282
class Collection;
8383
class Table;
8484

85+
template<>
86+
common::Value Value::get<common::Value>() const;
8587

8688
// ----------------------------------------------------------------------
8789

@@ -435,19 +437,19 @@ struct Collection_modify_base
435437
/**
436438
An operation which modifies all or selected documents in a collection.
437439
438-
Note that in operations such as `set()`, `unset()`, `arrayInsert()` and
439-
`arrayAppend()` the field argument is specified as a document path. It can be
440+
Note that in operations such as `set()`, `unset()`, `arrayInsert()` and
441+
`arrayAppend()` the field argument is specified as a document path. It can be
440442
a simple field name, but it can be a more complex expression like
441-
`$.foo.bar[3]`. One consequence of this is that document field names that
442-
contain spaces or other special characters need to be quoted, for example one
443+
`$.foo.bar[3]`. One consequence of this is that document field names that
444+
contain spaces or other special characters need to be quoted, for example one
443445
needs to use this form
444446
```
445447
.unset("\"field name with spaces\"")
446448
```
447-
as `.unset("field name with spaces")` would be an invalid document path
449+
as `.unset("field name with spaces")` would be an invalid document path
448450
expression.
449-
450-
Note also that wildcard paths that use `*` or `**` are not valid for these
451+
452+
Note also that wildcard paths that use `*` or `**` are not valid for these
451453
operations that modify documents.
452454
453455
See [MySQL Reference Manual](https://dev.mysql.com/doc/refman/en/json.html#json-path-syntax)
@@ -494,16 +496,15 @@ class CollectionModify
494496
Set the given field in a document to the given value.
495497
496498
The field is given by a document path. The value can be either a direct
497-
literal or an expression given as `expr(<string>)`, to be evaluated on
498-
the server.
499+
literal, `DbDoc` instance or an expression given as `expr(<string>)`, to be
500+
evaluated on the server.
499501
*/
500502

501503
CollectionModify& set(const Field &field, const Value &val)
502504
{
503505
try {
504-
get_impl()->add_operation(
505-
Impl::SET, field, (const common::Value&)val
506-
);
506+
get_impl()->add_operation(Impl::SET, field,
507+
val.get<common::Value>());
507508
return *this;
508509
}
509510
CATCH_AND_WRAP
@@ -534,11 +535,8 @@ class CollectionModify
534535
CollectionModify& arrayInsert(const Field &field, const Value &val)
535536
{
536537
try {
537-
get_impl()->add_operation(
538-
Impl::ARRAY_INSERT,
539-
field,
540-
(const common::Value&)val
541-
);
538+
get_impl()->add_operation(Impl::ARRAY_INSERT, field,
539+
val.get<common::Value>());
542540
return *this;
543541
}
544542
CATCH_AND_WRAP
@@ -555,11 +553,8 @@ class CollectionModify
555553
CollectionModify& arrayAppend(const Field &field, const Value &val)
556554
{
557555
try {
558-
get_impl()->add_operation(
559-
Impl::ARRAY_APPEND,
560-
field,
561-
(const common::Value&)val
562-
);
556+
get_impl()->add_operation(Impl::ARRAY_APPEND, field,
557+
val.get<common::Value>());
563558
return *this;
564559
}
565560
CATCH_AND_WRAP

include/mysqlx/devapi/crud.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
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

0 commit comments

Comments
 (0)