Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
7b203a6
Add iterative evaluation method
LinZhihao-723 May 6, 2025
e74e31f
Fix null query issue.
LinZhihao-723 May 6, 2025
e3636fe
Implement search.
LinZhihao-723 May 6, 2025
10008b5
Ops, forget to commit this one.
LinZhihao-723 May 7, 2025
00e2ecc
Fix clang-tidy warnings on the tes files.
LinZhihao-723 May 7, 2025
ca87b29
Ignore unsupported literal types.
LinZhihao-723 May 7, 2025
8f51308
WIP
LinZhihao-723 May 7, 2025
e7a578b
Working on the unit tests...
LinZhihao-723 May 7, 2025
6791912
Add basic
LinZhihao-723 May 7, 2025
7ea6075
Merge branch 'oss_main' into kvir-search-query-handler-evaluation
LinZhihao-723 May 7, 2025
dd35aa2
Finish test dev
LinZhihao-723 May 8, 2025
45320b1
WIP deserializer integration
LinZhihao-723 May 8, 2025
bfe5873
Add inverter to the test
LinZhihao-723 May 8, 2025
3ad1f84
Merge branch 'kvir-search-query-handler-evaluation' into deserializer…
LinZhihao-723 May 8, 2025
5d4e9e3
Integration with doc string updates.
LinZhihao-723 May 8, 2025
f32d575
Fix
LinZhihao-723 May 9, 2025
3985819
Fix cmake.
LinZhihao-723 May 9, 2025
85a5798
Add wrappers for factory.
LinZhihao-723 May 9, 2025
f4a627f
Apply suggestions from code review
LinZhihao-723 May 11, 2025
9f8dc3a
Apply code review comments
LinZhihao-723 May 11, 2025
9ee374c
Add unit tests for deserialization integration.
LinZhihao-723 May 12, 2025
939f0ab
Apply suggestions from code review
LinZhihao-723 May 12, 2025
4af0b4a
Apply suggestions from code review
LinZhihao-723 May 12, 2025
e6e32e2
Apply code review comments
LinZhihao-723 May 12, 2025
27b69c5
Merge #882.
LinZhihao-723 May 14, 2025
2cc5050
Merge branch 'oss-main' into deserializer-integration
LinZhihao-723 May 15, 2025
c4add7b
Add line filter
LinZhihao-723 May 15, 2025
c37c224
Fix...
LinZhihao-723 May 15, 2025
e4aec2e
Update taskfiles/lint.yaml
LinZhihao-723 May 16, 2025
f66a2ca
Update taskfiles/lint.yaml
LinZhihao-723 May 16, 2025
d6b3774
Make line filter as variables
LinZhihao-723 May 16, 2025
97516f4
Move comment to the top
LinZhihao-723 May 16, 2025
eec952b
Merge branch 'main' into deserializer-integration
LinZhihao-723 May 16, 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
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ set(SOURCE_FILES_unitTest
src/clp/ffi/ir_stream/search/QueryHandler.hpp
src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp
src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp
src/clp/ffi/ir_stream/search/QueryHandlerReq.hpp
src/clp/ffi/ir_stream/search/test/test_deserializer_integration.cpp
src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp
src/clp/ffi/ir_stream/search/test/test_utils.cpp
src/clp/ffi/ir_stream/search/test/utils.cpp
Expand Down
184 changes: 158 additions & 26 deletions components/core/src/clp/ffi/ir_stream/Deserializer.hpp

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@coderabbitai review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No description provided.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <memory>
#include <string>
#include <system_error>
#include <tuple>
#include <utility>
#include <vector>

#include <nlohmann/json.hpp>
Expand All @@ -20,37 +20,61 @@
#include "IrUnitHandlerInterface.hpp"
#include "IrUnitType.hpp"
#include "protocol_constants.hpp"
#include "search/QueryHandlerReq.hpp"
#include "utils.hpp"

namespace clp::ffi::ir_stream {
/**
* A deserializer for reading IR units from a CLP kv-pair IR stream. An IR unit handler should be
* provided to perform user-defined operations on each deserialized IR unit.
* provided to perform user-defined operations on each deserialized IR unit. Additionally, a query
* handler can be provided to handle queries and column projections.
*
* NOTE: This class is designed only to provide deserialization functionalities. Callers are
* responsible for maintaining a `ReaderInterface` to input IR bytes from an I/O stream.
*
* @tparam IrUnitHandler
* @tparam QueryHandlerType
*/
template <IrUnitHandlerInterface IrUnitHandler>
template <
IrUnitHandlerInterface IrUnitHandler,
search::QueryHandlerReq QueryHandlerType = search::EmptyQueryHandler>
requires(std::move_constructible<IrUnitHandler>)
class Deserializer {
public:
// Factory function
/**
* Creates a deserializer by reading the stream's preamble from the given reader.
* Creates a deserializer with an empty query handler (for use when the deserializer won't be
* used to perform queries or column projections).
* @param reader
* @param ir_unit_handler
* @return A result containing the deserializer or an error code indicating the failure:
* - std::errc::result_out_of_range if the IR stream is truncated
* - std::errc::protocol_error if the IR stream is corrupted
* - std::errc::protocol_not_supported if either:
* - the IR stream contains an unsupported metadata format;
* - the IR stream's version is unsupported;
* - or the IR stream's user-defined metadata is not a JSON object.
* @return A result containing the deserializer on success, or an error code indicating the
* failure:
* - Forwards `create_generic`'s return values.
*/
[[nodiscard]] static auto create(ReaderInterface& reader, IrUnitHandler ir_unit_handler)
-> OUTCOME_V2_NAMESPACE::std_result<Deserializer>;
-> OUTCOME_V2_NAMESPACE::std_result<Deserializer>
requires std::is_same_v<QueryHandlerType, search::EmptyQueryHandler>
{
return create_generic(reader, std::move(ir_unit_handler), {});
}

/**
* Creates a deserializer with a query handler (for use when the deserializer will be used to
* perform queries or column projections).
* @param reader
* @param ir_unit_handler
* @param query_handler
* @return A result containing the deserializer on success, or an error code indicating the
* failure:
* - Forwards `create_generic`'s return values.
*/
[[nodiscard]] static auto
create(ReaderInterface& reader, IrUnitHandler ir_unit_handler, QueryHandlerType query_handler)
-> OUTCOME_V2_NAMESPACE::std_result<Deserializer>
requires search::IsNonEmptyQueryHandler<QueryHandlerType>::value
{
return create_generic(reader, std::move(ir_unit_handler), std::move(query_handler));
}

// Delete copy constructor and assignment
Deserializer(Deserializer const&) = delete;
Expand All @@ -65,34 +89,45 @@ class Deserializer {

// Methods
/**
* Deserializes the stream from the given reader up to and including the next log event IR unit.
* Deserializes the stream from the given reader up to and including the next log event IR unit,
* and invokes the user-defined IR unit handler according to the deserialized IR unit type.
*
* NOTE: If the deserialized IR unit is `IrUnitType::LogEvent` and the query handler is not
* `search::EmptyQueryHandler`, `handle_log_event` will only be invoked if the query handler

* returns `search::AstEvaluationResult::True`.
*
* @param reader
* @return Forwards `deserialize_tag`s return values if no tag bytes can be read to determine
* the next IR unit type.
* @return std::errc::protocol_not_supported if the IR unit type is not supported.
* @return std::errc::operation_not_permitted if the deserializer already reached the end of
* stream by deserializing an end-of-stream IR unit in the previous calls.
* @return IRUnitType::LogEvent if a log event IR unit is deserialized, or an error code
* @return IrUnitType::LogEvent if a log event IR unit is deserialized, or an error code
* indicating the failure:
* - Forwards `deserialize_ir_unit_kv_pair_log_event`'s return values if it failed to
* deserialize and construct the log event.
* - Forwards `handle_log_event`'s return values from the user-defined IR unit handler on
* unit handling failure.
* @return IRUnitType::SchemaTreeNodeInsertion if a schema tree node insertion IR unit is
* - Forwards `search::QueryHandler::evaluate_kv_pair_log_event`'s return values on failure, if
* `QueryHandlerType` is not `search::EmptyQueryHandler`.
* @return IrUnitType::SchemaTreeNodeInsertion if a schema tree node insertion IR unit is
* deserialized, or an error code indicating the failure:
* - Forwards `deserialize_ir_unit_schema_tree_node_insertion`'s return values if it failed to
* deserialize and construct the schema tree node locator.
* - Forwards `handle_schema_tree_node_insertion`'s return values from the user-defined IR unit
* handler on unit handling failure.
* - Forwards `search::QueryHandler::update_partially_resolved_columns`'s return values on
* failure, if `QueryHandlerType` is not `search::EmptyQueryHandler`.
* - std::errc::protocol_error if the deserialized schema tree node already exists in the schema
* tree.
* @return IRUnitType::UtcOffsetChange if a UTC offset change IR unit is deserialized, or an
* @return IrUnitType::UtcOffsetChange if a UTC offset change IR unit is deserialized, or an
* error code indicating the failure:
* - Forwards `deserialize_ir_unit_utc_offset_change`'s return values if it failed to
* deserialize the UTC offset.
* - Forwards `handle_utc_offset_change`'s return values from the user-defined IR unit handler
* on unit handling failure.
* @return IRUnitType::EndOfStream if an end-of-stream IR unit is deserialized, or an error code
* @return IrUnitType::EndOfStream if an end-of-stream IR unit is deserialized, or an error code
* indicating the failure:
* - Forwards `handle_end_of_stream`'s return values from the user-defined IR unit handler on
* unit handling failure.
Expand All @@ -118,10 +153,35 @@ class Deserializer {
[[nodiscard]] auto get_metadata() const -> nlohmann::json const& { return m_metadata; }

private:
// Factory function
/**
* Creates a deserializer by reading the stream's preamble from the given reader.
* @param reader
* @param ir_unit_handler
* @param query_handler
* @return A result containing the deserializer or an error code indicating the failure:
* - std::errc::result_out_of_range if the IR stream is truncated
* - std::errc::protocol_error if the IR stream is corrupted
* - std::errc::protocol_not_supported if either:
* - the IR stream contains an unsupported metadata format;
* - the IR stream's version is unsupported;
* - or the IR stream's user-defined metadata is not a JSON object.
*/
[[nodiscard]] static auto create_generic(
ReaderInterface& reader,
IrUnitHandler ir_unit_handler,
QueryHandlerType query_handler
Comment thread
LinZhihao-723 marked this conversation as resolved.
) -> OUTCOME_V2_NAMESPACE::std_result<Deserializer>;

// Constructor
Deserializer(IrUnitHandler ir_unit_handler, nlohmann::json metadata)
Deserializer(
IrUnitHandler ir_unit_handler,
nlohmann::json metadata,
QueryHandlerType query_handler
)
: m_ir_unit_handler{std::move(ir_unit_handler)},
m_metadata(std::move(metadata)) {}
m_metadata(std::move(metadata)),
m_query_handler{std::move(query_handler)} {}

// Variables
std::shared_ptr<SchemaTree> m_auto_gen_keys_schema_tree{std::make_shared<SchemaTree>()};
Expand All @@ -130,12 +190,16 @@ class Deserializer {
UtcOffset m_utc_offset{0};
IrUnitHandler m_ir_unit_handler;
bool m_is_complete{false};
[[no_unique_address]] QueryHandlerType m_query_handler;
};

template <IrUnitHandlerInterface IrUnitHandler>
template <IrUnitHandlerInterface IrUnitHandler, search::QueryHandlerReq QueryHandlerType>
requires(std::move_constructible<IrUnitHandler>)
auto Deserializer<IrUnitHandler>::create(ReaderInterface& reader, IrUnitHandler ir_unit_handler)
-> OUTCOME_V2_NAMESPACE::std_result<Deserializer> {
auto Deserializer<IrUnitHandler, QueryHandlerType>::create_generic(
ReaderInterface& reader,
IrUnitHandler ir_unit_handler,
QueryHandlerType query_handler
) -> OUTCOME_V2_NAMESPACE::std_result<Deserializer> {
bool is_four_byte_encoded{};
if (auto const err{get_encoding_type(reader, is_four_byte_encoded)};
Comment on lines +196 to 204

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

SFINAE overlap risk between the two create overloads

Both factory overloads participate in overload resolution when QueryHandlerType
is EmptyQueryHandler and the caller supplies three arguments accidentally
(e.g., by std::move(empty_handler)).

Although currently well-ordered by arity, adding a defaulted parameter to the
second overload would create ambiguity. Guard against this by disabling the
third-parameter overload whenever the passed handler is EmptyQueryHandler:

requires(!std::is_same_v<QueryHandlerType, search::EmptyQueryHandler>)

(This mirrors the requires clause already present on the 2-parameter
overload).

IRErrorCode::IRErrorCode_Success != err)
Expand Down Expand Up @@ -176,13 +240,44 @@ auto Deserializer<IrUnitHandler>::create(ReaderInterface& reader, IrUnitHandler
return std::errc::protocol_not_supported;
}

return Deserializer{std::move(ir_unit_handler), std::move(metadata_json)};
return Deserializer{
std::move(ir_unit_handler),
std::move(metadata_json),
std::move(query_handler)
};
}

/**
* Wrapper for `Deserializer`'s factory function to enable automatic type deduction.
* @param reader
* @param ir_unit_handler
* @return Forwards `Deserializer::create`'s return values.
*/
template <IrUnitHandlerInterface IrUnitHandler>
requires std::is_move_constructible_v<IrUnitHandler>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we add this to IrUnitHandlerInterface?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd prefer to fix this in another PR. This concept was added before we have the new concept naming rule, so we should also rename it to IrUnitHandlerInterfaceReq.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you file an issue?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

[[nodiscard]] auto make_deserializer(ReaderInterface& reader, IrUnitHandler ir_unit_handler)
-> OUTCOME_V2_NAMESPACE::std_result<Deserializer<IrUnitHandler>>;

/**
* Wrapper for `Deserializer`'s factory function to enable automatic type deduction.
* @param reader
* @param ir_unit_handler
* @param query_handler
* @return Forwards `Deserializer::create`'s return values.
*/
template <IrUnitHandlerInterface IrUnitHandler, search::QueryHandlerReq QueryHandlerType>
requires std::move_constructible<IrUnitHandler>
[[nodiscard]] auto make_deserializer(
ReaderInterface& reader,
IrUnitHandler ir_unit_handler,
QueryHandlerType query_handler
) -> OUTCOME_V2_NAMESPACE::std_result<Deserializer<IrUnitHandler, QueryHandlerType>>;

template <IrUnitHandlerInterface IrUnitHandler, search::QueryHandlerReq QueryHandlerType>
requires(std::move_constructible<IrUnitHandler>)
auto Deserializer<IrUnitHandler>::deserialize_next_ir_unit(ReaderInterface& reader)
-> OUTCOME_V2_NAMESPACE::std_result<IrUnitType> {
auto Deserializer<IrUnitHandler, QueryHandlerType>::deserialize_next_ir_unit(
ReaderInterface& reader
) -> OUTCOME_V2_NAMESPACE::std_result<IrUnitType> {
if (is_stream_completed()) {
return std::errc::operation_not_permitted;
}
Expand Down Expand Up @@ -211,6 +306,14 @@ auto Deserializer<IrUnitHandler>::deserialize_next_ir_unit(ReaderInterface& read
return result.error();
}

if constexpr (search::IsNonEmptyQueryHandler<QueryHandlerType>::value) {
if (search::AstEvaluationResult::True
!= OUTCOME_TRYX(m_query_handler.evaluate_kv_pair_log_event(result.value())))
{
break;
}
}

if (auto const err{m_ir_unit_handler.handle_log_event(std::move(result.value()))};
IRErrorCode::IRErrorCode_Success != err)
{
Expand All @@ -237,7 +340,15 @@ auto Deserializer<IrUnitHandler>::deserialize_next_ir_unit(ReaderInterface& read
return std::errc::protocol_error;
}

std::ignore = schema_tree_to_insert->insert_node(node_locator);
auto const node_id{schema_tree_to_insert->insert_node(node_locator)};

if constexpr (search::IsNonEmptyQueryHandler<QueryHandlerType>::value) {
OUTCOME_TRYV(m_query_handler.update_partially_resolved_columns(
is_auto_generated,
node_locator,
node_id
));
}

if (auto const err{m_ir_unit_handler.handle_schema_tree_node_insertion(
is_auto_generated,
Expand Down Expand Up @@ -286,6 +397,27 @@ auto Deserializer<IrUnitHandler>::deserialize_next_ir_unit(ReaderInterface& read

return ir_unit_type;
}

template <IrUnitHandlerInterface IrUnitHandler>
requires std::is_move_constructible_v<IrUnitHandler>
[[nodiscard]] auto make_deserializer(ReaderInterface& reader, IrUnitHandler ir_unit_handler)
-> OUTCOME_V2_NAMESPACE::std_result<Deserializer<IrUnitHandler>> {
return Deserializer<IrUnitHandler>::create(reader, std::move(ir_unit_handler));
}

template <IrUnitHandlerInterface IrUnitHandler, search::QueryHandlerReq QueryHandlerType>
requires std::move_constructible<IrUnitHandler>
[[nodiscard]] auto make_deserializer(
ReaderInterface& reader,
IrUnitHandler ir_unit_handler,
QueryHandlerType query_handler
) -> OUTCOME_V2_NAMESPACE::std_result<Deserializer<IrUnitHandler, QueryHandlerType>> {
return Deserializer<IrUnitHandler, QueryHandlerType>::create(
reader,
std::move(ir_unit_handler),
std::move(query_handler)
);
}
} // namespace clp::ffi::ir_stream

#endif // CLP_FFI_IR_STREAM_DESERIALIZER_HPP

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@LinZhihao-723 I've reviewed the code in QueryHandlerReq.hpp and it looks good! The code is:

  • Well-structured with clear namespace usage
  • Properly documented with informative comments
  • Using modern C++ features like concepts and type traits appropriately
  • Following the coding guidelines (no instances of !<expression> where false == <expression> would be preferred)

The template metaprogramming approach with IsQueryHandler trait and QueryHandlerReq concept is a clean way to enforce type constraints. No issues to report!

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#ifndef CLP_FFI_IR_STREAM_SEARCH_QUERYHANDLERREQ_HPP
#define CLP_FFI_IR_STREAM_SEARCH_QUERYHANDLERREQ_HPP

#include <type_traits>

#include "NewProjectedSchemaTreeNodeCallbackReq.hpp"
#include "QueryHandler.hpp"

namespace clp::ffi::ir_stream::search {
/**
* Defines an empty query handler that can be used with `clp::ffi::ir_stream::Deserializer` to
* deserialize an IR stream without performing any query evaluation.
*/
struct EmptyQueryHandler {};

/**
* A type trait to determine if a given type is an instantiation of
* `clp::ffi::ir_stream::search::QueryHandler`.
* @tparam T The type to check.
*/
template <typename T>
struct IsNonEmptyQueryHandler : std::false_type {};

/**
* Specialization of `IsNonEmptyQueryHandler` for `clp::ffi::ir_stream::search::QueryHandler`.
* @tparam NewProjectedSchemaTreeNodeCallbackType
*/
template <NewProjectedSchemaTreeNodeCallbackReq NewProjectedSchemaTreeNodeCallbackType>
struct IsNonEmptyQueryHandler<QueryHandler<NewProjectedSchemaTreeNodeCallbackType>>
: std::true_type {};

/**
* Requirements for a query handler that can be used with `clp::ffi::ir_stream::Deserializer`. A
* valid query handler must be:
*
* - an `EmptyQueryHandler` or satisfy the `IsNonEmptyQueryHandler` trait.
* - move constructible.
*
* @tparam QueryHandlerType The type to check.
*/
template <typename QueryHandlerType>
concept QueryHandlerReq = (std::is_same_v<QueryHandlerType, EmptyQueryHandler>
|| IsNonEmptyQueryHandler<QueryHandlerType>::value)
&& std::is_move_constructible_v<QueryHandlerType>;
} // namespace clp::ffi::ir_stream::search

#endif // CLP_FFI_IR_STREAM_SEARCH_QUERYHANDLERREQ_HPP
Loading