fix: Support Returning Edge Properties Immediately After Edge Creation#309
fix: Support Returning Edge Properties Immediately After Edge Creation#309liulx20 wants to merge 3 commits intoalibaba:mainfrom
Conversation
| const std::vector<Property>& properties) override { | ||
| bool ok = | ||
| txn_.AddEdge(src_label, src, dst_label, dst, edge_label, properties); | ||
| return ok ? reinterpret_cast<const void*>(1) : nullptr; |
There was a problem hiding this comment.
What is the expected behavior for dealing with the returned reinterpret_cast<const void*>(1)
There was a problem hiding this comment.
Placeholder, cann't attempt to read this property value during insert-transaction.
| virtual bool AddEdge(label_t src_label, vid_t src, label_t dst_label, | ||
| vid_t dst, label_t edge_label, | ||
| const std::vector<Property>& properties) override = 0; | ||
| virtual const void* AddEdge( |
There was a problem hiding this comment.
Is there any better choice to wrap the returned edge, rather than returning a implementation-specific void* pointer?
There was a problem hiding this comment.
Pull request overview
This PR refactors the edge insertion path so that AddEdge/put_edge can return a reference to the newly inserted edge’s payload, enabling Cypher CREATE ... RETURN e.<prop> to surface edge properties immediately after creation.
Changes:
- Change
AddEdgereturn types across storage/transaction layers to return an edge-data pointer/handle (and, internally, a{offset, ptr}pair). - Plumb the returned edge payload pointer into execution (
CreateEdge) so returned edge columns carry property access data. - Add a Python binding test asserting
RETURN e.sinceworks immediately afterCREATE.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/python_bind/tests/test_db_query.py | Adds coverage for returning an edge property immediately after edge creation. |
| src/transaction/update_transaction.cc | Updates update-transaction edge insertion to return the inserted edge payload pointer. |
| src/transaction/insert_transaction.cc | Updates insert-transaction edge insertion API to return a pointer-like success indicator. |
| src/storages/graph/property_graph.cc | Changes PropertyGraph::AddEdge to return {offset, data_ptr} from the underlying edge table. |
| src/storages/graph/graph_interface.cc | Updates AP update interface AddEdge to return the inserted edge payload pointer. |
| src/storages/graph/edge_table.cc | Propagates CSR put_generic_edge return pair and returns {oe_offset, data_ptr}. |
| src/execution/common/operators/insert/create_edge.cc | Stores returned edge payload pointer into edge columns so edge property accessors can read it. |
| include/neug/transaction/update_transaction.h | Updates update transaction and TP update interface AddEdge signatures to return const void*. |
| include/neug/transaction/insert_transaction.h | Updates insert transaction and TP insert interface AddEdge signatures/docs to return const void*. |
| include/neug/storages/graph/property_graph.h | Updates PropertyGraph::AddEdge signature to return std::pair<int32_t, const void*>. |
| include/neug/storages/graph/graph_interface.h | Updates StorageInsertInterface/StorageUpdateInterface AddEdge return types and documentation. |
| include/neug/storages/graph/edge_table.h | Updates EdgeTable::AddEdge signature to return {offset, data_ptr}. |
| include/neug/storages/csr/mutable_csr.h | Updates CSR put_edge implementations to return {prev_size, data_ptr}. |
| include/neug/storages/csr/csr_base.h | Updates CSR base virtuals to return {offset, data_ptr} pairs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| nbr.data = data; | ||
| nbr.timestamp.store(ts); | ||
| edge_num_.fetch_add(1); | ||
| const void* data_ptr = static_cast<const void*>(&nbr.data); |
| InsertEdgeRedo::Serialize(arc_, src_label, src, dst_label, dst, edge_label, | ||
| properties); | ||
| return true; | ||
| return reinterpret_cast<const void*>(1); | ||
| } |
| result = conn.execute( | ||
| "MATCH (a:person {id: 1}), (b:person {id: 2}) " | ||
| "CREATE (a)-[e:knows {since: 2024}]->(b) " | ||
| "RETURN e.since;" | ||
| ) | ||
|
|
||
| records = list(result) | ||
| assert records == [[2024]], f"Expected [[2024]], got {records}" |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This pull request refactors the edge insertion API throughout the codebase to return additional information when adding edges. Instead of returning only a status or offset, the relevant
AddEdgeandput_edgemethods now return a pair containing the previous offset and a pointer to the edge data. This change provides more context to the caller and enables more advanced use cases, such as direct access to the newly inserted edge's data.