diff --git a/docs/user_technical/quick-start.md b/docs/user_technical/quick-start.md index ed321794d..92fc9a323 100644 --- a/docs/user_technical/quick-start.md +++ b/docs/user_technical/quick-start.md @@ -251,3 +251,9 @@ Memgraph processes. Secure connections are not supported in alpha. For this reason each client driver needs to be configured not to use encryption. Consult driver-specific guides for details. + + +#### Node and edge IDs +Unique node and edge IDs are returned by the Bolt protocol to the client. In Memgraph +these IDs are not guaranteed to be persistent during the whole database lifetime. They +should never be used for any client side logic outside a single query/transaction scope. diff --git a/src/communication/bolt/v1/encoder/base_encoder.hpp b/src/communication/bolt/v1/encoder/base_encoder.hpp index a2e44aa09..056205aaa 100644 --- a/src/communication/bolt/v1/encoder/base_encoder.hpp +++ b/src/communication/bolt/v1/encoder/base_encoder.hpp @@ -10,33 +10,18 @@ namespace communication::bolt { /** - * Bolt BaseEncoder. - * Has public interfaces for writing Bolt encoded data. - * Supported types are: Null, Bool, Int, Double, - * String, List, Map, Vertex, Edge + * Bolt BaseEncoder. Has public interfaces for writing Bolt encoded data. + * Supported types are: Null, Bool, Int, Double, String, List, Map, Vertex, Edge * - * This class has a dual purpose. The first is streaming of bolt data to - * network clients. The second is streaming to disk in the database snapshotter. - * The two usages are changed depending on the encode_ids boolean flag. - * If encode_ids is set to false (it's default value) then the encoder encodes - * normal bolt network client data. If encode_ids is set to true then the - * encoder encodes data for the database snapshotter. - * In the normal mode (encode_ids == false) the encoder doesn't output object - * IDs but instead it outputs fixed zeros. In the snapshotter mode it outputs - * temporary IDs obtained from the objects. - * - * Also note that currently expansion across the graph is not allowed during - * streaming, if an update has happened in the same transaction command. - * Attempting to do so crashes the DB. That's another reason why we encode - * IDs (expansion from edges) only in the snapshotter. + * This class has a dual purpose. The first is streaming of bolt data to network + * clients. The second is streaming to disk in the database snapshotter. * * @tparam Buffer the output buffer that should be used */ template class BaseEncoder { public: - BaseEncoder(Buffer &buffer, bool encode_ids = false) - : buffer_(buffer), encode_ids_(encode_ids) {} + BaseEncoder(Buffer &buffer) : buffer_(buffer) {} void WriteRAW(const uint8_t *data, uint64_t len) { buffer_.Write(data, len); } @@ -125,17 +110,7 @@ class BaseEncoder { void WriteVertex(const VertexAccessor &vertex) { WriteRAW(underlying_cast(Marker::TinyStruct) + 3); WriteRAW(underlying_cast(Signature::Node)); - - if (encode_ids_) { - // IMPORTANT: this is used only in the database snapshotter! - WriteUInt(vertex.temporary_id()); - } else { - // IMPORTANT: here we write a hardcoded 0 because we don't - // use internal IDs, but need to give something to Bolt - // note that OpenCypher has no id(x) function, so the client - // should not be able to do anything with this value anyway - WriteInt(0); - } + WriteUInt(vertex.temporary_id()); // write labels const auto &labels = vertex.labels(); @@ -156,20 +131,9 @@ class BaseEncoder { WriteRAW(underlying_cast(Marker::TinyStruct) + 5); WriteRAW(underlying_cast(Signature::Relationship)); - if (encode_ids_) { - // IMPORTANT: this is used only in the database snapshotter! - WriteUInt(edge.temporary_id()); - WriteUInt(edge.from().temporary_id()); - WriteUInt(edge.to().temporary_id()); - } else { - // IMPORTANT: here we write a hardcoded 0 because we don't - // use internal IDs, but need to give something to Bolt - // note that OpenCypher has no id(x) function, so the client - // should not be able to do anything with this value anyway - WriteInt(0); - WriteInt(0); - WriteInt(0); - } + WriteUInt(edge.temporary_id()); + WriteUInt(edge.from().temporary_id()); + WriteUInt(edge.to().temporary_id()); // write type WriteString(edge.db_accessor().edge_type_name(edge.edge_type())); @@ -225,7 +189,6 @@ class BaseEncoder { protected: Buffer &buffer_; - bool encode_ids_; private: void WriteUInt(const uint64_t &value) { diff --git a/src/durability/snapshooter.cpp b/src/durability/snapshooter.cpp index fa423563d..26a41dd97 100644 --- a/src/durability/snapshooter.cpp +++ b/src/durability/snapshooter.cpp @@ -30,8 +30,8 @@ bool Snapshooter::Encode(const fs::path &snapshot_file, GraphDbAccessor &db_accessor_) { try { FileWriterBuffer buffer; - // BaseEncoder encodes graph elements. Flag true is for storing vertex IDs. - communication::bolt::BaseEncoder encoder(buffer, true); + // BaseEncoder encodes graph elements. + communication::bolt::BaseEncoder encoder(buffer); int64_t vertex_num = 0, edge_num = 0; buffer.Open(snapshot_file); diff --git a/tests/unit/bolt_encoder.cpp b/tests/unit/bolt_encoder.cpp index 5f7995a61..03f365585 100644 --- a/tests/unit/bolt_encoder.cpp +++ b/tests/unit/bolt_encoder.cpp @@ -45,6 +45,15 @@ void CheckTypeSize(std::vector &v, int typ, uint64_t size) { ASSERT_EQ(len, size); } +void CheckInt(std::vector &output, int64_t value) { + TestSocket socket(20); + TestBuffer encoder_buffer(socket); + communication::bolt::BaseEncoder bolt_encoder(encoder_buffer); + std::vector &encoded = socket.output; + bolt_encoder.WriteInt(value); + CheckOutput(output, encoded.data(), encoded.size(), false); +} + void CheckRecordHeader(std::vector &v, uint64_t size) { CheckOutput(v, (const uint8_t *)"\xB1\x71", 2, false); CheckTypeSize(v, LIST, size); @@ -56,6 +65,7 @@ communication::bolt::Encoder bolt_encoder(encoder_buffer); std::vector &output = socket.output; TEST(BoltEncoder, NullAndBool) { + output.clear(); std::vector vals; vals.push_back(TypedValue::Null); vals.push_back(TypedValue(true)); @@ -67,6 +77,7 @@ TEST(BoltEncoder, NullAndBool) { TEST(BoltEncoder, Int) { int N = 28; + output.clear(); std::vector vals; for (int i = 0; i < N; ++i) vals.push_back(TypedValue(int_decoded[i])); bolt_encoder.MessageRecord(vals); @@ -78,6 +89,7 @@ TEST(BoltEncoder, Int) { TEST(BoltEncoder, Double) { int N = 4; + output.clear(); std::vector vals; for (int i = 0; i < N; ++i) vals.push_back(TypedValue(double_decoded[i])); bolt_encoder.MessageRecord(vals); @@ -87,6 +99,7 @@ TEST(BoltEncoder, Double) { } TEST(BoltEncoder, String) { + output.clear(); std::vector vals; for (uint64_t i = 0; i < sizes_num; ++i) vals.push_back(TypedValue(std::string((const char *)data, sizes[i]))); @@ -100,6 +113,7 @@ TEST(BoltEncoder, String) { } TEST(BoltEncoder, List) { + output.clear(); std::vector vals; for (uint64_t i = 0; i < sizes_num; ++i) { std::vector val; @@ -120,6 +134,7 @@ TEST(BoltEncoder, List) { } TEST(BoltEncoder, Map) { + output.clear(); std::vector vals; uint8_t buff[10]; for (int i = 0; i < sizes_num; ++i) { @@ -147,6 +162,8 @@ TEST(BoltEncoder, Map) { } TEST(BoltEncoder, VertexAndEdge) { + output.clear(); + // create vertex Dbms dbms; auto db_accessor = dbms.active(); @@ -174,12 +191,26 @@ TEST(BoltEncoder, VertexAndEdge) { vals.push_back(TypedValue(va2)); vals.push_back(TypedValue(ea)); bolt_encoder.MessageRecord(vals); - CheckOutput(output, vertexedge_encoded, 74); + + // The vertexedge_encoded testdata has hardcoded zeros for IDs, + // and Memgraph now encodes IDs so we need to check the output + // part by part. + CheckOutput(output, vertexedge_encoded, 5, false); + CheckInt(output, va1.temporary_id()); + CheckOutput(output, vertexedge_encoded + 6, 34, false); + CheckInt(output, va2.temporary_id()); + CheckOutput(output, vertexedge_encoded + 41, 4, false); + CheckInt(output, ea.temporary_id()); + CheckInt(output, va1.temporary_id()); + CheckInt(output, va2.temporary_id()); + CheckOutput(output, vertexedge_encoded + 48, 26); } TEST(BoltEncoder, BoltV1ExampleMessages) { // this test checks example messages from: http://boltprotocol.org/v1/ + output.clear(); + // record message std::vector rvals; for (int i = 1; i < 4; ++i) rvals.push_back(TypedValue(i));