Skip to content

feat(kv-ir): Add StringBlob for compact storage of multiple strings in a contiguous byte buffer. #1544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LinZhihao-723 merged 8 commits into from
Nov 4, 2025
Merged

feat(kv-ir): Add StringBlob for compact storage of multiple strings in a contiguous byte buffer. #1544

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6fe8d8b
Implement StringBlob.
LinZhihao-723 Nov 3, 2025
b41bca7
Merge branch 'main' into string-blob
LinZhihao-723 Nov 3, 2025
08c2765
clang-format...
LinZhihao-723 Nov 3, 2025
cee9ad7
Merge branch 'string-blob' of https://github.com/LinZhihao-723/clp in…
LinZhihao-723 Nov 3, 2025
a358948
Brokennnnn
LinZhihao-723 Nov 3, 2025
47ffdd8
Update components/core/src/clp/ffi/StringBlob.hpp
LinZhihao-723 Nov 4, 2025
fbec677
Merge branch 'main' into string-blob
LinZhihao-723 Nov 4, 2025
3132f0f
Merge branch 'main' into string-blob
LinZhihao-723 Nov 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions components/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,7 @@ set(SOURCE_FILES_unitTest
src/clp/ffi/encoding_methods.cpp
src/clp/ffi/encoding_methods.hpp
src/clp/ffi/encoding_methods.inc
src/clp/ffi/test/test_StringBlob.cpp
src/clp/ffi/ir_stream/byteswap.hpp
src/clp/ffi/ir_stream/Deserializer.hpp
src/clp/ffi/ir_stream/decoding_methods.cpp
Expand Down Expand Up @@ -535,6 +536,7 @@ set(SOURCE_FILES_unitTest
src/clp/ffi/search/Subquery.hpp
src/clp/ffi/search/WildcardToken.cpp
src/clp/ffi/search/WildcardToken.hpp
src/clp/ffi/StringBlob.hpp
src/clp/ffi/utils.cpp
src/clp/ffi/utils.hpp
src/clp/ffi/Value.hpp
Expand Down
66 changes: 66 additions & 0 deletions components/core/src/clp/ffi/StringBlob.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
#ifndef CLP_FFI_STRINGBLOB_HPP
#define CLP_FFI_STRINGBLOB_HPP

#include <cstddef>
#include <optional>
#include <string>
#include <string_view>
#include <vector>

#include "../ErrorCode.hpp"
#include "../ReaderInterface.hpp"

namespace clp::ffi {
// Stores a list of strings as an indexable blob.
class StringBlob {
public:
// Constructors
StringBlob() = default;

// Methods
[[nodiscard]] auto get_num_strings() const -> size_t { return m_offsets.size() - 1; }

Comment on lines +1 to +22
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Well-structured header with safe initialization.

The header guards, includes, and class structure are clean and appropriate. The initialization of m_offsets{0} on line 62 ensures that get_num_strings() safely returns 0 for an empty blob without underflow risk.

Consider adding noexcept to get_num_strings() since it cannot throw:

-    [[nodiscard]] auto get_num_strings() const -> size_t { return m_offsets.size() - 1; }
+    [[nodiscard]] auto get_num_strings() const noexcept -> size_t { return m_offsets.size() - 1; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#ifndef CLP_FFI_STRINGBLOB_HPP
#define CLP_FFI_STRINGBLOB_HPP
#include <cstddef>
#include <optional>
#include <string>
#include <string_view>
#include <vector>
#include "../ErrorCode.hpp"
#include "../ReaderInterface.hpp"
namespace clp::ffi {
// Stores a list of strings as an indexable blob.
class StringBlob {
public:
// Constructors
StringBlob() = default;
// Methods
[[nodiscard]] auto get_num_strings() const -> size_t { return m_offsets.size() - 1; }
#ifndef CLP_FFI_STRINGBLOB_HPP
#define CLP_FFI_STRINGBLOB_HPP
#include <cstddef>
#include <optional>
#include <string>
#include <string_view>
#include <vector>
#include "../ErrorCode.hpp"
#include "../ReaderInterface.hpp"
namespace clp::ffi {
// Stores a list of strings as an indexable blob.
class StringBlob {
public:
// Constructors
StringBlob() = default;
// Methods
[[nodiscard]] auto get_num_strings() const noexcept -> size_t { return m_offsets.size() - 1; }
🤖 Prompt for AI Agents 
In components/core/src/clp/ffi/StringBlob.hpp around lines 1 to 22, the getter
get_num_strings() is non-throwing and should be marked noexcept; update its
declaration/definition to add noexcept (e.g., auto get_num_strings() const
noexcept -> size_t) so the compiler and callers know it won’t throw and to
enable better optimization and exception-safety guarantees.

/**
* @param index
* @return A view of the string at the given `index` in the blob.
* @return std::nullopt if `index` is out of bounds.
*/
[[nodiscard]] auto get_string(size_t index) const -> std::optional<std::string_view> {
if (index >= get_num_strings()) {
return std::nullopt;
}
size_t const start_offset{m_offsets[index]};
size_t const end_offset{m_offsets[index + 1]};
return std::string_view{m_data}.substr(start_offset, end_offset - start_offset);
}
Comment on lines +23 to +35
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Bounds checking is correct; consider noexcept.

The bounds validation correctly ensures safe access to m_offsets[index + 1]. The method could be marked noexcept since it returns std::nullopt on invalid input rather than throwing.

-    [[nodiscard]] auto get_string(size_t index) const -> std::optional<std::string_view> {
+    [[nodiscard]] auto get_string(size_t index) const noexcept -> std::optional<std::string_view> {
🤖 Prompt for AI Agents 
components/core/src/clp/ffi/StringBlob.hpp around lines 23-35: the method
get_string performs bounds checks and never throws; mark it noexcept to document
and enable optimizations — update the function signature to append noexcept
(both declaration and definition if separated) while leaving behavior unchanged
(return std::nullopt on out-of-bounds and return the std::string_view
otherwise).


/**
* Reads a string of the given `length` from the `reader` and appends it to the blob.
* @param reader
* @param length The exact length of the string to read.
* @return std::nullopt on success.
* @return Forwards `ReaderInterface::try_read_exact_length`'s error code on failure.
*/
[[nodiscard]] auto read_from(ReaderInterface& reader, size_t length)
-> std::optional<ErrorCode> {
auto const start_offset{m_data.size()};
auto const end_offset{start_offset + length};
m_data.resize(static_cast<std::string::size_type>(end_offset));
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
if (auto const err{reader.try_read_exact_length(m_data.data() + start_offset, length)};
ErrorCode::ErrorCode_Success != err)
{
m_data.resize(start_offset);
return err;
}
m_offsets.emplace_back(end_offset);
return std::nullopt;
}
Comment on lines +37 to +58
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Excellent error handling with proper rollback; minor cast redundancy.

The read_from implementation correctly handles errors by rolling back m_data to its previous size before returning the error code. The logic properly maintains the invariant that m_offsets.size() - 1 equals the number of successfully stored strings.

Line 48 contains a redundant cast since end_offset is already size_t and std::string::size_type is typically size_t:

-        m_data.resize(static_cast<std::string::size_type>(end_offset));
+        m_data.resize(end_offset);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Reads a string of the given `length` from the `reader` and appends it to the blob.
* @param reader
* @param length The exact length of the string to read.
* @return std::nullopt on success.
* @return Forwards `ReaderInterface::try_read_exact_length`'s error code on failure.
*/
[[nodiscard]] auto read_from(ReaderInterface& reader, size_t length)
-> std::optional<ErrorCode> {
auto const start_offset{m_data.size()};
auto const end_offset{start_offset + length};
m_data.resize(static_cast<std::string::size_type>(end_offset));
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
if (auto const err{reader.try_read_exact_length(m_data.data() + start_offset, length)};
ErrorCode::ErrorCode_Success != err)
{
m_data.resize(start_offset);
return err;
}
m_offsets.emplace_back(end_offset);
return std::nullopt;
}
/**
* Reads a string of the given `length` from the `reader` and appends it to the blob.
* @param reader
* @param length The exact length of the string to read.
* @return std::nullopt on success.
* @return Forwards `ReaderInterface::try_read_exact_length`'s error code on failure.
*/
[[nodiscard]] auto read_from(ReaderInterface& reader, size_t length)
-> std::optional<ErrorCode> {
auto const start_offset{m_data.size()};
auto const end_offset{start_offset + length};
m_data.resize(end_offset);
// NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
if (auto const err{reader.try_read_exact_length(m_data.data() + start_offset, length)};
ErrorCode::ErrorCode_Success != err)
{
m_data.resize(start_offset);
return err;
}
m_offsets.emplace_back(end_offset);
return std::nullopt;
}
🤖 Prompt for AI Agents 
In components/core/src/clp/ffi/StringBlob.hpp around lines 37 to 58, the call to
m_data.resize uses a redundant static_cast to std::string::size_type for
end_offset (which is already size_t); remove the unnecessary cast and call
m_data.resize(end_offset) directly to simplify the code and avoid redundant
conversion.


private:
std::string m_data;
std::vector<size_t> m_offsets{0};
};
} // namespace clp::ffi

#endif // CLP_FFI_STRINGBLOB_HPP
45 changes: 45 additions & 0 deletions components/core/src/clp/ffi/test/test_StringBlob.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#include <cstddef>
#include <string>
#include <string_view>
#include <vector>

#include <catch2/catch_test_macros.hpp>

#include "../../BufferReader.hpp"
#include "../../ErrorCode.hpp"
#include "../StringBlob.hpp"

TEST_CASE("StringBlob basic functionality", "[StringBlob]") {
clp::ffi::StringBlob string_blob;

std::vector<std::string> const test_strings{
"Hello, World!",
"This is a test string.",
"StringBlob is working correctly.",
};

std::string buffer;
for (auto const& str : test_strings) {
buffer += str;
}
clp::BufferReader reader{buffer.data(), buffer.size()};

size_t expected_num_strings{0};
for (auto const& expected_str : test_strings) {
REQUIRE((expected_num_strings == string_blob.get_num_strings()));

auto const result{string_blob.read_from(reader, expected_str.size())};
REQUIRE_FALSE(result.has_value());
++expected_num_strings;
REQUIRE((expected_num_strings == string_blob.get_num_strings()));
auto const optional_retrieved_str{string_blob.get_string(expected_num_strings - 1)};
REQUIRE((optional_retrieved_str.has_value()));
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
REQUIRE((optional_retrieved_str.value() == expected_str));
}

auto const read_from_eof{string_blob.read_from(reader, 1)};
REQUIRE(read_from_eof.has_value());
// NOLINTNEXTLINE(bugprone-unchecked-optional-access)
REQUIRE((clp::ErrorCode::ErrorCode_EndOfFile == read_from_eof.value()));
}
Loading