This is an automated email from the ASF dual-hosted git repository.
yangxk1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-graphar.git
The following commit(s) were added to refs/heads/main by this push:
new 9816a9ee fix(cpp): remove UB when reading unset builder vertex id
(#892)
9816a9ee is described below
commit 9816a9eebea929d93a881d16b0bd33555dc7d6c7
Author: Jinye Wu <[email protected]>
AuthorDate: Sun Aug 16 23:41:43 2026 +0800
fix(cpp): remove UB when reading unset builder vertex id (#892)
* use optional and remove empty_
* chore: trigger CI
---
cpp/src/graphar/high-level/vertices_builder.h | 25 ++++++++++++++++---------
cpp/test/test_builder.cc | 17 +++++++++++++++++
python/src/bindings/high_level_binding.cc | 2 +-
python/test/test_high_level_api.py | 10 ++++++++++
4 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/cpp/src/graphar/high-level/vertices_builder.h
b/cpp/src/graphar/high-level/vertices_builder.h
index 019df6f0..4ddccada 100644
--- a/cpp/src/graphar/high-level/vertices_builder.h
+++ b/cpp/src/graphar/high-level/vertices_builder.h
@@ -23,6 +23,7 @@
#include <cassert>
#include <cstddef>
#include <memory>
+#include <optional>
#include <string>
#include <unordered_map>
#include <unordered_set>
@@ -51,21 +52,30 @@ namespace graphar::builder {
*/
class Vertex {
public:
- Vertex() : empty_(true) {}
+ Vertex() = default;
/**
* @brief Initialize the vertex with a given id.
*
* @param id The id of the vertex.
*/
- explicit Vertex(IdType id) : id_(id), empty_(false) {}
+ explicit Vertex(IdType id) : id_(id) {}
/**
* @brief Get id of the vertex.
*
+ * The id is absent until explicitly set or assigned by VerticesBuilder.
+ *
* @return The id of the vertex.
*/
- IdType GetId() const noexcept { return id_; }
+ IdType GetId() const { return id_.value(); }
+
+ /**
+ * @brief Check if the vertex id has been initialized.
+ *
+ * @return true/false.
+ */
+ bool HasId() const noexcept { return id_.has_value(); }
/**
* @brief Set id of the vertex.
@@ -75,11 +85,11 @@ class Vertex {
void SetId(IdType id) { id_ = id; }
/**
- * @brief Check if the vertex is empty.
+ * @brief Check if the vertex contains no property payload.
*
* @return true/false.
*/
- bool Empty() const noexcept { return empty_; }
+ bool Empty() const noexcept { return properties_.empty(); }
/**
* @brief Add a property to the vertex.
@@ -89,7 +99,6 @@ class Vertex {
*/
// TODO(@acezen): Enable the property to be a vector(list).
void AddProperty(const std::string& name, const std::any& val) {
- empty_ = false;
properties_[name] = val;
}
@@ -100,7 +109,6 @@ class Vertex {
AddProperty(name, val);
return;
}
- empty_ = false;
if (cardinalities_.find(name) != cardinalities_.end()) {
if (cardinalities_[name] != cardinality) {
throw std::runtime_error("Cardinality mismatch for property: " + name);
@@ -211,8 +219,7 @@ class Vertex {
}
private:
- IdType id_;
- bool empty_;
+ std::optional<IdType> id_;
std::unordered_map<std::string, std::any> properties_;
std::unordered_map<std::string, Cardinality> cardinalities_;
};
diff --git a/cpp/test/test_builder.cc b/cpp/test/test_builder.cc
index b97648f2..244f0d8c 100644
--- a/cpp/test/test_builder.cc
+++ b/cpp/test/test_builder.cc
@@ -67,6 +67,18 @@ TEST_CASE_METHOD(GlobalFixture, "Test_vertices_builder") {
builder->SetValidateLevel(ValidateLevel::strong_validate);
REQUIRE(builder->GetValidateLevel() == ValidateLevel::strong_validate);
+ // vertex id and payload state are tracked independently
+ builder::Vertex empty_vertex;
+ REQUIRE_FALSE(empty_vertex.HasId());
+ REQUIRE(empty_vertex.Empty());
+ REQUIRE_THROWS_AS(empty_vertex.GetId(), std::bad_optional_access);
+ empty_vertex.SetId(42);
+ REQUIRE(empty_vertex.HasId());
+ REQUIRE(empty_vertex.GetId() == 42);
+ REQUIRE(empty_vertex.Empty());
+ empty_vertex.AddProperty("id", int64_t{42});
+ REQUIRE_FALSE(empty_vertex.Empty());
+
// check different validate levels
builder::Vertex v;
v.AddProperty("id", "id_of_string");
@@ -84,6 +96,11 @@ TEST_CASE_METHOD(GlobalFixture, "Test_vertices_builder") {
builder->Clear();
REQUIRE(builder->GetNum() == 0);
+ builder::Vertex indexed_vertex(7);
+ REQUIRE(indexed_vertex.HasId());
+ REQUIRE(indexed_vertex.GetId() == 7);
+ REQUIRE(indexed_vertex.Empty());
+
// add vertices
std::ifstream fp(test_data_dir + "/ldbc_sample/person_0_0.csv");
std::string line;
diff --git a/python/src/bindings/high_level_binding.cc
b/python/src/bindings/high_level_binding.cc
index d13fda76..15777c64 100644
--- a/python/src/bindings/high_level_binding.cc
+++ b/python/src/bindings/high_level_binding.cc
@@ -415,4 +415,4 @@ extern "C" void bind_high_level_api(pybind11::module_& m) {
py::arg("dst_type"), py::arg("adj_list_type"), py::arg("num_vertices"),
py::arg("writer_options") = nullptr,
py::arg("validate_level") = graphar::ValidateLevel::no_validate);
-} // namespace graphar
\ No newline at end of file
+} // namespace graphar
diff --git a/python/test/test_high_level_api.py
b/python/test/test_high_level_api.py
index 8ab2bcbb..cc76f48d 100644
--- a/python/test/test_high_level_api.py
+++ b/python/test/test_high_level_api.py
@@ -111,6 +111,16 @@ def test_vertices_builder(sample_graph_vertex):
# Set validate level
builder.SetValidateLevel(ValidateLevel.strong_validate)
+ empty_vertex = BuilderVertex()
+ assert empty_vertex.Empty()
+ with pytest.raises(Exception):
+ empty_vertex.GetId()
+ empty_vertex.SetId(42)
+ assert empty_vertex.GetId() == 42
+ assert empty_vertex.Empty()
+ empty_vertex.AddProperty("id", 42)
+ assert not empty_vertex.Empty()
+
# Prepare vertex data
vertex_count = 3
property_names = ["id", "firstName", "lastName", "gender"]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
