From b2367921062641d0dc8de19cfcebba1c2ca53a16 Mon Sep 17 00:00:00 2001 From: Remy Boutonnet Date: Fri, 23 Oct 2026 13:48:48 +0200 Subject: [PATCH 1/2] Storage: introduce GraphPath, drop EntityList --- docs/ShortestPath/Guide.md | 42 +++--- docs/ShortestPath/shortest_path.mdx | 30 ++--- memory/LocalMemory.h | 3 +- net/decoders/DecodeContext.h | 9 +- net/decoders/Decoders.h | 120 ++++------------- net/decoders/TuringProtoDecoder.h | 9 +- net/decoders/TuringProtoDecoderConcepts.h | 4 +- net/encoders/JsonEncoder.h | 52 +++---- net/encoders/OutputValues.h | 8 +- net/encoders/TuringProtoEncoder.h | 72 +++------- net/turing_proto_common/TuringProtoHeaders.h | 5 +- python/turingdb/_common/NanobindUtils.cpp | 32 +++-- query/ir/codegen/DBProgramGenerator.cpp | 8 +- query/ir/codegen/DBProgramGenerator.h | 4 +- query/ir/dialect/db/DBOps.cpp | 2 +- query/ir/dialect/db/DBOps.td | 38 +++--- query/ir/dialect/db/DBTypes.td | 1 - query/ir/dialect/nl/NLOps.td | 18 +-- query/ir/dialect/storage/StorageTypes.td | 16 +-- query/ir/exec/NLExecutor.cpp | 127 +++++++----------- query/ir/exec/NLProgram.h | 27 ++-- query/ir/exec/NLTranslator.cpp | 21 +-- query/ir/exec/NLTranslator.h | 1 - query/ir/lowering/DBLowering.cpp | 6 +- samples/mlir/main.cpp | 25 ++-- samples/tfl/main.cpp | 46 +++---- storage/CMakeLists.txt | 2 +- storage/EntityList.cpp | 15 --- storage/EntityList.h | 73 ---------- storage/GraphPath.cpp | 29 ++++ storage/GraphPath.h | 40 +++++- storage/columns/AllowedKinds.h | 12 +- storage/columns/ColumnDispatcher.h | 10 +- storage/columns/InternalKind.h | 4 +- storage/list/PathTrie.cpp | 22 +-- storage/list/PathTrie.h | 12 +- test/net/TuringProtoRoundTripTest.cpp | 56 ++++++++ .../query-test-suite/QueryResultFormatter.cpp | 46 ++----- test/query/ir/StringRowSink.cpp | 52 ++----- test/query/ir/engine/EquivalenceTest.cpp | 42 ++---- test/query/ir/path/CypherShortestPathTest.cpp | 17 ++- .../ir/path/ShortestPathElementsTest.cpp | 6 +- .../ShortestPathEndpointPredicateTest.cpp | 20 ++- .../ir/path/ShortestPathLoweringTest.cpp | 20 +-- tools/turingdb/TuringShell.cpp | 40 +----- wasm/CMakeLists.txt | 2 +- wasm/DecoderModule.cpp | 32 +++-- wasm/TuringClient.mjs | 17 ++- wasm/WasmSink.h | 6 +- wasm/client_smoke.mjs | 25 ++-- 50 files changed, 545 insertions(+), 781 deletions(-) delete mode 100644 storage/EntityList.cpp delete mode 100644 storage/EntityList.h create mode 100644 storage/GraphPath.cpp diff --git a/docs/ShortestPath/Guide.md b/docs/ShortestPath/Guide.md index d9ca17c310..825fdcfe0f 100644 --- a/docs/ShortestPath/Guide.md +++ b/docs/ShortestPath/Guide.md @@ -15,11 +15,11 @@ We can provide the source and target sets through patterns that come before the ``` turing:dijkstraMinimalExample> match (n{name:"Ashchurch"}), (m{name:"Worcestershire Parkway"}) shortestPath(n,m,distance,dist,path) return dist,path -+-------+--------------+ -| dist | path | -+-------+--------------+ -| 11.29 | (0)-[1]->(6) | -+-------+--------------+ ++-------+---------------+ +| dist | path | ++-------+---------------+ +| 11.29 | (0), [1], (6) | ++-------+---------------+ ``` note that we cannot access the variables of any branch being used by the shortest path processor: @@ -34,11 +34,11 @@ we can access branches that aren't being used by the shortest path processor (th ``` turing:dijkstraMinimalExample> match (n{name:"Ashchurch"}), (m{name:"Worcestershire Parkway"}), (p{name:"Cheltenham Spa"}) shortestPath(n,m,distance,dist,path) return dist,path,p.name -+-------+--------------+----------------+ -| dist | path | p.name | -+-------+--------------+----------------+ -| 11.29 | (0)-[1]->(6) | Cheltenham Spa | -+-------+--------------+----------------+ ++-------+---------------+----------------+ +| dist | path | p.name | ++-------+---------------+----------------+ +| 11.29 | (0), [1], (6) | Cheltenham Spa | ++-------+---------------+----------------+ ``` @@ -55,14 +55,14 @@ turing:dijkstraMinimalExample> match (n{name:"Ashchurch"}), (m{name:"Worcester +-------+ ``` -The path variable is a string representing the shortest path (in the format ...(nodeID)-\[edgeID\]->(nodeID)...) +The path variable is the shortest path from source to target: its nodes and edges in order, a node as (nodeID) and an edge as \[edgeID\] ``` turing:dijkstraMinimalExample> match (n{name:"Ashchurch"}), (m{name:"Worcestershire Parkway"}) shortestPath(n,m,distance,dist,path) return path -+--------------+ -| path | -+--------------+ -| (0)-[1]->(6) | -+--------------+ ++---------------+ +| path | ++---------------+ +| (0), [1], (6) | ++---------------+ ``` As you can see the path does match the ids @@ -104,11 +104,11 @@ As you can see below we can pass multiple sources to the shortest path processor ``` turing:dijkstraMinimalExample> match (n), (m{name:"Worcestershire Parkway"}) where n.name="Bromsgrove" or n.name = "Cheltenham Spa" shortestPath(n,m,distance,dist,path) return dist,path -+------+--------------+ -| dist | path | -+------+--------------+ -| 12.6 | (1)-[5]->(6) | -+------+--------------+ ++------+---------------+ +| dist | path | ++------+---------------+ +| 12.6 | (1), [5], (6) | ++------+---------------+ ``` ### Cypher Sample the following cypher sample was used in the above examples diff --git a/docs/ShortestPath/shortest_path.mdx b/docs/ShortestPath/shortest_path.mdx index 497232531f..fa0eb1ec81 100644 --- a/docs/ShortestPath/shortest_path.mdx +++ b/docs/ShortestPath/shortest_path.mdx @@ -71,11 +71,11 @@ shortestPath(sourceSet, targetSet, edgePropName, distanceReturnVar, pathReturnVa ``` -+-------+--------------+ -| dist | path | -+-------+--------------+ -| 11.29 | (0)-[1]->(6) | -+-------+--------------+ ++-------+---------------+ +| dist | path | ++-------+---------------+ +| 11.29 | (0), [1], (6) | ++-------+---------------+ ``` ### Returning only the distance or only the path @@ -137,11 +137,11 @@ You can pass multiple nodes to the source or target set — `shortestPath` will ``` -+------+--------------+ -| dist | path | -+------+--------------+ -| 12.6 | (1)-[5]->(6) | -+------+--------------+ ++------+---------------+ +| dist | path | ++------+---------------+ +| 12.6 | (1), [5], (6) | ++------+---------------+ ``` ## Limitations @@ -170,10 +170,10 @@ You can pass multiple nodes to the source or target set — `shortestPath` will ``` ``` - +-------+--------------+----------------+ - | dist | path | p.name | - +-------+--------------+----------------+ - | 11.29 | (0)-[1]->(6) | Cheltenham Spa | - +-------+--------------+----------------+ + +-------+---------------+----------------+ + | dist | path | p.name | + +-------+---------------+----------------+ + | 11.29 | (0), [1], (6) | Cheltenham Spa | + +-------+---------------+----------------+ ``` diff --git a/memory/LocalMemory.h b/memory/LocalMemory.h index 320c3c857f..70913b0481 100644 --- a/memory/LocalMemory.h +++ b/memory/LocalMemory.h @@ -50,8 +50,7 @@ class LocalMemory { MakeMemoryPool>::type, MakeMemoryPool>::type, MakeMemoryPool>::type, - MakeMemoryPool>::type, - MakeMemoryPool>::type, + MakeMemoryPool>::type, MakeMemoryPool>::type, MakeMemoryPool>::type, MakeMemoryPool>::type, diff --git a/net/decoders/DecodeContext.h b/net/decoders/DecodeContext.h index 5f543c9790..ed31d2cdb6 100644 --- a/net/decoders/DecodeContext.h +++ b/net/decoders/DecodeContext.h @@ -1,7 +1,6 @@ #pragma once #include -#include namespace net::proto { @@ -32,9 +31,9 @@ struct DecodeContext { // Pointer to the TuringProto receive buffer TuringProtoInBuf* _inBuf {nullptr}; InterruptedBufferState _bufferState; - // Entry count of the EntityList row being decoded. The count and actual data can be - // split across chunks so we need to store state here. - std::optional _entityListEntryCount; + // Set once the nodes of the path row being decoded have arrived, so a resumed pass + // goes on to its edges rather than reading a node count again. + bool _pathNodesRead {false}; // Set once a constant list column's header has been read and its space reserved, so a // resumed pass drains elements rather than reading the header again. An optional // constant spends _rowIndex on its has-value flag, so the two cannot share it. @@ -44,7 +43,7 @@ struct DecodeContext { _columnIndex = 0; _rowIndex = 0; _bufferState.reset(); - _entityListEntryCount.reset(); + _pathNodesRead = false; _constListStarted = false; } }; diff --git a/net/decoders/Decoders.h b/net/decoders/Decoders.h index 709b3f160b..8a14be8e34 100644 --- a/net/decoders/Decoders.h +++ b/net/decoders/Decoders.h @@ -236,34 +236,50 @@ struct VectorColumnDecoder, Sink> { } }; +// A path is [nodeCount][nodes][edges], with one edge fewer than nodes, or none for the +// null path. template -struct VectorColumnDecoder { - using T = db::Path; +struct VectorColumnDecoder { + using T = db::GraphPath; static bool decode(DecodeContext* context, Sink* sink, SinkColumnVector* typedColumn, ProtoColumnState* columnState) { if (context->_rowIndex == 0) { - typedColumn->reserve(columnState->getNumRows()); + typedColumn->resize(columnState->getNumRows()); } for (size_t i = 0; context->_rowIndex + i < columnState->getNumRows(); ++i) { - if (context->_inBuf->readable() < sizeof(WireSize)) { - context->_rowIndex += i; - return false; + db::GraphPath& path = (*typedColumn)[context->_rowIndex + i]; + + if (!context->_pathNodesRead) { + if (context->_inBuf->readable() < sizeof(WireSize)) { + context->_rowIndex += i; + return false; + } + + WireSize nodeCount = 0; + context->_inBuf->readData(&nodeCount, sizeof(nodeCount)); + path.resize(nodeCount, nodeCount == 0 ? 0 : nodeCount - 1); + + const std::span nodes = path.getNodes(); + if (!readVarLenPayload(context, reinterpret_cast(nodes.data()), nodes.size_bytes())) { + context->_rowIndex += i; + context->_pathNodesRead = true; + return false; + } } - WireSize pathByteSize = 0; - context->_inBuf->readData(&pathByteSize, sizeof(pathByteSize)); - const size_t numEntities = checkedElementCount(pathByteSize, "path"); + context->_pathNodesRead = false; - auto& path = typedColumn->emplace_back(numEntities); - if (!readVarLenPayload(context, reinterpret_cast(path.data()), pathByteSize)) { + const std::span edges = path.getEdges(); + if (!readVarLenPayload(context, reinterpret_cast(edges.data()), edges.size_bytes())) { context->_rowIndex += i + 1; return false; } } + return true; } }; @@ -303,59 +319,6 @@ struct VectorColumnDecoder { } }; -template -struct VectorColumnDecoder { - using T = db::EntityList; - - static bool decode(DecodeContext* context, - Sink* sink, - SinkColumnVector* typedColumn, - ProtoColumnState* columnState) { - if (context->_rowIndex == 0) { - typedColumn->resize(columnState->getNumRows()); - } - - for (size_t i = 0; context->_rowIndex + i < columnState->getNumRows(); ++i) { - auto& entityList = (*typedColumn)[context->_rowIndex + i]; - - if (!context->_entityListEntryCount.has_value()) { - if (context->_inBuf->readable() < sizeof(WireSize)) { - context->_rowIndex += i; - return false; - } - - WireSize numberOfEntries = 0; - context->_inBuf->readData(&numberOfEntries, sizeof(numberOfEntries)); - context->_entityListEntryCount = numberOfEntries; - entityList.reserve(numberOfEntries); - } - - const size_t expectedEntries = *context->_entityListEntryCount; - - while (entityList.size() < expectedEntries) { - if (context->_inBuf->readable() < sizeof(db::EntityList::Entry::_type)) { - context->_rowIndex += i; - return false; - } - - entityList.add(); - - context->_inBuf->readData(&entityList.back()._type, sizeof(db::EntityList::Entry::_type)); - - if (context->_inBuf->readable() < sizeof(db::EntityList::Entry::_id)) { - throw TuringException("Entity List Entry can't be broken up across packet/buffer boundaries"); - } - - context->_inBuf->readData(&entityList.back()._id, sizeof(db::EntityList::Entry::_id)); - } - - context->_entityListEntryCount.reset(); - } - - return true; - } -}; - template struct OptionalVectorColumnDecoder { static bool decode(DecodeContext* context, @@ -635,35 +598,6 @@ struct ConstColumnDecoder { } }; -template -struct ConstColumnDecoder { - using T = db::Path; - - template Column> - static bool decode(DecodeContext* context, Sink* sink, Column* typedColumn) { - if (context->_inBuf->readable() < sizeof(WireSize)) { - return false; - } - - WireSize pathByteSize = 0; - context->_inBuf->readData(&pathByteSize, sizeof(pathByteSize)); - const size_t numEntities = checkedElementCount(pathByteSize, "path"); - - db::Path path(numEntities); - char* pathBytes = reinterpret_cast(path.data()); - - // db::Path is a std::vector, so moving it into the column keeps pathBytes valid. - typedColumn->set(std::move(path)); - if (!readVarLenPayload(context, pathBytes, pathByteSize)) { - // See the string branch: completing the resume completes the column. - ++context->_columnIndex; - context->_rowIndex = 0; - return false; - } - return true; - } -}; - template struct ConstColumnDecoder { using T = db::types::Embedding::Primitive; diff --git a/net/decoders/TuringProtoDecoder.h b/net/decoders/TuringProtoDecoder.h index 074b0c1b9e..c0022f3a6e 100644 --- a/net/decoders/TuringProtoDecoder.h +++ b/net/decoders/TuringProtoDecoder.h @@ -88,14 +88,10 @@ decltype(auto) TuringProtoDecoder::dispatchColumnType(ColumnInternalKind t return fn.template operator()(encoding); case ColumnInternalKind::BOOL: return fn.template operator()(encoding); - case ColumnInternalKind::PATH: - return fn.template operator()(encoding); case ColumnInternalKind::EMBEDDING: return fn.template operator()(encoding); case ColumnInternalKind::VALUE_TYPE: return fn.template operator()(encoding); - case ColumnInternalKind::ENTITY_LIST: - return fn.template operator()(encoding); case ColumnInternalKind::LIST_VIEW: return fn.template operator()>(encoding); case net::proto::ColumnInternalKind::MAP_VIEW: @@ -126,6 +122,11 @@ decltype(auto) TuringProtoDecoder::dispatchColumnType(ColumnInternalKind t return fn.template operator()(encoding); case ColumnInternalKind::DURATION: return fn.template operator()(encoding); + case ColumnInternalKind::GRAPH_PATH: + return fn.template operator()(encoding); + case ColumnInternalKind::RETIRED_PATH: + case ColumnInternalKind::RETIRED_ENTITY_LIST: + break; } throw TuringException("Unsupported incoming column type"); diff --git a/net/decoders/TuringProtoDecoderConcepts.h b/net/decoders/TuringProtoDecoderConcepts.h index fb653e34b4..85b9b5c729 100644 --- a/net/decoders/TuringProtoDecoderConcepts.h +++ b/net/decoders/TuringProtoDecoderConcepts.h @@ -2,7 +2,6 @@ #include -#include "EntityList.h" #include "GraphPath.h" #include "metadata/PropertyNull.h" #include "metadata/PropertyType.h" @@ -42,8 +41,7 @@ concept SupportedColumnVectorTypes = std::is_same_v || std::is_same_v || std::is_same_v -|| std::is_same_v -|| std::is_same_v +|| std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v diff --git a/net/encoders/JsonEncoder.h b/net/encoders/JsonEncoder.h index c05e2c1419..b08a60295b 100644 --- a/net/encoders/JsonEncoder.h +++ b/net/encoders/JsonEncoder.h @@ -155,21 +155,6 @@ class ChunkJsonEncoder { _writer.write(value ? "true" : "false"); } - void encodeValue(const Path& value) { - _writer.write("["); - if(!value.empty()) { - auto it = value.rbegin(); - _writer.write(it->getValue()); - ++it; - while(it != value.rend()) { - _writer.write(","); - _writer.write(it->getValue()); - ++it; - } - } - _writer.write("]"); - } - void encodeValue(ValueType value) { JsonStringEscaper::escapeAndSurroundByQuotes(ValueTypeName::value(value), _sanitized); _writer.write(_sanitized); @@ -221,31 +206,34 @@ class ChunkJsonEncoder { _writer.write(_sanitized); } - void encodeValue(const EntityList& value) { - if (value.empty()) { + void encodeValue(const GraphPath& value) { + if (value.isNull()) { _writer.write("null"); return; } - const auto& entries = value.getEntries(); - const auto& firstValue = entries.front(); + const std::span nodes = value.getNodes(); + const std::span edges = value.getEdges(); - _writer.write("[{\"type\":\""); - _writer.write(firstValue._type == EntityType::Node ? "node" : "edge"); - _writer.write("\",\"id\":"); - _writer.write(std::to_string(firstValue._id.getValue())); - _writer.write('}'); + _writer.write('['); + encodePathEntity("node", nodes.front().getValue()); - for (const auto& [type, id] : entries | std::views::drop(1)) { + for (size_t hop = 0; hop < edges.size(); hop++) { + _writer.write(','); + encodePathEntity("edge", edges[hop].getValue()); _writer.write(','); - _writer.write('{'); - _writer.write("\"type\":\""); - _writer.write(type == EntityType::Node ? "node" : "edge"); - _writer.write("\",\"id\":"); - _writer.write(std::to_string(id.getValue())); - _writer.write('}'); + encodePathEntity("node", nodes[hop + 1].getValue()); } - _writer.write("]"); + + _writer.write(']'); + } + + void encodePathEntity(const char* type, uint64_t id) { + _writer.write("{\"type\":\""); + _writer.write(type); + _writer.write("\",\"id\":"); + _writer.write(id); + _writer.write('}'); } void encodeValue(const ListElementView ele) { diff --git a/net/encoders/OutputValues.h b/net/encoders/OutputValues.h index 11f0f9fa94..7bfbdc7672 100644 --- a/net/encoders/OutputValues.h +++ b/net/encoders/OutputValues.h @@ -19,7 +19,6 @@ namespace db { class ColumnMask; -class EntityList; template struct IsHash : std::false_type {}; @@ -63,10 +62,7 @@ concept IsBool = std::is_same_v || OptionalBool; template -concept IsPath = std::is_same_v; - -template -concept IsEntityList = std::is_same_v; +concept IsPath = std::is_same_v; template concept IsListView = std::is_same_v, ListView>; @@ -119,8 +115,6 @@ struct ColumnTypeGenerator { _name = fmt::format("Path"); } else if constexpr (IsNull) { _name = fmt::format("NULL"); - } else if constexpr (IsEntityList) { - _name = fmt::format("EntityList"); } else if constexpr (IsListView) { _name = fmt::format("List"); } else if constexpr (IsListElement) { diff --git a/net/encoders/TuringProtoEncoder.h b/net/encoders/TuringProtoEncoder.h index cdb804364e..5c43e56460 100644 --- a/net/encoders/TuringProtoEncoder.h +++ b/net/encoders/TuringProtoEncoder.h @@ -53,15 +53,13 @@ struct ColInternalKindToProtoEnum { } else if constexpr (db::IsBool) { return Enum::BOOL; } else if constexpr (db::IsPath) { - return Enum::PATH; + return Enum::GRAPH_PATH; } else if constexpr (db::IsEmbedding) { return Enum::EMBEDDING; } else if constexpr (db::IsDateTime) { return Enum::DATE_TIME; } else if constexpr (db::IsDuration) { return Enum::DURATION; - } else if constexpr (db::IsEntityList) { - return Enum::ENTITY_LIST; } else if constexpr (db::IsListView) { return Enum::LIST_VIEW; } else if constexpr (db::IsMap) { @@ -171,11 +169,8 @@ class DataWriter { _outBuf->copyVarLenData(val.data(), columnByteSize); } } else if constexpr (db::IsPath) { - for (const auto& val : values) { - bioassert(val.size() * sizeof(db::EntityID) <= MAX_WIRE_SIZE, "Path length exceeds maximum wire size"); - const WireSize columnByteSize = static_cast(val.size() * sizeof(db::EntityID)); - _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); - _outBuf->copyVarLenData(val.data(), columnByteSize); + for (const db::GraphPath& path : values) { + writePath(path); } } else if constexpr (db::IsEmbedding) { for (const auto& val : values) { @@ -184,21 +179,6 @@ class DataWriter { _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); _outBuf->copyVarLenData(val.data(), columnByteSize); } - } else if constexpr (db::IsEntityList) { - constexpr size_t sizeOfEntry = sizeof(db::EntityList::Entry::_id) + sizeof(db::EntityList::Entry::_type); - - for (const auto& entityList : values) { - bioassert(entityList.size() <= MAX_WIRE_SIZE, "Entity list length exceeds maximum wire size"); - const WireSize columnByteSize = static_cast(entityList.size()); - _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); - for (const auto& entry : entityList) { - // Ensure that we can copy a full entry into the packet - _outBuf->checkRemainingAndFlush(sizeOfEntry); - - _outBuf->copyFixedLenData(&entry._type, sizeof(entry._type)); - _outBuf->copyFixedLenData(&entry._id, sizeof(entry._id)); - } - } } else if constexpr (db::IsListView) { for (const auto& listView : values) { _nestedWriter.writeListView(listView); @@ -244,19 +224,6 @@ class DataWriter { _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); _outBuf->copyVarLenData(val->data(), columnByteSize); } - } else if constexpr (db::IsPath) { - for (const auto& val : values) { - if (!val.has_value()) { - const WireSize columnByteSize = 0; - _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); - continue; - } - - bioassert(val->size() * sizeof(db::EntityID) <= MAX_WIRE_SIZE, "Optional path length exceeds maximum wire size"); - const WireSize columnByteSize = static_cast(val->size() * sizeof(db::EntityID)); - _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); - _outBuf->copyVarLenData(val->data(), columnByteSize); - } } else if constexpr (db::IsEmbedding) { for (const auto& val : values) { if (!val.has_value()) { @@ -280,8 +247,6 @@ class DataWriter { for (const auto& val : values) { _nestedWriter.writeMapView(val.has_value() ? *val : db::MapView {}); } - } else if constexpr (db::IsEntityList) { - static_assert(sizeof(T) == 0, "Sending ColumnOptVector not supported"); } else if constexpr (db::IsListView) { for (const auto& val : *col) { _nestedWriter.writeListView(val.has_value() ? *val : db::ListView {}); @@ -314,21 +279,13 @@ class DataWriter { _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); _outBuf->copyVarLenData(val.data(), columnByteSize); } else if constexpr (db::IsPath) { - const auto& val = col->at(0); - bioassert(val.size() * sizeof(db::EntityID) <= MAX_WIRE_SIZE, "Const path length exceeds maximum wire size"); - const WireSize columnByteSize = static_cast(val.size() * sizeof(db::EntityID)); - _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); - _outBuf->copyVarLenData(val.data(), columnByteSize); + writePath(col->at(0)); } else if constexpr (db::IsEmbedding) { const auto& val = col->at(0); bioassert(val.size() * sizeof(float) <= MAX_WIRE_SIZE, "Const embedding length exceeds maximum wire size"); const WireSize columnByteSize = static_cast(val.size() * sizeof(float)); _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); _outBuf->copyVarLenData(val.data(), columnByteSize); - } else if constexpr (db::IsEntityList) { - // It is not used/returned anywhere in the codebase so we disable support for - // it now. We can renable it if ever needed. - throw FatalException("ColumnConst is not supported"); } else if constexpr (db::IsListView) { _nestedWriter.writeListView(col->at(0)); } else if constexpr (db::IsMap) { @@ -364,12 +321,6 @@ class DataWriter { const WireSize columnByteSize = static_cast(val.size() * sizeof(char)); _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); _outBuf->copyVarLenData(val.data(), columnByteSize); - } else if constexpr (db::IsPath) { - const auto& val = *opt; - bioassert(val.size() * sizeof(db::EntityID) <= MAX_WIRE_SIZE, "Optional const path length exceeds maximum wire size"); - const WireSize columnByteSize = static_cast(val.size() * sizeof(db::EntityID)); - _outBuf->copyFixedLenData(&columnByteSize, sizeof(columnByteSize)); - _outBuf->copyVarLenData(val.data(), columnByteSize); } else if constexpr (db::IsEmbedding) { const auto& val = *opt; bioassert(val.size() * sizeof(float) <= MAX_WIRE_SIZE, "Optional const embedding length exceeds maximum wire size"); @@ -380,10 +331,6 @@ class DataWriter { _nestedWriter.writeListView(*opt); } else if constexpr (db::IsMap) { _nestedWriter.writeMapView(*opt); - } else if constexpr (db::IsEntityList) { - // EntityList is only used as ColumnVector, never optional. - // Dependent condition (see the ColumnOptVector branch above). - static_assert(sizeof(T) == 0, "ColumnOptConst is not supported"); } else if constexpr (db::IsListView) { _nestedWriter.writeListView(*opt); } else if constexpr (db::IsListElement) { @@ -420,6 +367,17 @@ class DataWriter { return std::span(data + _offset, std::min(_rowCount, available)); } + void writePath(const db::GraphPath& path) { + const std::span nodes = path.getNodes(); + const std::span edges = path.getEdges(); + bioassert(nodes.size_bytes() <= MAX_WIRE_SIZE, "Path length exceeds maximum wire size"); + + const WireSize nodeCount = static_cast(nodes.size()); + _outBuf->copyFixedLenData(&nodeCount, sizeof(nodeCount)); + _outBuf->copyVarLenData(nodes.data(), nodes.size_bytes()); + _outBuf->copyVarLenData(edges.data(), edges.size_bytes()); + } + net::proto::TuringProtoOutBuf* _outBuf {nullptr}; NestedContainerWriter& _nestedWriter; size_t _offset {0}; diff --git a/net/turing_proto_common/TuringProtoHeaders.h b/net/turing_proto_common/TuringProtoHeaders.h index 8eae8fea45..02d6df7c06 100644 --- a/net/turing_proto_common/TuringProtoHeaders.h +++ b/net/turing_proto_common/TuringProtoHeaders.h @@ -59,9 +59,9 @@ enum class ColumnInternalKind : uint32_t { DOUBLE, STRING, BOOL, - PATH, + RETIRED_PATH, EMBEDDING, - ENTITY_LIST, + RETIRED_ENTITY_LIST, LIST_VIEW, VALUE_TYPE, LIST_ELEMENT_VIEW, @@ -78,6 +78,7 @@ enum class ColumnInternalKind : uint32_t { MAP_ENTRY_VIEW, DATE_TIME, DURATION, + GRAPH_PATH, }; enum class ColumnKind : uint8_t { diff --git a/python/turingdb/_common/NanobindUtils.cpp b/python/turingdb/_common/NanobindUtils.cpp index 7518377918..b419c17118 100644 --- a/python/turingdb/_common/NanobindUtils.cpp +++ b/python/turingdb/_common/NanobindUtils.cpp @@ -7,8 +7,7 @@ #include #include -#include "EntityList.h" -#include "EntityType.h" +#include "GraphPath.h" #include "LocalMemory.h" #include "dataframe/Dataframe.h" #include "dataframe/DataframeManager.h" @@ -170,6 +169,13 @@ nb::object embeddingToNdarray(std::span s) { namespace { +nb::dict pathEntityDict(const char* type, uint64_t id) { + nb::dict entity; + entity["type"] = nb::cast(type); + entity["id"] = nb::cast(id); + return entity; +} + // Converts a list or map value (or a single list element / map entry) to plain Python // objects — scalars, a list of floats for an embedding, nested lists, and dicts for maps. // Deliberately plain objects, not ndarrays, so the result compares equal to the JSON-parsed @@ -407,26 +413,28 @@ nb::dict dataframeToNumpy(db::Dataframe* df) { dtypeName = "String"; break; } - case db::ColumnVector::staticKind(): { - const auto& src = static_cast*>(col)->getRaw(); + case db::ColumnVector::staticKind(): { + const auto& src = static_cast*>(col)->getRaw(); nb::list lst; - for (const db::EntityList& entityList : src) { - if (entityList.empty()) { + for (const db::GraphPath& path : src) { + if (path.isNull()) { lst.append(nb::none()); continue; } + const std::span nodes = path.getNodes(); + const std::span edges = path.getEdges(); + nb::list rowList; - for (const db::EntityList::Entry& entry : entityList.getEntries()) { - nb::dict d; - d["type"] = nb::cast(entry._type == db::EntityType::Node ? "node" : "edge"); - d["id"] = nb::cast(entry._id.getValue()); - rowList.append(d); + rowList.append(pathEntityDict("node", nodes.front().getValue())); + for (size_t hop = 0; hop < edges.size(); hop++) { + rowList.append(pathEntityDict("edge", edges[hop].getValue())); + rowList.append(pathEntityDict("node", nodes[hop + 1].getValue())); } lst.append(rowList); } value = lst; - dtypeName = "EntityList"; + dtypeName = "Path"; break; } case db::ColumnVector::staticKind(): { diff --git a/query/ir/codegen/DBProgramGenerator.cpp b/query/ir/codegen/DBProgramGenerator.cpp index c319bcc1a9..a67a904405 100644 --- a/query/ir/codegen/DBProgramGenerator.cpp +++ b/query/ir/codegen/DBProgramGenerator.cpp @@ -1849,8 +1849,8 @@ mlir::Value DBProgramGenerator::pathLengthColumn(const Expr* argExpr, mlir::Valu mlir::Value DBProgramGenerator::pathElementsColumn(mlir::Value column, mlir::storage::PathElementsKind kind) { const auto columnType = mlir::dyn_cast(column.getType()); - const bool readsAPath = columnType && mlir::isa(columnType.getType()); - bioassert(readsAPath, "A named path is read off the entity sequence its element built or the path a SHORTESTPATH found"); + const bool readsAPath = columnType && mlir::isa(columnType.getType()); + bioassert(readsAPath, "A named path is read off the path its element built or the path a SHORTESTPATH found"); const mlir::Type resultType = pathElementsType(_mlirCtxt, kind); const mlir::db::ColumnType nullableType = @@ -3678,7 +3678,7 @@ void DBProgramGenerator::generateNamedPath(const PatternElement* element, const } mlir::Value DBProgramGenerator::makePathColumn(llvm::ArrayRef entities, llvm::ArrayRef reversedPaths) { - const mlir::db::ColumnType pathType = allocColumnType(mlir::storage::EntityListType::get(_mlirCtxt)); + const mlir::db::ColumnType pathType = allocColumnType(mlir::storage::PathType::get(_mlirCtxt)); const mlir::ArrayAttr reversedAttr = reversedPaths.empty() ? mlir::ArrayAttr() : _opBuilder.getI64ArrayAttr(reversedPaths); return _opBuilder.create(_opBuilder.getUnknownLoc(), pathType, entities, reversedAttr).getResult(); @@ -3717,7 +3717,7 @@ mlir::Value DBProgramGenerator::namedPathColumn(const PartScope::NamedPathWalk& entities = {seedColumn, pathColumn}; } - const mlir::db::ColumnType pathType = allocColumnType(mlir::storage::EntityListType::get(_mlirCtxt)); + const mlir::db::ColumnType pathType = allocColumnType(mlir::storage::PathType::get(_mlirCtxt)); return _opBuilder.create(_opBuilder.getUnknownLoc(), pathType, entities, reversedPaths).getResult(); } diff --git a/query/ir/codegen/DBProgramGenerator.h b/query/ir/codegen/DBProgramGenerator.h index 99188f7db8..38db0e047b 100644 --- a/query/ir/codegen/DBProgramGenerator.h +++ b/query/ir/codegen/DBProgramGenerator.h @@ -285,7 +285,7 @@ class DBProgramGenerator { std::unordered_map _namedPathWalks; - // The entity sequence each `MATCH p = ...` built for an element that names no + // The path each `MATCH p = ...` built for an element that names no // single walk. A path variable appears in no pattern, so it is no VDG variable and // cannot live in _varMap. std::unordered_map _namedPaths; @@ -815,7 +815,7 @@ class DBProgramGenerator { void generateMatchFilter(const MatchStmt* matchStmt); // Binds every element a `MATCH p = ...` named: to the walk when the element is one - // quantified relationship, and otherwise to the entity sequence built here out of the + // quantified relationship, and otherwise to the path built here out of the // columns its entities are bound to void generateNamedPaths(const MatchStmt* matchStmt); void generateNamedPath(const PatternElement* element, const VarDecl* pathDecl); diff --git a/query/ir/dialect/db/DBOps.cpp b/query/ir/dialect/db/DBOps.cpp index 35ad0f5031..59188cc2ba 100644 --- a/query/ir/dialect/db/DBOps.cpp +++ b/query/ir/dialect/db/DBOps.cpp @@ -2215,7 +2215,7 @@ LogicalResult OptionalMatch::verify() { const bool isPaddable = llvm::isa_and_present(elementType); + storage::PathType>(elementType); if (!isPaddable) { return emitOpError("pattern variable column ") diff --git a/query/ir/dialect/db/DBOps.td b/query/ir/dialect/db/DBOps.td index cc6303beb5..6e63563de0 100644 --- a/query/ir/dialect/db/DBOps.td +++ b/query/ir/dialect/db/DBOps.td @@ -1633,44 +1633,44 @@ def PathLength : TuringOp<"path_length", [Pure]> { * MakePath */ def MakePath : TuringOp<"make_path", [Pure]> { - let summary = "Build the path a named pattern element binds, one entity sequence per row"; + let summary = "Build the path a named pattern element binds, one path per row"; let description = [{ `MATCH p = (a)-[x]->(b)` binds `p` to the path the element spells out. This op lays - that path out as one sequence of graph entities per row - the nodes and the edges it - runs through, in the order they are written, each entry saying which of the two it is. + that path out as one path per row - the nodes and the edges it runs through, in the + order they are written. %p = db.make_path(%a, %x, %b) : (!db.column, !db.column, !db.column) - -> !db.column + -> !db.column The operands are the element's entities in written order. A node column contributes - one node entry, an edge column one edge entry, and a path column - what a quantified - relationship binds - the whole walk it stands for: one edge entry and one node entry - per hop, its seed left out because the node operand ahead of it already holds it. So + one node, an edge column one edge, and a path column - what a quantified relationship + binds - the whole walk it stands for: one edge and one node per hop, its seed left + out because the node operand ahead of it already holds it. So `MATCH p = (a)-[e]->+(b)` is `db.make_path(%a, %pe)`, with no operand for `b`: the last node the walk reaches is `b`. - %p = db.make_path(%a, %pe) : (...) -> !db.column - %p = db.make_path(%a, %x, %b, %pe) : (...) -> !db.column + %p = db.make_path(%a, %pe) : (...) -> !db.column + %p = db.make_path(%a, %x, %b, %pe) : (...) -> !db.column - A path of no hops contributes no entry, so `MATCH p = (a)-[e]->*(m)` reads back as + A path of no hops contributes nothing, so `MATCH p = (a)-[e]->*(m)` reads back as the single node `a` on the rows where the walk stayed put. `reversed_paths` lists the path operands the exploration walked against the pattern, because the pattern's far end was bound first and seeding there was cheaper. Such an operand contributes its hops from the far end back - the node an edge lands on, then - that edge - so the sequence still reads the way the pattern is written. The node the + that edge - so the path still reads the way the pattern is written. The node the walk seeded from is written after the path rather than before it. Written after a node, the path writes over that node, which is the one the walk ended at. - %p = db.make_path(%pe, %b) {reversed_paths = [0]} : (...) -> !db.column - %p = db.make_path(%a, %x, %k, %pe, %b) {reversed_paths = [3]} : (...) -> !db.column + %p = db.make_path(%pe, %b) {reversed_paths = [0]} : (...) -> !db.column + %p = db.make_path(%a, %x, %k, %pe, %b) {reversed_paths = [3]} : (...) -> !db.column }]; let arguments = (ins Variadic:$entities, OptionalAttr:$reversed_paths); - let results = (outs ColumnEntityList:$result); + let results = (outs ColumnPath:$result); let assemblyFormat = [{ `(` $entities `)` attr-dict `:` functional-type(operands, results) @@ -1688,21 +1688,21 @@ def PathElements : TuringOp<"path_elements", [Pure]> { let description = [{ `nodes(p)`, `relationships(p)` and `length(p)` over the path a named pattern element binds: one list of node IDs, one list of edge IDs or one hop count per row, - read off the entity sequence db.make_path laid out, or off the path db.shortest_path + read off the path db.make_path laid out, or off the path db.shortest_path found. %ns = db.path_elements(%p) kind nodes - : (!db.column) -> !db.column>> + : (!db.column) -> !db.column>> %n = db.path_elements(%p) kind length - : (!db.column) -> !db.column> + : (!db.column) -> !db.column> - A path an OPTIONAL MATCH missed is an empty sequence, and each function of it is + A path an OPTIONAL MATCH missed is null, and each function of it is null, so the results are nullable. Over the path of one node and one walk, fuse_path_elements answers from the walk's handles instead: db.path_length for the length and db.expand_path for the two lists. }]; - let arguments = (ins AnyTypeOf<[ColumnEntityList, ColumnPath]>:$path, PathElementsKind:$kind); + let arguments = (ins ColumnPath:$path, PathElementsKind:$kind); let results = (outs Column:$result); let assemblyFormat = [{ diff --git a/query/ir/dialect/db/DBTypes.td b/query/ir/dialect/db/DBTypes.td index ac8141f52c..db1c3876b6 100644 --- a/query/ir/dialect/db/DBTypes.td +++ b/query/ir/dialect/db/DBTypes.td @@ -42,7 +42,6 @@ def ColumnEdgeIDs : ColumnOf<"::mlir::storage::EdgeIDType", "edgeid">; def ColumnEdgeTypes : ColumnOf<"::mlir::storage::EdgeTypeIDType", "edge_type_id">; def ColumnPath : ColumnOf<"::mlir::storage::PathType", "path">; def ColumnPathRefs : ColumnOf<"::mlir::storage::PathRefType", "path_ref">; -def ColumnEntityList : ColumnOf<"::mlir::storage::EntityListType", "entity_list">; def ColumnChangeIDs : ColumnOf<"::mlir::storage::ChangeIDType", "change_id">; diff --git a/query/ir/dialect/nl/NLOps.td b/query/ir/dialect/nl/NLOps.td index 7dbb89183b..e4279936ab 100644 --- a/query/ir/dialect/nl/NLOps.td +++ b/query/ir/dialect/nl/NLOps.td @@ -1159,15 +1159,15 @@ def PathLength : NLOp<"path_length", [Pure, RowAlignedThroughOperands]> { * MakePath */ def MakePath : NLOp<"make_path", [Pure, RowAlignedThroughOperands]> { - let summary = "Build the path of a named pattern element, one entity sequence per row"; + let summary = "Build the path of a named pattern element, one path per row"; let description = [{ - Imperative counterpart of db.make_path: one sequence of graph entities per row, the - nodes and edges the element runs through in written order, a path operand expanded - into the hops it stands for. + Imperative counterpart of db.make_path: one path per row, the nodes and edges the + element runs through in written order, a path operand expanded into the hops it + stands for. %p = nl.make_path(%a, %x, %b) : (!nl.chunk, !nl.chunk, !nl.chunk) - -> !nl.chunk + -> !nl.chunk }]; let arguments = (ins Variadic:$entities, @@ -1186,12 +1186,12 @@ def PathElements : NLOp<"path_elements", [Pure, RowAlignedThroughOperands]> { let summary = "Read the nodes, the relationships or the length of each named path of a chunk"; let description = [{ Imperative counterpart of db.path_elements: one list of node IDs, one list of edge - IDs or one hop count per row, read off the entity sequence nl.make_path laid out - or the path nl.shortest_path found, and null on the rows whose sequence is empty - - the paths an OPTIONAL MATCH missed. + IDs or one hop count per row, read off the path nl.make_path laid out or the path + nl.shortest_path found, and null on the rows whose path is null - the paths an + OPTIONAL MATCH missed. %ns = nl.path_elements(%p) kind nodes - : (!nl.chunk) -> !nl.chunk>> + : (!nl.chunk) -> !nl.chunk>> }]; let arguments = (ins NLChunk:$path, PathElementsKind:$kind); diff --git a/query/ir/dialect/storage/StorageTypes.td b/query/ir/dialect/storage/StorageTypes.td index 8573b0dbee..5a6eb9adb0 100644 --- a/query/ir/dialect/storage/StorageTypes.td +++ b/query/ir/dialect/storage/StorageTypes.td @@ -141,7 +141,11 @@ def ListElement : StorageType<"ListElement", "list_element"> { def Path : StorageType<"Path", "path"> { let summary = "A graph path value"; let description = [{ - Chunk representation of ColumnVector. + The element type of a chunk holding one path per row - storage's + ColumnVector. Each cell holds the nodes and the edges the path runs + through, in order, so a path reads back as nodes and relationships told apart at + output. A path with no node is null. Produced by db.make_path and + db.shortest_path. }]; } @@ -157,16 +161,6 @@ def PathRef : StorageType<"PathRef", "path_ref"> { }]; } -def EntityList : StorageType<"EntityList", "entity_list"> { - let summary = "A sequence of graph entities"; - let description = [{ - The element type of a chunk holding one entity sequence per row - storage's - ColumnVector. Each entry carries whether it is a node or an edge - beside its ID, so a path reads back as the nodes and the relationships it runs - through, told apart at output. Produced by db.make_path. - }]; -} - def List : StorageType<"List", "list"> { let summary = "A list value"; let description = [{ diff --git a/query/ir/exec/NLExecutor.cpp b/query/ir/exec/NLExecutor.cpp index fdadc88566..14f00d45cf 100644 --- a/query/ir/exec/NLExecutor.cpp +++ b/query/ir/exec/NLExecutor.cpp @@ -1887,18 +1887,18 @@ void entityToNullableColumn(Column* result, const Column* operand) { } } -// A path an OPTIONAL MATCH did not match is empty, where any other holds at least the node -// it starts on, so the empty path is the null and every other reads as its entry count +// Every path but the null one holds at least the node it starts on, so it reads as its +// node count void pathToNullableColumn(Column* result, const Column* operand) { - const std::vector& paths = static_cast*>(operand)->getRaw(); + const std::vector& paths = static_cast*>(operand)->getRaw(); auto& nullables = static_cast*>(result)->getRaw(); nullables.resize(paths.size()); for (size_t row = 0; row < paths.size(); row++) { - if (paths[row].empty()) { + if (paths[row].isNull()) { nullables[row] = std::nullopt; } else { - nullables[row] = paths[row].size(); + nullables[row] = paths[row].getNodes().size(); } } } @@ -2031,32 +2031,18 @@ std::optional readOptTaggedCellTruth(const Column* cells, size_t row) { return cellTruth(*cell); } -EntityID pathEntityAt(const EntityList& path, size_t index) { - return path.getEntries()[index]._id; -} - -EntityID pathEntityAt(const Path& path, size_t index) { - return path[index]; -} - -// The entities of one kind a path runs through, in the order the path holds them. A path -// alternates node and edge from its first node, so they are every other entry from @param first -template -ListView listPathEntities(const PathType& path, size_t first, ListBufferTypeTag tag, QueryListBuffer& buffer) { - const size_t count = (path.size() - first + 1) / 2; - ListWriteCursor cursor = buffer.reserveList(count, count * sizeof(IDType)); +template +ListView listPathEntities(std::span entities, ListBufferTypeTag tag, QueryListBuffer& buffer) { + ListWriteCursor cursor = buffer.reserveList(entities.size(), entities.size() * sizeof(IDType)); - for (size_t index = 0; index < count; index++) { - cursor.writeValueAt(index, tag, IDType(pathEntityAt(path, first + 2 * index).getValue())); + for (size_t index = 0; index < entities.size(); index++) { + cursor.writeValueAt(index, tag, entities[index]); } return cursor.getView(); } -// A path an OPTIONAL MATCH missed is an empty sequence; every path found runs -// through at least one node -template -void readPathElements(const std::vector& paths, +void readPathElements(const std::vector& paths, PathElementsKind kind, Column* output, QueryListBuffer& listBuffer) { @@ -2065,11 +2051,11 @@ void readPathElements(const std::vector& paths, lengths.resize(paths.size()); for (size_t row = 0; row < paths.size(); row++) { - const PathType& path = paths[row]; - if (path.empty()) { + const GraphPath& path = paths[row]; + if (path.isNull()) { lengths[row] = std::nullopt; } else { - lengths[row] = path.size() / 2; + lengths[row] = path.getLength(); } } @@ -2080,13 +2066,13 @@ void readPathElements(const std::vector& paths, lists.resize(paths.size()); for (size_t row = 0; row < paths.size(); row++) { - const PathType& path = paths[row]; - if (path.empty()) { + const GraphPath& path = paths[row]; + if (path.isNull()) { lists[row] = std::nullopt; } else if (kind == PathElementsKind::Nodes) { - lists[row] = listPathEntities(path, 0, ListBufferTypeTag::NodeID, listBuffer); + lists[row] = listPathEntities(path.getNodes(), ListBufferTypeTag::NodeID, listBuffer); } else { - lists[row] = listPathEntities(path, 1, ListBufferTypeTag::EdgeID, listBuffer); + lists[row] = listPathEntities(path.getEdges(), ListBufferTypeTag::EdgeID, listBuffer); } } } @@ -2764,17 +2750,22 @@ void distinctKeyAppendMapColumn(const Column* column, size_t row, std::string& k distinctAppendMapBytes(key, raw[row]); } -// A null path is an empty one, so it keys as length 0, apart from every path -void distinctKeyAppendEntityListColumn(const Column* column, size_t row, std::string& key) { - const std::vector& raw = static_cast*>(column)->getRaw(); - const EntityList& path = raw[row]; +// The null path has no node, so it keys as node count 0, apart from every path. The node +// count also fixes the edge count, so the IDs that follow it need no separator. +void distinctKeyAppendPathColumn(const Column* column, size_t row, std::string& key) { + const std::vector& raw = static_cast*>(column)->getRaw(); + const GraphPath& path = raw[row]; - const size_t size = path.size(); - key.append(reinterpret_cast(&size), sizeof(size)); + const std::span nodes = path.getNodes(); + const size_t nodeCount = nodes.size(); + key.append(reinterpret_cast(&nodeCount), sizeof(nodeCount)); + + for (const NodeID node : nodes) { + distinctAppendValueBytes(key, node.getValue()); + } - for (const EntityList::Entry& entry : path) { - key.push_back(static_cast(entry._type)); - distinctAppendValueBytes(key, entry._id.getValue()); + for (const EdgeID edge : path.getEdges()) { + distinctAppendValueBytes(key, edge.getValue()); } } @@ -3026,9 +3017,9 @@ size_t countValidIDs(const Column* column) { } size_t countPresentPaths(const Column* column) { - const std::vector& raw = static_cast*>(column)->getRaw(); + const std::vector& raw = static_cast*>(column)->getRaw(); - return std::ranges::count_if(raw, [](const EntityList& path) { return !path.empty(); }); + return std::ranges::count_if(raw, [](const GraphPath& path) { return !path.isNull(); }); } // Count the present (non-null) values of a nullable value column - a @@ -3963,10 +3954,10 @@ void groupFoldCountPresentPath(Column* accumulator, const std::vector& groups, NLGroupDistinctTally& distinct, NLValueStore& values) { - const std::vector& inputRaw = static_cast*>(input)->getRaw(); + const std::vector& inputRaw = static_cast*>(input)->getRaw(); for (size_t row = 0; row < inputRaw.size(); row++) { - if (!inputRaw[row].empty()) { + if (!inputRaw[row].isNull()) { counts[groups[row]]++; } } @@ -5883,31 +5874,34 @@ void shortestPathSearch(NLExecutionContext* context, NLShortestPathLoopData* loo Column* dist = loopData->getDistanceOutput(); auto* distCol = static_cast*>(dist); - ColumnVector* pathCol = loopData->getPathOutput(); + ColumnVector* pathCol = loopData->getPathOutput(); distCol->clear(); pathCol->clear(); // Get the total length of the path distCol->push_back(heap.top().distance); - auto& pathVec = pathCol->emplace_back(); + GraphPath& path = pathCol->emplace_back(); // In the heap all the (non-stale) values represent the shortest path to that // nodeID from the source set. So we can find the shortet path by following the // previousNode back from the target node that we have found. auto lastNode = heap.top().prevNode; auto edge = heap.top().edge; - pathVec.push_back(heap.top().id.getValue()); + path.addNode(heap.top().id); while (lastNode.isValid()) { - pathVec.push_back(edge.getValue()); - pathVec.push_back(lastNode.getValue()); + path.addEdge(edge); + path.addNode(lastNode); const auto& pathInfo = heapValueMap[lastNode]; lastNode = pathInfo.prevNode; edge = pathInfo.edge; } + std::ranges::reverse(path.getNodes()); + std::ranges::reverse(path.getEdges()); + runBody(context, loopData->getStmts()); } @@ -7563,17 +7557,13 @@ void NLExecutor::runPathElements(NLExecutionContext* context, NLFunctionData* da QueryListBuffer& listBuffer = *elements->getListBuffer(); listBuffer.clear(); - if (paths->getKind() == ColumnVector::staticKind()) { - readPathElements(static_cast*>(paths)->getRaw(), kind, output, listBuffer); - } else { - readPathElements(static_cast*>(paths)->getRaw(), kind, output, listBuffer); - } + readPathElements(static_cast*>(paths)->getRaw(), kind, output, listBuffer); } void NLExecutor::runMakePath(NLExecutionContext* context, NLFunctionData* data) { const NLMakePathData* make = static_cast(data); const NLMakePathData::PathEntities& entities = make->entities(); - std::vector& paths = make->getOutput()->getRaw(); + std::vector& paths = make->getOutput()->getRaw(); const PathTrie& trie = *make->getTrie(); bioassert(!entities.empty(), "nl.make_path builds a path out of at least one node"); @@ -7588,7 +7578,7 @@ void NLExecutor::runMakePath(NLExecutionContext* context, NLFunctionData* data) "Entity columns of a path build are not row-aligned."); paths.resize(rowCount); - for (EntityList& path : paths) { + for (GraphPath& path : paths) { path.clear(); } @@ -7597,7 +7587,7 @@ void NLExecutor::runMakePath(NLExecutionContext* context, NLFunctionData* data) case NLPathEntity::Kind::Node: { const std::vector& nodes = static_cast(entity._column)->getRaw(); for (size_t row = 0; row < rowCount; row++) { - paths[row].add(EntityType::Node, EntityID(nodes[row].getValue())); + paths[row].addNode(nodes[row]); } } break; @@ -7605,7 +7595,7 @@ void NLExecutor::runMakePath(NLExecutionContext* context, NLFunctionData* data) case NLPathEntity::Kind::Edge: { const std::vector& edges = static_cast(entity._column)->getRaw(); for (size_t row = 0; row < rowCount; row++) { - paths[row].add(EntityType::Edge, EntityID(edges[row].getValue())); + paths[row].addEdge(edges[row]); } } break; @@ -11371,7 +11361,7 @@ NLKeyAppendFunction NLExecutor::selectKeyAppendFunction(NLChunkKind kind) { break; case NLChunkKind::Path: - throw IRException("A path column cannot be a DISTINCT or grouping key: a path has no scalar value to key on"); + return &distinctKeyAppendPathColumn; break; case NLChunkKind::DateTime: @@ -11385,10 +11375,6 @@ NLKeyAppendFunction NLExecutor::selectKeyAppendFunction(NLChunkKind kind) { case NLChunkKind::PathRef: throw IRException("A path column cannot be a DISTINCT or grouping key: expand it into its list first"); break; - - case NLChunkKind::EntityList: - return &distinctKeyAppendEntityListColumn; - break; } bioassert(false, "Unknown NLChunkKind"); @@ -11558,10 +11544,6 @@ NLJoinKeyFunctions NLExecutor::selectJoinKeyFunctions(NLChunkKind kind) { case NLChunkKind::PathRef: throw IRException("A path column cannot be a join key: expand it into its list first"); break; - - case NLChunkKind::EntityList: - throw IRException("A path column cannot be a join key: a path has no scalar value to key on"); - break; } bioassert(false, "Unknown NLChunkKind"); @@ -12044,10 +12026,6 @@ NLGroupAggregateFoldFunction NLExecutor::selectGroupCountDistinctChunkFold(NLChu case NLChunkKind::PathRef: throw IRException("count(DISTINCT) cannot key on a path column: expand it into its list first"); break; - - case NLChunkKind::EntityList: - throw IRException("count(DISTINCT) cannot key on a path column: a path has no scalar value to count distinct"); - break; } bioassert(false, "Unknown NLChunkKind"); @@ -12237,7 +12215,6 @@ NLKeyAppendFunction NLExecutor::selectPlainMergeKeyAppendFunction(NLChunkKind ki case NLChunkKind::Map: case NLChunkKind::Path: case NLChunkKind::PathRef: - case NLChunkKind::EntityList: throw IRException("a MERGE pattern cannot constrain a property to this value"); break; @@ -12486,10 +12463,6 @@ NLCompareFunction NLExecutor::selectCompareFunction(NLChunkKind kind) { case NLChunkKind::PathRef: throw IRException("A path column cannot be a sort key: expand it into its list first"); break; - - case NLChunkKind::EntityList: - throw IRException("A path column cannot be a sort key: a path has no order here"); - break; } bioassert(false, "Unknown NLChunkKind"); diff --git a/query/ir/exec/NLProgram.h b/query/ir/exec/NLProgram.h index 2196d69935..71ec168c73 100644 --- a/query/ir/exec/NLProgram.h +++ b/query/ir/exec/NLProgram.h @@ -14,7 +14,6 @@ #include #include -#include "EntityList.h" #include "GraphPath.h" #include "ID.h" #include "LocalMemory.h" @@ -76,7 +75,6 @@ enum class NLChunkKind { DateTime, Duration, PathRef, - EntityList, }; // Invoke handler with the column element type a chunk kind stands for, so the families of @@ -142,7 +140,7 @@ void dispatchChunkKind(NLChunkKind kind, Handler&& handler) { break; case NLChunkKind::Path: - return handler.template operator()(); + return handler.template operator()(); break; case NLChunkKind::DateTime: @@ -156,10 +154,6 @@ void dispatchChunkKind(NLChunkKind kind, Handler&& handler) { case NLChunkKind::PathRef: return handler.template operator()(); break; - - case NLChunkKind::EntityList: - return handler.template operator()(); - break; } bioassert(false, "Unknown NLChunkKind"); @@ -1102,9 +1096,8 @@ enum class PathElementsKind { }; // nl.path_elements data: one list of node IDs, one list of edge IDs or one hop count per -// row, read off the entity sequence of each path and null where that sequence is empty. -// The paths are entity lists or the bare alternating IDs of a shortest path. The lists -// are written into the query's list buffer. +// row, read off each path and null where the path is null. The lists are written into the +// query's list buffer. class NLPathElementsData : public NLFunctionData { public: NLPathElementsData(const Column* paths, @@ -1145,20 +1138,20 @@ struct NLPathEntity { bool _reversed {false}; }; -// nl.make_path data: one entity sequence per row, appended in operand order. A Path +// nl.make_path data: one path per row, appended in operand order. A Path // entity contributes the hops its handle stands for, read out of the trie. class NLMakePathData : public NLFunctionData { public: using PathEntities = std::vector; - NLMakePathData(ColumnVector* output, const PathTrie* trie) + NLMakePathData(ColumnVector* output, const PathTrie* trie) : _output(output), _trie(trie) { } const PathEntities& entities() const { return _entities; } - ColumnVector* getOutput() const { return _output; } + ColumnVector* getOutput() const { return _output; } const PathTrie* getTrie() const { return _trie; } void addEntity(const NLPathEntity& entity) { @@ -1167,7 +1160,7 @@ class NLMakePathData : public NLFunctionData { private: PathEntities _entities; - ColumnVector* _output {nullptr}; + ColumnVector* _output {nullptr}; const PathTrie* _trie {nullptr}; }; @@ -3282,10 +3275,10 @@ class NLShortestPathLoopData : public NLFunctionData { NLShortestPathState* getState() const { return _state; } void setDistanceOutput(Column* distance) { _distance = distance; } - void setPathOutput(ColumnVector* path) { _path = path; } + void setPathOutput(ColumnVector* path) { _path = path; } Column* getDistanceOutput() const { return _distance; } - ColumnVector* getPathOutput() const { return _path; } + ColumnVector* getPathOutput() const { return _path; } // The scratch the search reuses to expand one popped node's out-edges and read their // weights, allocated once at translation. The edge/node/weight columns are LocalMemory @@ -3313,7 +3306,7 @@ class NLShortestPathLoopData : public NLFunctionData { NLShortestPathState* _state {nullptr}; Column* _distance {nullptr}; - ColumnVector* _path {nullptr}; + ColumnVector* _path {nullptr}; ColumnNodeIDs* _expansionInput {nullptr}; ColumnEdgeIDs* _expandedEdges {nullptr}; diff --git a/query/ir/exec/NLTranslator.cpp b/query/ir/exec/NLTranslator.cpp index ce80ed5ac8..e889c6288e 100644 --- a/query/ir/exec/NLTranslator.cpp +++ b/query/ir/exec/NLTranslator.cpp @@ -2429,7 +2429,7 @@ void NLTranslator::translatePathElements(nl::PathElements elements, NLStmtContai } void NLTranslator::translateMakePath(nl::MakePath makePath, NLStmtContainer* body) { - ColumnVector* output = static_cast*>(allocEntityListColumn()); + ColumnVector* output = static_cast*>(allocColumnForChunkType(makePath.getResult().getType())); _valueSlots[makePath.getResult()] = output; NLMakePathData* data = _program->allocFunctionData(output, &_memory->pathTrie()); @@ -3319,7 +3319,7 @@ void NLTranslator::translateToNullable(nl::ToNullable toNullable, NLStmtContaine const auto operandChunk = mlir::cast(operandValue.getType()); const mlir::Type operandElement = operandChunk.getElementType(); const bool readsAnEntity = mlir::isa(operandElement); - const bool readsAPath = mlir::isa(operandElement); + const bool readsAPath = mlir::isa(operandElement); const bool readsAMask = isMaskElementType(operandElement); Column* result = nullptr; @@ -5363,7 +5363,7 @@ void NLTranslator::buildGroupAggregate(mlir::storage::GroupAggregateKind mlirKin // its valid rows alone. const NLChunkKind countKind = chunkKindFromElementType(countElementType); aggregate._fold = NLExecutor::selectGroupCountValidIDFold(countKind); - } else if (mlir::isa(countElementType)) { + } else if (mlir::isa(countElementType)) { aggregate._fold = NLExecutor::selectGroupCountPresentPathFold(); } else { // Every other non-nullable chunk holds no null to skip - a column a CALL @@ -5883,7 +5883,7 @@ void NLTranslator::translateShortestPathLoop(const IteratorConfig& config, _valueSlots[distanceVariable] = distanceOutput; loopData->setDistanceOutput(distanceOutput); - ColumnVector* pathOutput = static_cast*>(allocColumnForChunkType(pathVariable.getType())); + ColumnVector* pathOutput = static_cast*>(allocColumnForChunkType(pathVariable.getType())); _valueSlots[pathVariable] = pathOutput; loopData->setPathOutput(pathOutput); @@ -6673,12 +6673,12 @@ NLCountFunction NLTranslator::selectCountForChunkType(mlir::Type chunkType) { } // An entity an OPTIONAL MATCH did not match is an invalid ID and a path it did not match - // is empty, so those chunks count their other rows; every other plain chunk holds no + // is null, so those chunks count their other rows; every other plain chunk holds no // null and counts them all, whether or not it is one chunkKindFromElementType has a // kind for. if (isEntityIDElement(elementType)) { return NLExecutor::selectIDCountFunction(chunkKindFromElementType(elementType)); - } else if (mlir::isa(elementType)) { + } else if (mlir::isa(elementType)) { return NLExecutor::selectPathCountFunction(); } @@ -7138,13 +7138,6 @@ Column* NLTranslator::allocOptListColumn() { return column; } -Column* NLTranslator::allocEntityListColumn() { - ColumnVector* column = _memory->alloc>(); - column->reserve(_program->getChunkSize()); - - return column; -} - Column* NLTranslator::allocListElementColumn() { ColumnVector* column = _memory->alloc>(); column->reserve(_program->getChunkSize()); @@ -7235,8 +7228,6 @@ NLChunkKind NLTranslator::chunkKindFromElementType(mlir::Type elementType) { return NLChunkKind::Duration; } else if (mlir::isa(elementType)) { return NLChunkKind::PathRef; - } else if (mlir::isa(elementType)) { - return NLChunkKind::EntityList; } else if (mlir::isa(elementType)) { return NLChunkKind::Bool; } else if (mlir::isa(elementType)) { diff --git a/query/ir/exec/NLTranslator.h b/query/ir/exec/NLTranslator.h index c99c9f6ded..c3c82cb010 100644 --- a/query/ir/exec/NLTranslator.h +++ b/query/ir/exec/NLTranslator.h @@ -1199,7 +1199,6 @@ class NLTranslator { // A column of list cells, each a view over the query's list buffer Column* allocListColumn(); Column* allocOptListColumn(); - Column* allocEntityListColumn(); Column* allocListElementColumn(); Column* allocOptListElementColumn(); diff --git a/query/ir/lowering/DBLowering.cpp b/query/ir/lowering/DBLowering.cpp index 12d012b457..e10c2cc098 100644 --- a/query/ir/lowering/DBLowering.cpp +++ b/query/ir/lowering/DBLowering.cpp @@ -7098,13 +7098,13 @@ mlir::Value DBLowering::reducedValueChunk(mlir::Value chunk) { } } -// A path's null is the empty path, and no value of a path is read here: tested against null -// it reads as its entry count, absent where the path is empty. A walk's handle is an ID, absent +// A path's null is the path with no node, and no value of a path is read here: tested +// against null it reads as its node count, absent where the path is null. A walk's handle is an ID, absent // where it is invalid: the handle of a walk of no hops is valid. mlir::Value DBLowering::nullTestedChunk(mlir::Value chunk) { const auto chunkType = mlir::cast(chunk.getType()); const mlir::Type element = chunkType.getElementType(); - if (mlir::isa(element)) { + if (mlir::isa(element)) { return toNullableChunk(chunk, _builder.getIntegerType(64, /*isSigned=*/false)); } else if (mlir::isa(element)) { return nullableListChunk(chunk); diff --git a/samples/mlir/main.cpp b/samples/mlir/main.cpp index 085fd0d26f..979ca604f2 100644 --- a/samples/mlir/main.cpp +++ b/samples/mlir/main.cpp @@ -547,23 +547,26 @@ class PrintingSink : public NLOutputSink { return true; } - // Print one cell of a path column (a ColumnVector from an nl.shortest_path) - // as the alternating node and edge IDs it holds, target first; returns whether the - // column matched. + // Print one cell of a path column as its nodes and edges in order, a node as (id) and + // an edge as [id]; returns whether the column matched. static bool printPathCell(const Column* column, size_t row) { - const auto* paths = dynamic_cast*>(column); + const auto* paths = dynamic_cast*>(column); if (!paths) { return false; } - std::cout << "["; - for (size_t index = 0; const EntityID entity : (*paths)[row]) { - if (index > 0) { - std::cout << ", "; - } + const GraphPath& path = (*paths)[row]; + if (path.isNull()) { + std::cout << "null"; + return true; + } - std::cout << entity.getValue(); - index++; + const std::span nodes = path.getNodes(); + const std::span edges = path.getEdges(); + + std::cout << "[(" << nodes.front().getValue() << ")"; + for (size_t hop = 0; hop < edges.size(); hop++) { + std::cout << ", [" << edges[hop].getValue() << "], (" << nodes[hop + 1].getValue() << ")"; } std::cout << "]"; diff --git a/samples/tfl/main.cpp b/samples/tfl/main.cpp index 6769f75213..6bf9ccbd8d 100644 --- a/samples/tfl/main.cpp +++ b/samples/tfl/main.cpp @@ -74,7 +74,7 @@ class ConnectionNLSink : public NLOutputSink { class ShortestPathNLSink : public NLOutputSink { public: - ShortestPathNLSink(double& distance, Path& path) + ShortestPathNLSink(double& distance, GraphPath& path) : _distance(distance), _path(path) { @@ -86,12 +86,12 @@ class ShortestPathNLSink : public NLOutputSink { } _distance = (*static_cast*>(chunks[0]))[offset]; - _path = (*static_cast*>(chunks[1]))[offset]; + _path = (*static_cast*>(chunks[1]))[offset]; } private: double& _distance; - Path& _path; + GraphPath& _path; }; } @@ -352,7 +352,7 @@ int main(int argc, const char** argv) { "RETURN dist, path"; double distance = 0; - Path pathResult; + GraphPath pathResult; ShortestPathNLSink shortestPathSink(distance, pathResult); const auto spStatus = runQuery(spQuery, &shortestPathSink); @@ -390,33 +390,25 @@ int main(int argc, const char** argv) { } PropertyTypeID linePropID = linePropOpt.value()._id; - // Path is [target, edge, node, edge, node, ...] reversed - // Even indices are nodes, odd indices are edges std::vector stops; - std::vector edgeLines; - for (size_t i = 0; i < pathResult.size(); i++) { - if (i % 2 == 0) { - NodeID nodeID(pathResult[i].getValue()); - const auto* name = - reader.tryGetNodeProperty( - namePropID, nodeID).value_or(nullptr); - if (name) { - stops.emplace_back(*name); - } - } else { - EdgeID edgeID(pathResult[i].getValue()); - const auto* line = - reader.tryGetEdgeProperty( - linePropID, edgeID).value_or(nullptr); - if (line) { - edgeLines.emplace_back(*line); - } + for (const NodeID nodeID : pathResult.getNodes()) { + const auto* name = + reader.tryGetNodeProperty( + namePropID, nodeID).value_or(nullptr); + if (name) { + stops.emplace_back(*name); } } - // Reverse: path goes target->source, we want source->target - std::reverse(stops.begin(), stops.end()); - std::reverse(edgeLines.begin(), edgeLines.end()); + std::vector edgeLines; + for (const EdgeID edgeID : pathResult.getEdges()) { + const auto* line = + reader.tryGetEdgeProperty( + linePropID, edgeID).value_or(nullptr); + if (line) { + edgeLines.emplace_back(*line); + } + } // Strip virtual hub nodes (__src at front, __dst at back) if (stops.size() >= 2 && stops.front() == "__src") { diff --git a/storage/CMakeLists.txt b/storage/CMakeLists.txt index bab79c5a70..b847922032 100644 --- a/storage/CMakeLists.txt +++ b/storage/CMakeLists.txt @@ -3,7 +3,7 @@ set(storage_sources ChangeManager.cpp DebugDump.cpp RowStore.cpp - EntityList.cpp + GraphPath.cpp buffers/BufferChunk.cpp buffers/RawBuffer.cpp diff --git a/storage/EntityList.cpp b/storage/EntityList.cpp deleted file mode 100644 index 80ddf10fdf..0000000000 --- a/storage/EntityList.cpp +++ /dev/null @@ -1,15 +0,0 @@ -#include "EntityList.h" - -using namespace db; - -EntityList::EntityList() = default; - -EntityList::~EntityList() = default; - -EntityList::EntityList(const EntityList& other) = default; - -EntityList::EntityList(EntityList&& other) noexcept = default; - -EntityList& EntityList::operator=(const EntityList& other) = default; - -EntityList& EntityList::operator=(EntityList&& other) noexcept = default; diff --git a/storage/EntityList.h b/storage/EntityList.h deleted file mode 100644 index 1c561e8b17..0000000000 --- a/storage/EntityList.h +++ /dev/null @@ -1,73 +0,0 @@ -#pragma once - -#include "EntityType.h" -#include "ID.h" - -namespace db { - -class EntityList { -public: - struct Entry { - EntityType _type {}; - EntityID _id; - - bool operator==(const Entry& other) const { - return _type == other._type && _id == other._id; - } - }; - - using Container = std::vector; - - EntityList(); - ~EntityList(); - - EntityList(const EntityList&); - EntityList(EntityList&&) noexcept; - EntityList& operator=(const EntityList&); - EntityList& operator=(EntityList&&) noexcept; - - void add(EntityType type, EntityID id) { - _entries.emplace_back(type, id); - } - - void add() { - _entries.emplace_back(); - } - - Entry& back() { - return _entries.back(); - } - - void assign(Container::const_iterator first, Container::const_iterator last) { - _entries.assign(first, last); - } - - void clear() { - _entries.clear(); - } - - void resize(size_t size) { - _entries.resize(size); - } - - void reserve(size_t size) { - _entries.reserve(size); - } - - EntityList::Entry& operator[](size_t idx) { - return _entries[idx]; - } - - bool empty() const { return _entries.empty(); } - size_t size() const { return _entries.size(); } - size_t capacity() const { return _entries.capacity(); } - const Container& getEntries() const { return _entries; } - - Container::const_iterator begin() const { return _entries.begin(); } - Container::const_iterator end() const { return _entries.end(); } - -private: - Container _entries; -}; - -} diff --git a/storage/GraphPath.cpp b/storage/GraphPath.cpp new file mode 100644 index 0000000000..a5c01a86db --- /dev/null +++ b/storage/GraphPath.cpp @@ -0,0 +1,29 @@ +#include "GraphPath.h" + +using namespace db; + +GraphPath::GraphPath() = default; + +GraphPath::~GraphPath() = default; + +GraphPath::GraphPath(const GraphPath& other) = default; + +GraphPath::GraphPath(GraphPath&& other) noexcept = default; + +GraphPath& GraphPath::operator=(const GraphPath& other) = default; + +GraphPath& GraphPath::operator=(GraphPath&& other) noexcept = default; + +bool GraphPath::operator==(const GraphPath& other) const { + return _nodes == other._nodes && _edges == other._edges; +} + +void GraphPath::resize(size_t nodeCount, size_t edgeCount) { + _nodes.resize(nodeCount); + _edges.resize(edgeCount); +} + +void GraphPath::clear() { + _nodes.clear(); + _edges.clear(); +} diff --git a/storage/GraphPath.h b/storage/GraphPath.h index a5fb0ee168..d014e2d10b 100644 --- a/storage/GraphPath.h +++ b/storage/GraphPath.h @@ -1,8 +1,44 @@ #pragma once +#include +#include +#include + #include "ID.h" namespace db { -// The vector represents the alternating node and edge IDs in a path. -using Path = std::vector; + +// Edge i joins node i to node i + 1, so a path holds one node more than it has edges. +// A path with no node is the null path, which an OPTIONAL MATCH that missed binds. +class GraphPath { +public: + GraphPath(); + ~GraphPath(); + + GraphPath(const GraphPath& other); + GraphPath(GraphPath&& other) noexcept; + GraphPath& operator=(const GraphPath& other); + GraphPath& operator=(GraphPath&& other) noexcept; + + bool operator==(const GraphPath& other) const; + + void addNode(NodeID node) { _nodes.push_back(node); } + void addEdge(EdgeID edge) { _edges.push_back(edge); } + + void resize(size_t nodeCount, size_t edgeCount); + void clear(); + + bool isNull() const { return _nodes.empty(); } + size_t getLength() const { return _edges.size(); } + + std::span getNodes() const { return _nodes; } + std::span getEdges() const { return _edges; } + std::span getNodes() { return _nodes; } + std::span getEdges() { return _edges; } + +private: + std::vector _nodes; + std::vector _edges; +}; + } diff --git a/storage/columns/AllowedKinds.h b/storage/columns/AllowedKinds.h index de42be29da..df2f10ff42 100644 --- a/storage/columns/AllowedKinds.h +++ b/storage/columns/AllowedKinds.h @@ -19,8 +19,6 @@ namespace db { -class EntityList; - template struct TupleFlatten { using Type = decltype(std::tuple_cat(std::declval()...)); @@ -608,7 +606,7 @@ struct MaskedPairs { MixedKind, MixedKind, MixedKind, - MixedKind, + MixedKind, MixedKind, MixedKind, MixedKind, @@ -616,8 +614,7 @@ struct MaskedPairs { MixedKind, MixedKind, MixedKind, - MixedKind, - MixedKind + MixedKind >; using Excluded = ExcludedContainers< @@ -740,7 +737,7 @@ struct OutputtedTypes { NodeID, EdgeID, LabelID, - Path, + GraphPath, LabelSetID, EdgeTypeID, ValueType, @@ -758,7 +755,6 @@ struct OutputtedTypes { std::string, size_t, - EntityList, ListView, std::optional, @@ -1011,7 +1007,7 @@ struct CartesianProductKinds { // Lists ListView, ListElementView, - EntityList, + GraphPath, // Owning strings occasionally needed for procedures; tests std::string diff --git a/storage/columns/ColumnDispatcher.h b/storage/columns/ColumnDispatcher.h index 7f8cea590e..59aab97198 100644 --- a/storage/columns/ColumnDispatcher.h +++ b/storage/columns/ColumnDispatcher.h @@ -5,7 +5,7 @@ #include "ColumnConst.h" #include "ColumnOptVector.h" #include "ColumnKind.h" -#include "EntityList.h" +#include "GraphPath.h" #include "list/ListElementView.h" #include "metadata/PropertyType.h" #include "versioning/ChangeID.h" @@ -24,7 +24,7 @@ using AllValueTypes = TypeList< NodeID, EdgeID, EntityID, - Path, + GraphPath, LabelID, EdgeTypeID, PropertyTypeID, @@ -53,7 +53,6 @@ using AllValueTypes = TypeList< std::optional, const CommitBuilder*, const Change*, - EntityList, ListView, std::optional, ListElementView @@ -123,7 +122,7 @@ struct ColumnTypeFromKind { switch ((col)->getKind()) { \ COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ - COL_CASE(ColumnVector) \ + COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ @@ -149,7 +148,6 @@ struct ColumnTypeFromKind { COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ - COL_CASE(ColumnVector) \ COL_CASE(ColumnVector) \ \ default: { \ @@ -187,7 +185,7 @@ struct ColumnTypeFromKind { CONST_COL_CASE(ColumnVector) \ CONST_COL_CASE(ColumnVector) \ CONST_COL_CASE(ColumnVector) \ - CONST_COL_CASE(ColumnVector) \ + CONST_COL_CASE(ColumnVector) \ \ default: { \ throw FatalException(fmt::format( \ diff --git a/storage/columns/InternalKind.h b/storage/columns/InternalKind.h index 66e6eadc60..334c09d80d 100644 --- a/storage/columns/InternalKind.h +++ b/storage/columns/InternalKind.h @@ -5,7 +5,6 @@ #include #include -#include "EntityList.h" #include "columns/KindTypes.h" #include "map/MapEntryView.h" #include "metadata/PropertyNull.h" @@ -55,8 +54,7 @@ class InternalKind { NodeID, EdgeID, PathRef, - Path, - EntityList, + GraphPath, LabelSetID, LabelSet, EdgeTypeID, diff --git a/storage/list/PathTrie.cpp b/storage/list/PathTrie.cpp index 254c29cdbd..d674b00980 100644 --- a/storage/list/PathTrie.cpp +++ b/storage/list/PathTrie.cpp @@ -83,22 +83,24 @@ PathRef PathTrie::handleOf(size_t arena, size_t index) { return PathRef((static_cast(arena) << indexBits) | index); } -void PathTrie::appendHops(PathRef path, EntityList& entities, bool reversed) const { +void PathTrie::appendHops(PathRef path, GraphPath& graphPath, bool reversed) const { const uint64_t depth = getDepth(path); - const bool writesOverTheNodeAhead = reversed && !entities.empty(); - const size_t firstEntry = writesOverTheNodeAhead ? entities.size() - 1 : entities.size(); - entities.resize(firstEntry + depth * 2); + const size_t nodeCount = graphPath.getNodes().size(); + const size_t firstEdge = graphPath.getLength(); + const bool writesOverTheLastNode = reversed && !graphPath.isNull(); + const size_t firstNode = writesOverTheLastNode ? nodeCount - 1 : nodeCount; + graphPath.resize(firstNode + depth, firstEdge + depth); + + const std::span nodes = graphPath.getNodes(); + const std::span edges = graphPath.getEdges(); PathRef current = path; for (uint64_t hop = depth; hop > 0; hop--) { const PathTrieEntry& entry = get(current); - const size_t entryIndex = firstEntry + (reversed ? (depth - hop) : (hop - 1)) * 2; - - const EntityList::Entry edge {EntityType::Edge, EntityID(entry._edge.getValue())}; - const EntityList::Entry node {EntityType::Node, EntityID(entry._node.getValue())}; + const size_t step = reversed ? (depth - hop) : (hop - 1); - entities[entryIndex] = reversed ? node : edge; - entities[entryIndex + 1] = reversed ? edge : node; + nodes[firstNode + step] = entry._node; + edges[firstEdge + step] = entry._edge; current = entry._parent; } diff --git a/storage/list/PathTrie.h b/storage/list/PathTrie.h index 2eb9bbdd33..4a4902cf16 100644 --- a/storage/list/PathTrie.h +++ b/storage/list/PathTrie.h @@ -7,7 +7,7 @@ #include "ListBuffer.h" #include "ListView.h" -#include "EntityList.h" +#include "GraphPath.h" #include "ID.h" namespace db { @@ -55,11 +55,11 @@ class PathTrie { void clear(); - // Appends the hops of the path in walk order, an edge entry then the node it lands on - // for each. Reversed it appends them from the far end back, the node an edge lands on - // before that edge, over the node entry ahead: that is the node the walk ended at, and - // a walk of no hops leaves it to the seed the caller writes next - void appendHops(PathRef path, EntityList& entities, bool reversed) const; + // Appends the hops of the path in walk order, each edge and the node it lands on. + // Reversed it appends them from the far end back, over the last node of the path: that + // is the node the walk ended at, and a walk of no hops leaves it to the seed the caller + // writes next + void appendHops(PathRef path, GraphPath& graphPath, bool reversed) const; // Each list in walk order, or backwards from the node the walk ended on ListView expandEdges(PathRef path, QueryListBuffer& buffer, bool reversed, PathHopStride hops = {}) const; diff --git a/test/net/TuringProtoRoundTripTest.cpp b/test/net/TuringProtoRoundTripTest.cpp index bd4aa3f914..0b46625b91 100644 --- a/test/net/TuringProtoRoundTripTest.cpp +++ b/test/net/TuringProtoRoundTripTest.cpp @@ -23,6 +23,7 @@ #include "TuringProtoHeaders.h" #include "TuringProtoInBuf.h" #include "TuringProtoOutBuf.h" +#include "GraphPath.h" #include "LocalMemory.h" #include "columns/ColumnConst.h" #include "columns/ColumnMask.h" @@ -1951,3 +1952,58 @@ TEST(TuringProtoRoundTripTest, RoundTripsListElementViewColumnOfMaps) { EXPECT_EQ(second.front().getKey(), "b"); EXPECT_EQ(second.front().getValueAs(), 2); } + +// A path goes over as its node count, then its nodes, then its edges. The long path's two +// payloads each span several packets at these sizes, so a resume lands inside the nodes, +// between them and the edges, and inside the edges. The null path and the zero-length path +// carry no edges at all. +TEST(TuringProtoRoundTripTest, RoundTripsGraphPathColumnsAcrossChunkSizes) { + for (const size_t chunkSize : std::array {48, 64, 97, 256}) { + SCOPED_TRACE(::testing::Message() << "chunkSize=" << chunkSize); + + db::LocalMemory localMem; + db::DataframeManager dfMan; + db::Dataframe source; + + db::GraphPath longPath; + longPath.addNode(db::NodeID(1000)); + for (uint64_t hop = 1; hop <= 40; hop++) { + longPath.addEdge(db::EdgeID(2000 + hop)); + longPath.addNode(db::NodeID(1000 + hop)); + } + + db::GraphPath zeroLengthPath; + zeroLengthPath.addNode(db::NodeID(7)); + + db::GraphPath twoHopPath; + twoHopPath.addNode(db::NodeID(3)); + twoHopPath.addEdge(db::EdgeID(4)); + twoHopPath.addNode(db::NodeID(5)); + twoHopPath.addEdge(db::EdgeID(6)); + twoHopPath.addNode(db::NodeID(3)); + + const std::vector paths {longPath, db::GraphPath {}, zeroLengthPath, twoHopPath, longPath}; + + auto* column = localMem.alloc>(); + for (const db::GraphPath& path : paths) { + column->push_back(path); + } + addColumn(&dfMan, &source, "p", column); + + const auto packets = encodeDataframeWithChunkSize(source, chunkSize); + expectPacketSequence(packets, true); + + net::proto::ChunkedBuffer embeddingBuffer; + net::proto::ChunkedBuffer stringBuffer; + db::ListBuffer<> listBuffer; + db::MapBuffer<> mapBuffer; + db::Dataframe decoded; + std::vector schemas; + decodeChunkPackets(packets, &localMem, &embeddingBuffer, &stringBuffer, &listBuffer, &mapBuffer, &dfMan, &decoded, &schemas); + + ASSERT_EQ(decoded.cols().size(), 1u); + const auto* decodedPaths = decoded.cols().at(0)->as>(); + ASSERT_NE(decodedPaths, nullptr); + EXPECT_EQ(decodedPaths->getRaw(), paths); + } +} diff --git a/test/query-test-suite/QueryResultFormatter.cpp b/test/query-test-suite/QueryResultFormatter.cpp index f1f010daea..5b57cbaf6e 100644 --- a/test/query-test-suite/QueryResultFormatter.cpp +++ b/test/query-test-suite/QueryResultFormatter.cpp @@ -1,14 +1,12 @@ #include "QueryResultFormatter.h" #include -#include #include #include #include #include "BioAssert.h" -#include "EntityList.h" #include "GraphPath.h" #include "ID.h" #include "list/ListElementView.h" @@ -44,24 +42,20 @@ namespace { return value; } -[[maybe_unused]] std::string valueToString(const db::Path& value) { - std::string result; - - if (value.empty()) { - return ""; +[[maybe_unused]] std::string valueToString(const db::GraphPath& value) { + if (value.isNull()) { + return "null"; } - const auto reversed = value | std::views::reverse; - size_t i = 0; - for (auto val : reversed) { - if (i % 2 == 0) { - result += fmt::format("({})", val.getValue()); - } else { - result += fmt::format("-[{}]->", val.getValue()); - } - ++i; + const std::span nodes = value.getNodes(); + const std::span edges = value.getEdges(); + + std::string result = fmt::format("[({})", nodes.front().getValue()); + for (size_t hop = 0; hop < edges.size(); hop++) { + result += fmt::format(", [{}], ({})", edges[hop].getValue(), nodes[hop + 1].getValue()); } + result += "]"; return result; } @@ -146,26 +140,6 @@ template return valueToString(*value); } -[[maybe_unused]] std::string valueToString(const db::EntityList& value) { - std::string result = "["; - size_t i = 0; - - for (const auto& entry : value) { - if (i++ > 0) { - result += ", "; - } - - if (entry._type == db::EntityType::Node) { - result += fmt::format("({})", entry._id.getValue()); - } else { - result += fmt::format("[{}]", entry._id.getValue()); - } - } - - result += "]"; - return result; -} - [[maybe_unused]] std::string valueToString(const db::ListElementView element) { const auto writeTyped = [](const db::ListElementView element) -> std::string { return valueToString(element.getAs()); diff --git a/test/query/ir/StringRowSink.cpp b/test/query/ir/StringRowSink.cpp index 16e7fb3550..0677a3b06a 100644 --- a/test/query/ir/StringRowSink.cpp +++ b/test/query/ir/StringRowSink.cpp @@ -8,7 +8,6 @@ #include -#include "EntityList.h" #include "GraphPath.h" #include "ID.h" #include "columns/ColumnConst.h" @@ -316,19 +315,25 @@ bool textOfOptionalList(const Column* chunk, size_t rowIndex, std::string& text) return true; } +// A path, read as its nodes and edges in order: a node as (id), an edge as [id] bool textOfPath(const Column* chunk, size_t rowIndex, std::string& text) { - const auto* column = dynamic_cast*>(chunk); + const auto* column = dynamic_cast*>(chunk); if (!column) { return false; } - text.clear(); - for (const EntityID entity : column->getRaw()[rowIndex]) { - if (!text.empty()) { - text += ", "; - } + const GraphPath& path = column->getRaw()[rowIndex]; + if (path.isNull()) { + text = "null"; + return true; + } - text += fmt::format("{}", entity.getValue()); + const std::span nodes = path.getNodes(); + const std::span edges = path.getEdges(); + + text = fmt::format("({})", nodes.front().getValue()); + for (size_t hop = 0; hop < edges.size(); hop++) { + text += fmt::format(", [{}], ({})", edges[hop].getValue(), nodes[hop + 1].getValue()); } return true; @@ -365,35 +370,6 @@ bool textOfConstOptionalBool(const Column* chunk, size_t rowIndex, std::string& return true; } -// A named path, read as its nodes and edges in order: a node as (id), an edge as [id] -bool textOfEntityList(const Column* chunk, size_t rowIndex, std::string& text) { - const auto* column = dynamic_cast*>(chunk); - if (!column) { - return false; - } - - const EntityList& path = column->getRaw()[rowIndex]; - if (path.empty()) { - text = "null"; - return true; - } - - text.clear(); - for (const EntityList::Entry& entry : path) { - if (!text.empty()) { - text += ", "; - } - - if (entry._type == EntityType::Node) { - text += fmt::format("({})", entry._id.getValue()); - } else { - text += fmt::format("[{}]", entry._id.getValue()); - } - } - - return true; -} - template bool textOfOptional(const Column* chunk, size_t rowIndex, std::string& text) { const auto* column = dynamic_cast*>(chunk); @@ -547,8 +523,6 @@ std::string StringRowSink::cellText(const Column* chunk, size_t rowIndex) { return text; } else if (textOfPath(chunk, rowIndex, text)) { return text; - } else if (textOfEntityList(chunk, rowIndex, text)) { - return text; } throw std::runtime_error("StringRowSink cannot read this column kind as text"); diff --git a/test/query/ir/engine/EquivalenceTest.cpp b/test/query/ir/engine/EquivalenceTest.cpp index ce048302b8..fd6fce76f2 100644 --- a/test/query/ir/engine/EquivalenceTest.cpp +++ b/test/query/ir/engine/EquivalenceTest.cpp @@ -30,7 +30,6 @@ #include "CypherAnalyzer.h" #include "CypherParser.h" -#include "EntityList.h" #include "Graph.h" #include "GraphPath.h" #include "ID.h" @@ -80,19 +79,20 @@ std::string valueToString(const std::string_view value) { return std::string(value); } -std::string valueToString(const Path& value) { - std::string result; +std::string valueToString(const GraphPath& value) { + if (value.isNull()) { + return "null"; + } - size_t index = 0; - for (const auto entity : value | std::views::reverse) { - if (index % 2 == 0) { - result += fmt::format("({})", entity.getValue()); - } else { - result += fmt::format("-[{}]->", entity.getValue()); - } - index++; + const std::span nodes = value.getNodes(); + const std::span edges = value.getEdges(); + + std::string result = fmt::format("[({})", nodes.front().getValue()); + for (size_t hop = 0; hop < edges.size(); hop++) { + result += fmt::format(", [{}], ({})", edges[hop].getValue(), nodes[hop + 1].getValue()); } + result += "]"; return result; } @@ -173,26 +173,6 @@ std::string valueToString(const std::optional& value) { return valueToString(*value); } -std::string valueToString(const EntityList& value) { - std::string result = "["; - size_t index = 0; - - for (const auto& entity : value) { - if (index++ > 0) { - result += ", "; - } - - if (entity._type == EntityType::Node) { - result += fmt::format("({})", entity._id.getValue()); - } else { - result += fmt::format("[{}]", entity._id.getValue()); - } - } - - result += "]"; - return result; -} - std::string valueToString(const ListElementView element) { const auto writeTyped = [](const ListElementView element) -> std::string { return valueToString(element.getAs()); diff --git a/test/query/ir/path/CypherShortestPathTest.cpp b/test/query/ir/path/CypherShortestPathTest.cpp index d7aa688f4b..8cd082acb1 100644 --- a/test/query/ir/path/CypherShortestPathTest.cpp +++ b/test/query/ir/path/CypherShortestPathTest.cpp @@ -87,8 +87,7 @@ class CypherShortestPathTest : public TuringTest { // B -7-> D G -1-> A // // Roads are added grouped by source in node order, so the committed edge IDs are - // A->B 0, A->C 1, B->C 2, B->D 3, C->D 4, C->E 5, D->E 6, E->F 7, G->A 8 - the paths - // below are stored target-first as alternating node and edge IDs. + // A->B 0, A->C 1, B->C 2, B->D 3, C->D 4, C->E 5, D->E 6, E->F 7, G->A 8. void buildWeightedGraph(Graph* graph) { JobSystem jobSystem; jobSystem.init(); @@ -162,7 +161,7 @@ class CypherShortestPathTest : public TuringTest { }; TEST_F(CypherShortestPathTest, findsCheapestTwoHopPath) { - const std::vector expected {{"3", "2, 2, 1, 0, 0"}}; + const std::vector expected {{"3", "(0), [0], (1), [2], (2)"}}; EXPECT_EQ(run(_graphName, "MATCH (a {name: 'A'}), (b {name: 'C'}) " "SHORTESTPATH(a, b, weight, d, p) RETURN d, p"), @@ -170,7 +169,7 @@ TEST_F(CypherShortestPathTest, findsCheapestTwoHopPath) { } TEST_F(CypherShortestPathTest, findsCheapestFourHopPath) { - const std::vector expected {{"5", "4, 6, 3, 4, 2, 2, 1, 0, 0"}}; + const std::vector expected {{"5", "(0), [0], (1), [2], (2), [4], (3), [6], (4)"}}; EXPECT_EQ(run(_graphName, "MATCH (a {name: 'A'}), (b {name: 'E'}) " "SHORTESTPATH(a, b, weight, d, p) RETURN d, p"), @@ -178,7 +177,7 @@ TEST_F(CypherShortestPathTest, findsCheapestFourHopPath) { } TEST_F(CypherShortestPathTest, multiSourceMatchPicksCheapest) { - const std::vector expected {{"3", "3, 4, 2, 2, 1"}}; + const std::vector expected {{"3", "(1), [2], (2), [4], (3)"}}; EXPECT_EQ(run(_graphName, "MATCH (a), (b {name: 'D'}) WHERE a.name = 'A' OR a.name = 'B' " "SHORTESTPATH(a, b, weight, d, p) RETURN d, p"), @@ -186,7 +185,7 @@ TEST_F(CypherShortestPathTest, multiSourceMatchPicksCheapest) { } TEST_F(CypherShortestPathTest, multiTargetMatchStopsAtNearest) { - const std::vector expected {{"4", "3, 4, 2, 2, 1, 0, 0"}}; + const std::vector expected {{"4", "(0), [0], (1), [2], (2), [4], (3)"}}; EXPECT_EQ(run(_graphName, "MATCH (a {name: 'A'}), (b) WHERE b.name = 'D' OR b.name = 'F' " "SHORTESTPATH(a, b, weight, d, p) RETURN d, p"), @@ -194,7 +193,7 @@ TEST_F(CypherShortestPathTest, multiTargetMatchStopsAtNearest) { } TEST_F(CypherShortestPathTest, multiSourceMultiTargetPicksGlobalCheapest) { - const std::vector expected {{"3", "3, 4, 2, 2, 1"}}; + const std::vector expected {{"3", "(1), [2], (2), [4], (3)"}}; EXPECT_EQ(run(_graphName, "MATCH (a), (b) WHERE (a.name = 'A' OR a.name = 'B') " "AND (b.name = 'D' OR b.name = 'E') " @@ -203,7 +202,7 @@ TEST_F(CypherShortestPathTest, multiSourceMultiTargetPicksGlobalCheapest) { } TEST_F(CypherShortestPathTest, zeroLengthPathWhenSourceIsTarget) { - const std::vector expected {{"0", "0"}}; + const std::vector expected {{"0", "(0)"}}; EXPECT_EQ(run(_graphName, "MATCH (a {name: 'A'}), (b {name: 'A'}) " "SHORTESTPATH(a, b, weight, d, p) RETURN d, p"), @@ -225,7 +224,7 @@ TEST_F(CypherShortestPathTest, emitsNoRowWhenTargetUnreachable) { } TEST_F(CypherShortestPathTest, handlesDoubleWeights) { - const std::vector expected {{"3.5", "2, 2, 1, 0, 0"}}; + const std::vector expected {{"3.5", "(0), [0], (1), [2], (2)"}}; EXPECT_EQ(run(_doubleGraphName, "MATCH (a {name: 'A'}), (b {name: 'C'}) " "SHORTESTPATH(a, b, weight, d, p) RETURN d, p"), diff --git a/test/query/ir/path/ShortestPathElementsTest.cpp b/test/query/ir/path/ShortestPathElementsTest.cpp index f69317bcb2..9904b70de6 100644 --- a/test/query/ir/path/ShortestPathElementsTest.cpp +++ b/test/query/ir/path/ShortestPathElementsTest.cpp @@ -13,8 +13,8 @@ using Rows = std::vector; } -// nodes(p), relationships(p) and length(p) over the path a SHORTESTPATH binds. The path is -// stored target-first: Adam reaches Ghosts through Remy as Ghosts(6) -e1- Remy(0) -e4- Adam(1). +// nodes(p), relationships(p) and length(p) over the path a SHORTESTPATH binds: Adam reaches +// Ghosts through Remy as Adam(1) -e4- Remy(0) -e1- Ghosts(6). class ShortestPathElementsTest : public CallV3Test { }; @@ -24,7 +24,7 @@ TEST_F(ShortestPathElementsTest, readsTheElementsOfATwoHopPath) { "SHORTESTPATH(n, m, duration, d, p) RETURN d, nodes(p), relationships(p), length(p)", sink); - EXPECT_EQ(sink.getRows(), (Rows {{"40", "6, 0, 1", "1, 4", "2"}})); + EXPECT_EQ(sink.getRows(), (Rows {{"40", "1, 0, 6", "4, 1", "2"}})); } TEST_F(ShortestPathElementsTest, readsTheElementsOfAZeroLengthPath) { diff --git a/test/query/ir/path/ShortestPathEndpointPredicateTest.cpp b/test/query/ir/path/ShortestPathEndpointPredicateTest.cpp index 7defc4ca97..abe27047cd 100644 --- a/test/query/ir/path/ShortestPathEndpointPredicateTest.cpp +++ b/test/query/ir/path/ShortestPathEndpointPredicateTest.cpp @@ -40,8 +40,6 @@ using Rows = std::vector; // Remy(0) -e1-> Ghosts(6) 20 // Adam(1) -e4-> Remy(0) 20 // Ghosts(6) -e7-> Remy(0) 200 -// -// Paths are read target-first as alternating node and edge IDs. class ShortestPathEndpointPredicateTest : public TuringTest { protected: void initialize() override { @@ -77,14 +75,14 @@ class ShortestPathEndpointPredicateTest : public TuringTest { // Remy and Adam both reach Ghosts, Remy in one hop and Adam in two, so the source the // predicate keeps is the one the answer names. TEST_F(ShortestPathEndpointPredicateTest, keepsBothSourcesWithoutAPredicateOverTheEndpoints) { - const Rows expected = {{"20", "6, 1, 0"}}; + const Rows expected = {{"20", "(0), [1], (6)"}}; expectRows("MATCH (n), (m) WHERE m.name = 'Ghosts' AND (n.name = 'Remy' OR n.name = 'Adam') " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", expected); } TEST_F(ShortestPathEndpointPredicateTest, narrowsTheSourceSetToRemy) { - const Rows expected = {{"20", "6, 1, 0"}}; + const Rows expected = {{"20", "(0), [1], (6)"}}; expectRows("MATCH (n), (m) WHERE m.name = 'Ghosts' AND (n.name = 'Remy' OR n.name = 'Adam') " "AND n.name > m.name " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", @@ -92,7 +90,7 @@ TEST_F(ShortestPathEndpointPredicateTest, narrowsTheSourceSetToRemy) { } TEST_F(ShortestPathEndpointPredicateTest, narrowsTheSourceSetToAdam) { - const Rows expected = {{"40", "6, 1, 0, 4, 1"}}; + const Rows expected = {{"40", "(1), [4], (0), [1], (6)"}}; expectRows("MATCH (n), (m) WHERE m.name = 'Ghosts' AND (n.name = 'Remy' OR n.name = 'Adam') " "AND n.name < m.name " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", @@ -101,14 +99,14 @@ TEST_F(ShortestPathEndpointPredicateTest, narrowsTheSourceSetToAdam) { // The same over the target set: Ghosts reaches Remy at 200 and Adam one hop further. TEST_F(ShortestPathEndpointPredicateTest, keepsBothTargetsWithoutAPredicateOverTheEndpoints) { - const Rows expected = {{"200", "0, 7, 6"}}; + const Rows expected = {{"200", "(6), [7], (0)"}}; expectRows("MATCH (n), (m) WHERE n.name = 'Ghosts' AND (m.name = 'Remy' OR m.name = 'Adam') " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", expected); } TEST_F(ShortestPathEndpointPredicateTest, narrowsTheTargetSetToRemy) { - const Rows expected = {{"200", "0, 7, 6"}}; + const Rows expected = {{"200", "(6), [7], (0)"}}; expectRows("MATCH (n), (m) WHERE n.name = 'Ghosts' AND (m.name = 'Remy' OR m.name = 'Adam') " "AND m.name > n.name " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", @@ -116,7 +114,7 @@ TEST_F(ShortestPathEndpointPredicateTest, narrowsTheTargetSetToRemy) { } TEST_F(ShortestPathEndpointPredicateTest, narrowsTheTargetSetToAdam) { - const Rows expected = {{"220", "1, 0, 0, 7, 6"}}; + const Rows expected = {{"220", "(6), [7], (0), [0], (1)"}}; expectRows("MATCH (n), (m) WHERE n.name = 'Ghosts' AND (m.name = 'Remy' OR m.name = 'Adam') " "AND m.name < n.name " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", @@ -134,7 +132,7 @@ TEST_F(ShortestPathEndpointPredicateTest, emitsNoRowWhenThePredicateEmptiesTheMa } TEST_F(ShortestPathEndpointPredicateTest, answersTheZeroLengthPathWithoutTheInequality) { - const Rows expected = {{"0", "0"}}; + const Rows expected = {{"0", "(0)"}}; expectRows("MATCH (n), (m) WHERE n.name = 'Remy' AND m.name = 'Remy' " "SHORTESTPATH(n, m, duration, d, p) RETURN d, p", expected); @@ -144,13 +142,13 @@ TEST_F(ShortestPathEndpointPredicateTest, answersTheZeroLengthPathWithoutTheIneq // each came from, so an inequality over the whole graph answers what no predicate at all // answers: every node is both a source and a target, and the zero-length path wins. TEST_F(ShortestPathEndpointPredicateTest, answersZeroForTheInequalityOverTheWholeGraph) { - const Rows expected = {{"0", "0"}}; + const Rows expected = {{"0", "(0)"}}; expectRows("MATCH (n), (m) WHERE n <> m SHORTESTPATH(n, m, duration, d, p) RETURN d, p", expected); } TEST_F(ShortestPathEndpointPredicateTest, answersZeroForTheWholeGraphWithoutAPredicate) { - const Rows expected = {{"0", "0"}}; + const Rows expected = {{"0", "(0)"}}; expectRows("MATCH (n), (m) SHORTESTPATH(n, m, duration, d, p) RETURN d, p", expected); } diff --git a/test/query/ir/path/ShortestPathLoweringTest.cpp b/test/query/ir/path/ShortestPathLoweringTest.cpp index 22faf9ee6f..6a9c9d4b51 100644 --- a/test/query/ir/path/ShortestPathLoweringTest.cpp +++ b/test/query/ir/path/ShortestPathLoweringTest.cpp @@ -222,9 +222,9 @@ class ShortestPathLoweringTest : public TuringTest { TEST_F(ShortestPathLoweringTest, findsCheapestTwoHopPath) { const std::unique_ptr graph = buildWeightedGraph(); - // A -> B -> C = 1 + 2 = 3 beats the direct A -> C edge at 4. Path stored target-first - // as alternating node/edge IDs: C(2), edge B->C(2), B(1), edge A->B(0), A(0). - const std::vector expected {{"3", "2, 2, 1, 0, 0"}}; + // A -> B -> C = 1 + 2 = 3 beats the direct A -> C edge at 4: A(0), edge A->B(0), B(1), + // edge B->C(2), C(2). + const std::vector expected {{"3", "(0), [0], (1), [2], (2)"}}; EXPECT_EQ(run(*graph, "0", "2"), expected); } @@ -232,7 +232,7 @@ TEST_F(ShortestPathLoweringTest, findsCheapestThreeHopPath) { const std::unique_ptr graph = buildWeightedGraph(); // A -> B -> C -> D = 1 + 2 + 1 = 4 beats A -> C -> D (5) and A -> B -> D (8). - const std::vector expected {{"4", "3, 4, 2, 2, 1, 0, 0"}}; + const std::vector expected {{"4", "(0), [0], (1), [2], (2), [4], (3)"}}; EXPECT_EQ(run(*graph, "0", "3"), expected); } @@ -240,7 +240,7 @@ TEST_F(ShortestPathLoweringTest, findsCheapestFourHopPath) { const std::unique_ptr graph = buildWeightedGraph(); // A -> B -> C -> D -> E = 5 beats A -> C -> E (6) and A -> B -> C -> E (6). - const std::vector expected {{"5", "4, 6, 3, 4, 2, 2, 1, 0, 0"}}; + const std::vector expected {{"5", "(0), [0], (1), [2], (2), [4], (3), [6], (4)"}}; EXPECT_EQ(run(*graph, "0", "4"), expected); } @@ -249,7 +249,7 @@ TEST_F(ShortestPathLoweringTest, multiSourcePicksCheapest) { // With both A and B as sources, D is cheapest from B (B -> C -> D = 3) rather than from // A (A -> B -> C -> D = 4), so the path stops at B. - const std::vector expected {{"3", "3, 4, 2, 2, 1"}}; + const std::vector expected {{"3", "(1), [2], (2), [4], (3)"}}; EXPECT_EQ(run(*graph, "0, 1", "3"), expected); } @@ -257,7 +257,7 @@ TEST_F(ShortestPathLoweringTest, multiTargetStopsAtNearest) { const std::unique_ptr graph = buildWeightedGraph(); // D (distance 4) is nearer than F (distance 7), so the search halts at D. - const std::vector expected {{"4", "3, 4, 2, 2, 1, 0, 0"}}; + const std::vector expected {{"4", "(0), [0], (1), [2], (2), [4], (3)"}}; EXPECT_EQ(run(*graph, "0", "3, 5"), expected); } @@ -266,7 +266,7 @@ TEST_F(ShortestPathLoweringTest, zeroLengthPathWhenSourceIsTarget) { // A source that is already a target is reached at distance 0, and the path is the lone // node with no edges. - const std::vector expected {{"0", "0"}}; + const std::vector expected {{"0", "(0)"}}; EXPECT_EQ(run(*graph, "0", "0"), expected); } @@ -289,7 +289,7 @@ TEST_F(ShortestPathLoweringTest, handlesDoubleWeights) { const std::unique_ptr graph = buildTriangleGraph(ValueType::Double, 1.5, 2.0, 5.0); // A -> B -> C = 3.5 beats the direct A -> C at 5.0, exercising the f64 search. - const std::vector expected {{"3.5", "2, 2, 1, 0, 0"}}; + const std::vector expected {{"3.5", "(0), [0], (1), [2], (2)"}}; EXPECT_EQ(run(*graph, "0", "2"), expected); } @@ -297,6 +297,6 @@ TEST_F(ShortestPathLoweringTest, handlesUnsignedWeights) { const std::unique_ptr graph = buildTriangleGraph(ValueType::UInt64, 1, 2, 5); // A -> B -> C = 3 beats the direct A -> C at 5, exercising the ui64 search. - const std::vector expected {{"3", "2, 2, 1, 0, 0"}}; + const std::vector expected {{"3", "(0), [0], (1), [2], (2)"}}; EXPECT_EQ(run(*graph, "0", "2"), expected); } diff --git a/tools/turingdb/TuringShell.cpp b/tools/turingdb/TuringShell.cpp index 10a1ca0a4d..1c6f15a429 100644 --- a/tools/turingdb/TuringShell.cpp +++ b/tools/turingdb/TuringShell.cpp @@ -523,44 +523,18 @@ std::string TuringShell::composePrompt() { : fmt::format("{}:{}@{}(detached {})> ", basePrompt, _graphName, _changeID, _hash); } -void asString(std::string& out, const db::Path& path) { - if (path.empty()) { - return; - } - - const auto reversed = path | std::views::reverse; - size_t i = 0; - for (auto val : reversed) { - if (i % 2 == 0) { - // NodeID - out += fmt::format("({})", val); - } else { - // EdgeID - out += fmt::format("-[{}]->", val); - } - ++i; - } -} - -void asString(std::string& out, const db::EntityList& list) { - if (list.empty()) { +void asString(std::string& out, const db::GraphPath& path) { + if (path.isNull()) { out += "null"; return; } - const auto& entries = list.getEntries(); - - size_t i = 0; - for (const auto& [type, val] : entries) { - if (i++ != 0) { - out += ", "; - } + const std::span nodes = path.getNodes(); + const std::span edges = path.getEdges(); - if (type == EntityType::Node) { - out += fmt::format("({})", val); - } else { - out += fmt::format("[{}]", val); - } + out += fmt::format("({})", nodes.front().getValue()); + for (size_t hop = 0; hop < edges.size(); hop++) { + out += fmt::format(", [{}], ({})", edges[hop].getValue(), nodes[hop + 1].getValue()); } } diff --git a/wasm/CMakeLists.txt b/wasm/CMakeLists.txt index 2c7255817d..679de4e773 100644 --- a/wasm/CMakeLists.txt +++ b/wasm/CMakeLists.txt @@ -47,7 +47,7 @@ if (EMSCRIPTEN) ${TURINGDB_ROOT}/common/BioAssert.cpp ${TURINGDB_ROOT}/common/FatalException.cpp ${TURINGDB_ROOT}/common/TuringException.cpp - ${TURINGDB_ROOT}/storage/EntityList.cpp) + ${TURINGDB_ROOT}/storage/GraphPath.cpp) target_include_directories(turing_wasm_decoder PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} diff --git a/wasm/DecoderModule.cpp b/wasm/DecoderModule.cpp index 614fa31c0f..f8ae65664c 100644 --- a/wasm/DecoderModule.cpp +++ b/wasm/DecoderModule.cpp @@ -2,12 +2,15 @@ #include #include #include +#include #include #include #include #include +#include "EntityType.h" +#include "GraphPath.h" #include "TuringProtoDecoder.h" #include "TuringProtoHeaders.h" #include "TuringProtoInBuf.h" @@ -83,6 +86,14 @@ size_t utf16Length(std::string_view text) { return length; } +// A path exports as the entries it runs through, each [u8 entity type][u64 id] +void appendPathEntry(ExportBuffers& out, db::EntityType type, uint64_t id) { + const char* idBytes = reinterpret_cast(&id); + + out._values.push_back(static_cast(type)); + out._values.insert(out._values.end(), idBytes, idBytes + sizeof(id)); +} + template void appendVariableWidthItem(ExportBuffers& out, const T& item) { if constexpr (isString) { @@ -90,16 +101,17 @@ void appendVariableWidthItem(ExportBuffers& out, const T& item) { } else if constexpr (std::is_same_v) { const char* bytes = reinterpret_cast(item.data()); out._values.insert(out._values.end(), bytes, bytes + item.size_bytes()); - } else if constexpr (std::is_same_v) { - const char* bytes = reinterpret_cast(item.data()); - out._values.insert(out._values.end(), bytes, bytes + item.size() * sizeof(db::EntityID)); - } else if constexpr (std::is_same_v) { - for (const db::EntityList::Entry& entry : item) { - const uint64_t id = entry._id.getValue(); - const char* idBytes = reinterpret_cast(&id); - - out._values.push_back(static_cast(entry._type)); - out._values.insert(out._values.end(), idBytes, idBytes + sizeof(id)); + } else if constexpr (std::is_same_v) { + const std::span nodes = item.getNodes(); + const std::span edges = item.getEdges(); + if (nodes.empty()) { + return; + } + + appendPathEntry(out, db::EntityType::Node, nodes.front().getValue()); + for (size_t hop = 0; hop < edges.size(); hop++) { + appendPathEntry(out, db::EntityType::Edge, edges[hop].getValue()); + appendPathEntry(out, db::EntityType::Node, nodes[hop + 1].getValue()); } } else { static_assert(sizeof(T) == 0, "No export layout for this element type"); diff --git a/wasm/TuringClient.mjs b/wasm/TuringClient.mjs index 61d9f63ddb..02c3b0e02a 100644 --- a/wasm/TuringClient.mjs +++ b/wasm/TuringClient.mjs @@ -38,9 +38,9 @@ export const ColumnType = Object.freeze({ DOUBLE: 2, STRING: 3, BOOL: 4, - PATH: 5, + RETIRED_PATH: 5, EMBEDDING: 6, - ENTITY_LIST: 7, + RETIRED_ENTITY_LIST: 7, LIST_VIEW: 8, VALUE_TYPE: 9, LIST_ELEMENT_VIEW: 10, @@ -57,6 +57,7 @@ export const ColumnType = Object.freeze({ MAP_ENTRY_VIEW: 21, DATE_TIME: 22, DURATION: 23, + GRAPH_PATH: 24, }); // The column types whose values the nested reader walks out of the flat nested bytes @@ -345,8 +346,8 @@ export class NestedReader { } } -// Entity list entries are [u8 type][u64 id], 9 bytes each. -function readEntityList(bytes, view, start, end) { +// A path's entries are [u8 type][u64 id], 9 bytes each, in the order the path runs. +function readGraphPath(bytes, view, start, end) { const entries = []; for (let offset = start; offset < end; offset += 9) { entries.push({ type: bytes[offset], id: view.getBigUint64(offset + 1, true) }); @@ -378,11 +379,9 @@ function makeItemReader(buffers, nestedReader) { return variable(makeStringReader(buffers.values, offsets, new Uint32Array(buffers.utf16Offsets.buffer))); case ColumnType.EMBEDDING: return variable((index) => new Float32Array(buffers.values.buffer, offsets[index], (offsets[index + 1] - offsets[index]) / 4)); - case ColumnType.PATH: - return variable((index) => Array.from(new BigUint64Array(buffers.values.buffer, offsets[index], (offsets[index + 1] - offsets[index]) / 8))); - case ColumnType.ENTITY_LIST: { + case ColumnType.GRAPH_PATH: { const view = new DataView(buffers.values.buffer); - return variable((index) => readEntityList(buffers.values, view, offsets[index], offsets[index + 1])); + return variable((index) => readGraphPath(buffers.values, view, offsets[index], offsets[index + 1])); } case ColumnType.LIST_VIEW: return variable((index) => nestedReader.readListAt(offsets[index])); @@ -397,7 +396,7 @@ function makeItemReader(buffers, nestedReader) { } } -// Only a nested column's plain objects are maps: an entity list's entries are plain objects +// Only a nested column's plain objects are maps: a path's entries are plain objects // too, and keep their BigInt ids. function bigIntToNumberDeep(value, convertsMaps) { if (typeof value === "bigint") { diff --git a/wasm/WasmSink.h b/wasm/WasmSink.h index 225e71966d..023bb472f5 100644 --- a/wasm/WasmSink.h +++ b/wasm/WasmSink.h @@ -55,10 +55,8 @@ consteval ColumnType wireCodeOfElement() { return ColumnType::DATE_TIME; } else if constexpr (std::is_same_v) { return ColumnType::DURATION; - } else if constexpr (std::is_same_v) { - return ColumnType::PATH; - } else if constexpr (std::is_same_v) { - return ColumnType::ENTITY_LIST; + } else if constexpr (std::is_same_v) { + return ColumnType::GRAPH_PATH; } else if constexpr (std::is_same_v) { return ColumnType::LIST_VIEW; } else if constexpr (std::is_same_v) { diff --git a/wasm/client_smoke.mjs b/wasm/client_smoke.mjs index 8a18df7317..26b6942bf5 100644 --- a/wasm/client_smoke.mjs +++ b/wasm/client_smoke.mjs @@ -20,11 +20,11 @@ const MESSAGE_ERROR = 4; const TYPE_UINT64 = 0; const TYPE_DOUBLE = 2; const TYPE_STRING = 3; -const TYPE_ENTITY_LIST = 7; const TYPE_LIST_VIEW = 8; const TYPE_LIST_ELEMENT_VIEW = 10; const TYPE_NODE_ID = 11; const TYPE_MAP_VIEW = 20; +const TYPE_GRAPH_PATH = 24; const ENCODING_VECTOR = 0; const ENCODING_OPTIONAL_VECTOR = 1; @@ -538,21 +538,26 @@ async function testRaggedQueryData(module) { ]]); } -// An entity list's entries are plain objects, but not maps: queryData keeps their ids BigInt, -// so an id above 2^53 survives exactly. -async function testEntityListIdsStayExact(module) { - const header = makeHeader([{ name: "path", typeCode: TYPE_ENTITY_LIST, encoding: ENCODING_VECTOR }]); +// A path's entries are plain objects, but not maps: queryData keeps their ids BigInt, so an +// id above 2^53 survives exactly. +async function testGraphPathIdsStayExact(module) { + const header = makeHeader([{ name: "path", typeCode: TYPE_GRAPH_PATH, encoding: ENCODING_VECTOR }]); const chunk = new Writer(); chunk.u32(1); - chunk.u32(1); - chunk.u8(0); + chunk.u32(2); chunk.u64(9007199254740993n); + chunk.u64(5n); + chunk.u64(9007199254740995n); const bytes = concatPackets([frame(MESSAGE_CHUNK_HEADER, header), frame(MESSAGE_CHUNK, chunk), makeEndChunk(1), makeEnd(1.0)]); const data = await new TuringClient(module, { fetch: makeFakeFetch(bytes, {}) }).queryData("RETURN 1"); - assert.deepStrictEqual(data, [[[[{ type: 0, id: 9007199254740993n }]]]]); + assert.deepStrictEqual(data, [[[[ + { type: 0, id: 9007199254740993n }, + { type: 1, id: 9007199254740995n }, + { type: 0, id: 5n }, + ]]]]); } createTuringDecoderModule().then(async (module) => { @@ -562,6 +567,6 @@ createTuringDecoderModule().then(async (module) => { await testMaps(module, false); await testMaps(module, true); await testRaggedQueryData(module); - await testEntityListIdsStayExact(module); - console.log("wasm client smoke test passed: query, queryData, error, maps, ragged, entity lists"); + await testGraphPathIdsStayExact(module); + console.log("wasm client smoke test passed: query, queryData, error, maps, ragged, paths"); }); From 8a9e28a01f633a31c6e59358cb2b040f68f7476b Mon Sep 17 00:00:00 2001 From: Remy Boutonnet Date: Fri, 23 Oct 2026 14:27:26 +0200 Subject: [PATCH 2/2] Proto: drop the retired path kinds --- net/decoders/TuringProtoDecoder.h | 3 -- net/turing_proto_common/TuringProtoHeaders.h | 2 -- wasm/TuringClient.mjs | 38 ++++++++++---------- wasm/client_smoke.mjs | 10 +++--- wasm/decoder_smoke.js | 2 +- 5 files changed, 24 insertions(+), 31 deletions(-) diff --git a/net/decoders/TuringProtoDecoder.h b/net/decoders/TuringProtoDecoder.h index c0022f3a6e..57a2734f39 100644 --- a/net/decoders/TuringProtoDecoder.h +++ b/net/decoders/TuringProtoDecoder.h @@ -124,9 +124,6 @@ decltype(auto) TuringProtoDecoder::dispatchColumnType(ColumnInternalKind t return fn.template operator()(encoding); case ColumnInternalKind::GRAPH_PATH: return fn.template operator()(encoding); - case ColumnInternalKind::RETIRED_PATH: - case ColumnInternalKind::RETIRED_ENTITY_LIST: - break; } throw TuringException("Unsupported incoming column type"); diff --git a/net/turing_proto_common/TuringProtoHeaders.h b/net/turing_proto_common/TuringProtoHeaders.h index 02d6df7c06..cfe708d34e 100644 --- a/net/turing_proto_common/TuringProtoHeaders.h +++ b/net/turing_proto_common/TuringProtoHeaders.h @@ -59,9 +59,7 @@ enum class ColumnInternalKind : uint32_t { DOUBLE, STRING, BOOL, - RETIRED_PATH, EMBEDDING, - RETIRED_ENTITY_LIST, LIST_VIEW, VALUE_TYPE, LIST_ELEMENT_VIEW, diff --git a/wasm/TuringClient.mjs b/wasm/TuringClient.mjs index 02c3b0e02a..29a63fc692 100644 --- a/wasm/TuringClient.mjs +++ b/wasm/TuringClient.mjs @@ -38,26 +38,24 @@ export const ColumnType = Object.freeze({ DOUBLE: 2, STRING: 3, BOOL: 4, - RETIRED_PATH: 5, - EMBEDDING: 6, - RETIRED_ENTITY_LIST: 7, - LIST_VIEW: 8, - VALUE_TYPE: 9, - LIST_ELEMENT_VIEW: 10, - NODE_ID: 11, - EDGE_ID: 12, - EDGE_TYPE_ID: 13, - PROPERTY_TYPE_ID: 14, - LABEL_ID: 15, - LABEL_SET_ID: 16, - COMMIT_HASH: 17, - CHANGE_ID: 18, - PROPERTY_NULL: 19, - MAP_VIEW: 20, - MAP_ENTRY_VIEW: 21, - DATE_TIME: 22, - DURATION: 23, - GRAPH_PATH: 24, + EMBEDDING: 5, + LIST_VIEW: 6, + VALUE_TYPE: 7, + LIST_ELEMENT_VIEW: 8, + NODE_ID: 9, + EDGE_ID: 10, + EDGE_TYPE_ID: 11, + PROPERTY_TYPE_ID: 12, + LABEL_ID: 13, + LABEL_SET_ID: 14, + COMMIT_HASH: 15, + CHANGE_ID: 16, + PROPERTY_NULL: 17, + MAP_VIEW: 18, + MAP_ENTRY_VIEW: 19, + DATE_TIME: 20, + DURATION: 21, + GRAPH_PATH: 22, }); // The column types whose values the nested reader walks out of the flat nested bytes diff --git a/wasm/client_smoke.mjs b/wasm/client_smoke.mjs index 26b6942bf5..b12eac8a7b 100644 --- a/wasm/client_smoke.mjs +++ b/wasm/client_smoke.mjs @@ -20,11 +20,11 @@ const MESSAGE_ERROR = 4; const TYPE_UINT64 = 0; const TYPE_DOUBLE = 2; const TYPE_STRING = 3; -const TYPE_LIST_VIEW = 8; -const TYPE_LIST_ELEMENT_VIEW = 10; -const TYPE_NODE_ID = 11; -const TYPE_MAP_VIEW = 20; -const TYPE_GRAPH_PATH = 24; +const TYPE_LIST_VIEW = 6; +const TYPE_LIST_ELEMENT_VIEW = 8; +const TYPE_NODE_ID = 9; +const TYPE_MAP_VIEW = 18; +const TYPE_GRAPH_PATH = 22; const ENCODING_VECTOR = 0; const ENCODING_OPTIONAL_VECTOR = 1; diff --git a/wasm/decoder_smoke.js b/wasm/decoder_smoke.js index f77ead2a36..b34dbb2a4b 100644 --- a/wasm/decoder_smoke.js +++ b/wasm/decoder_smoke.js @@ -12,7 +12,7 @@ const MESSAGE_END_CHUNK = 2; const TYPE_UINT64 = 0; const TYPE_DOUBLE = 2; const TYPE_STRING = 3; -const TYPE_NODE_ID = 11; +const TYPE_NODE_ID = 9; const ENCODING_VECTOR = 0; const ENCODING_OPTIONAL_VECTOR = 1;