From 7b203a65bc053a5592c9890ce6f69bce0ac353fd Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 5 May 2025 22:12:02 -0400 Subject: [PATCH 01/25] Add iterative evaluation method --- .../ir_stream/search/AstEvaluationResult.hpp | 11 +- .../clp/ffi/ir_stream/search/ErrorCode.cpp | 7 + .../clp/ffi/ir_stream/search/ErrorCode.hpp | 3 + .../ffi/ir_stream/search/QueryHandlerImpl.cpp | 191 +++++++++++++++++- .../ffi/ir_stream/search/QueryHandlerImpl.hpp | 5 + 5 files changed, 210 insertions(+), 7 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp b/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp index d018b363d0..5cf887b0f6 100644 --- a/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp @@ -2,19 +2,22 @@ #define CLP_FFI_IR_STREAM_SEARCH_ASTEVALUATIONRESULT_HPP #include +#include namespace clp::ffi::ir_stream::search { /** * Enum representing the result of evaluating a search AST. */ -enum class AstEvaluationResult : uint8_t { - True, - False, +enum AstEvaluationResult : uint8_t { + True = 1, + False = 1 << 1, // The AST evaluation is intentionally skipped because it belongs to a pruned branch of the // parent tree. - Pruned, + Pruned = 1 << 2, }; + +using AstEvaluationResultBitmask = std::underlying_type_t; } // namespace clp::ffi::ir_stream::search #endif // CLP_FFI_IR_STREAM_SEARCH_ASTEVALUATIONRESULT_HPP diff --git a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp index 79868bfac6..3b0fe1c93a 100644 --- a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp @@ -19,6 +19,11 @@ auto ErrorCategory::message(ErrorCodeEnum error_enum) const -> std::string { switch (error_enum) { case ErrorCodeEnum::AstDynamicCastFailure: return "Failed to dynamically cast an AST node to the expected type."; + case ErrorCodeEnum::AstEvaluationInvariantViolation: + return "Internal invariant violated during AST evaluation. This indicates a serious " + "bug in the evaluation logic."; + case ErrorCodeEnum::AttemptToIterateAstLeafExpr: + return "Attempted to iterate an leaf expression of an AST."; case ErrorCodeEnum::ColumnDescriptorTokenIteratorOutOfBounds: return "Attempted to access a token beyond the end of the column descriptor."; case ErrorCodeEnum::ColumnTokenizationFailure: @@ -27,6 +32,8 @@ auto ErrorCategory::message(ErrorCodeEnum error_enum) const -> std::string { return "The projected column is not unique."; case ErrorCodeEnum::EncodedTextAstDecodingFailure: return "Failed to decode the given encoded text AST."; + case ErrorCodeEnum::ExpressionTypeUnexpected: + return "Unexpected expression type."; case ErrorCodeEnum::LiteralTypeUnexpected: return "Unexpected literal type."; case ErrorCodeEnum::LiteralTypeUnsupported: diff --git a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp index 9198ffbca9..407d6c53b0 100644 --- a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp @@ -11,10 +11,13 @@ namespace clp::ffi::ir_stream::search { */ enum class ErrorCodeEnum : uint8_t { AstDynamicCastFailure = 1, + AstEvaluationInvariantViolation, + AttemptToIterateAstLeafExpr, ColumnDescriptorTokenIteratorOutOfBounds, ColumnTokenizationFailure, DuplicateProjectedColumn, EncodedTextAstDecodingFailure, + ExpressionTypeUnexpected, LiteralTypeUnexpected, LiteralTypeUnsupported, MethodNotImplemented, diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index 6fe14781fa..3ecf4952c0 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -7,11 +7,13 @@ #include #include #include +#include #include #include #include "../../../../clp_s/archive_constants.hpp" +#include "../../../../clp_s/search/ast/AndExpr.hpp" #include "../../../../clp_s/search/ast/ColumnDescriptor.hpp" #include "../../../../clp_s/search/ast/ConvertToExists.hpp" #include "../../../../clp_s/search/ast/EmptyExpr.hpp" @@ -19,8 +21,10 @@ #include "../../../../clp_s/search/ast/FilterExpr.hpp" #include "../../../../clp_s/search/ast/Literal.hpp" #include "../../../../clp_s/search/ast/NarrowTypes.hpp" +#include "../../../../clp_s/search/ast/OrExpr.hpp" #include "../../../../clp_s/search/ast/OrOfAndForm.hpp" #include "../../../../clp_s/search/ast/SearchUtils.hpp" +#include "../../../../clp_s/search/ast/Value.hpp" #include "../../KeyValuePairLogEvent.hpp" #include "../../SchemaTree.hpp" #include "AstEvaluationResult.hpp" @@ -28,11 +32,96 @@ namespace clp::ffi::ir_stream::search { namespace { +using clp_s::search::ast::AndExpr; using clp_s::search::ast::ColumnDescriptor; using clp_s::search::ast::EmptyExpr; using clp_s::search::ast::Expression; using clp_s::search::ast::FilterExpr; using clp_s::search::ast::LiteralTypeBitmask; +using clp_s::search::ast::OrExpr; + +/** + * Iterator for efficiently traversing and evaluating clp-s AST's expressions. + */ +class AstExprIterator { +public: + // Factory function + // TODO + [[nodiscard]] static auto create(clp_s::search::ast::Value* expr) + -> outcome_v2::std_result { + if (auto* and_expr{dynamic_cast(expr)}; nullptr != and_expr) { + return AstExprIterator{ExprVariant{and_expr}, and_expr->op_begin(), and_expr->op_end()}; + } + + if (auto* or_expr{dynamic_cast(expr)}; nullptr != or_expr) { + return AstExprIterator{ExprVariant{or_expr}, or_expr->op_begin(), or_expr->op_end()}; + } + + if (auto* filter_expr{dynamic_cast(expr)}; nullptr != filter_expr) { + return AstExprIterator{ + ExprVariant{filter_expr}, + filter_expr->op_begin(), + filter_expr->op_end() + }; + } + + return ErrorCode{ErrorCodeEnum::ExpressionTypeUnexpected}; + } + + // Methods + // TODO + [[nodiscard]] auto next_op() -> std::optional> { + if (m_op_end_it == m_op_next_it) { + return std::nullopt; + } + if (std::holds_alternative(m_expr)) { + return ErrorCode{ErrorCodeEnum::AttemptToIterateAstLeafExpr}; + } + return create((m_op_next_it++)->get()); + } + + // TODO + [[nodiscard]] auto as_and_expr() const -> AndExpr* { + if (std::holds_alternative(m_expr)) { + return std::get(m_expr); + } + return nullptr; + } + + // TODO + [[nodiscard]] auto as_or_expr() const -> OrExpr* { + if (std::holds_alternative(m_expr)) { + return std::get(m_expr); + } + return nullptr; + } + + // TODO + [[nodiscard]] auto as_filter_expr() const -> FilterExpr* { + if (std::holds_alternative(m_expr)) { + return std::get(m_expr); + } + return nullptr; + } + +private: + // Types + using ExprVariant = std::variant; + + // Constructor + AstExprIterator( + ExprVariant expr, + clp_s::search::ast::OpList::iterator op_next_it, + clp_s::search::ast::OpList::iterator op_end_it + ) + : m_expr{expr}, + m_op_next_it{op_next_it}, + m_op_end_it{op_end_it} {} + + ExprVariant m_expr; + clp_s::search::ast::OpList::iterator m_op_next_it; + clp_s::search::ast::OpList::iterator m_op_end_it; +}; /** * Pre-processes a search query by applying several transformation passes. @@ -318,12 +407,108 @@ auto QueryHandlerImpl::create( }; } -// TODO: Fix clang-tidy -// NOLINTNEXTLINE(*) auto QueryHandlerImpl::evaluate_node_id_value_pairs( [[maybe_unused]] KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, [[maybe_unused]] KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs ) -> outcome_v2::std_result { - return ErrorCode{ErrorCodeEnum::MethodNotImplemented}; + if (m_is_empty_query) { + return AstEvaluationResult::True; + } + + std::optional optional_evaluation_result; + std::vector> ast_dfs_stack; + ast_dfs_stack.emplace_back( + OUTCOME_TRYX(AstExprIterator::create(m_query.get())), + AstEvaluationResultBitmask{} + ); + + auto pop_stack_and_update_parent_evaluation_result + = [&](AstEvaluationResult child_expr_result, bool is_inverted = false) -> void { + ast_dfs_stack.pop_back(); + if (child_expr_result != AstEvaluationResult::Pruned && is_inverted) { + child_expr_result = (child_expr_result == AstEvaluationResult::True) + ? AstEvaluationResult::False + : AstEvaluationResult::True; + } + if (ast_dfs_stack.empty()) { + optional_evaluation_result.emplace(child_expr_result); + return; + } + ast_dfs_stack.back().second |= child_expr_result; + }; + + while (false == ast_dfs_stack.empty()) { + auto& [expr_it, evaluation_results] = ast_dfs_stack.back(); + if (auto* filter_expr{expr_it.as_filter_expr()}; nullptr != filter_expr) { + // Handle `FilterExpr` evaluation + // TODO: Evaluate filter expression. + // pop_stack_and_update_parent_evaluation_result(filter_result); + continue; + } + + if (auto* and_expr{expr_it.as_and_expr()}; nullptr != and_expr) { + // Handle `AndExpr` evaluation + if (0 != (evaluation_results & AstEvaluationResult::Pruned)) { + pop_stack_and_update_parent_evaluation_result(AstEvaluationResult::Pruned); + continue; + } + if (0 != (evaluation_results & AstEvaluationResult::False)) { + pop_stack_and_update_parent_evaluation_result( + AstEvaluationResult::False, + and_expr->is_inverted() + ); + continue; + } + auto const optional_next_op_it{expr_it.next_op()}; + if (optional_next_op_it.has_value()) { + ast_dfs_stack.emplace_back( + OUTCOME_TRYX(optional_next_op_it.value()), + AstEvaluationResultBitmask{} + ); + } else { + pop_stack_and_update_parent_evaluation_result( + AstEvaluationResult::True, + and_expr->is_inverted() + ); + } + continue; + } + + // Handle `OrExpr` evaluation + auto* or_expr{expr_it.as_or_expr()}; + if (nullptr == or_expr) { + return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; + } + if (0 != (evaluation_results & AstEvaluationResult::True)) { + pop_stack_and_update_parent_evaluation_result( + AstEvaluationResult::True, + or_expr->is_inverted() + ); + continue; + } + auto const optional_next_op_it{expr_it.next_op()}; + if (optional_next_op_it.has_value()) { + ast_dfs_stack.emplace_back( + OUTCOME_TRYX(optional_next_op_it.value()), + AstEvaluationResultBitmask{} + ); + continue; + } + if (evaluation_results == (evaluation_results & AstEvaluationResult::Pruned)) { + // All pruned + pop_stack_and_update_parent_evaluation_result(AstEvaluationResult::Pruned); + continue; + } + pop_stack_and_update_parent_evaluation_result( + AstEvaluationResult::False, + or_expr->is_inverted() + ); + } + + if (false == optional_evaluation_result.has_value()) { + return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; + } + + return optional_evaluation_result.value(); } } // namespace clp::ffi::ir_stream::search diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index c809c1b42e..14c17bb260 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -12,6 +12,7 @@ #include #include "../../../../clp_s/search/ast/ColumnDescriptor.hpp" +#include "../../../../clp_s/search/ast/EmptyExpr.hpp" #include "../../../../clp_s/search/ast/Expression.hpp" #include "../../../../clp_s/search/ast/Literal.hpp" #include "../../KeyValuePairLogEvent.hpp" @@ -193,6 +194,9 @@ class QueryHandlerImpl { bool case_sensitive_match ) : m_query{std::move(query)}, + m_is_empty_query{ + nullptr != dynamic_cast(m_query.get()) + }, m_auto_gen_namespace_partial_resolutions{ std::move(auto_gen_namespace_partial_resolutions) }, @@ -227,6 +231,7 @@ class QueryHandlerImpl { // Variables std::shared_ptr m_query; + bool m_is_empty_query; PartialResolutionMap m_auto_gen_namespace_partial_resolutions; PartialResolutionMap m_user_gen_namespace_partial_resolutions; std::unordered_map< From e74e31f2034f5bb14c0bd5afe1ef31ab47135bfb Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 6 May 2025 11:40:06 -0400 Subject: [PATCH 02/25] Fix null query issue. --- .../core/src/clp/ffi/ir_stream/search/ErrorCode.cpp | 2 -- .../core/src/clp/ffi/ir_stream/search/ErrorCode.hpp | 1 - .../clp/ffi/ir_stream/search/QueryHandlerImpl.cpp | 13 ++++++++++--- .../ir_stream/search/test/test_QueryHandlerImpl.cpp | 4 ++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp index 3b0fe1c93a..135d5bbc86 100644 --- a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp @@ -42,8 +42,6 @@ auto ErrorCategory::message(ErrorCodeEnum error_enum) const -> std::string { return "The requested method is not implemented."; case ErrorCodeEnum::ProjectionColumnDescriptorCreationFailure: return "Failed to create a column descriptor for the given projection."; - case ErrorCodeEnum::QueryExpressionIsNull: - return "The query expression is NULL."; case ErrorCodeEnum::QueryTransformationPassFailed: return "Failed to execute transformation passes on the query expression."; default: diff --git a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp index 407d6c53b0..3b2b7e9fed 100644 --- a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.hpp @@ -23,7 +23,6 @@ enum class ErrorCodeEnum : uint8_t { MethodNotImplemented, ProjectionColumnDescriptorCreationFailure, QueryTransformationPassFailed, - QueryExpressionIsNull, }; using ErrorCode = ystdlib::error_handling::ErrorCode; diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index 3ecf4952c0..a7728d361c 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -127,7 +127,6 @@ class AstExprIterator { * Pre-processes a search query by applying several transformation passes. * @param query * @return A result containing the transformed - * - ErrorCodeEnum::QueryExpressionIsNull if `query` is nullptr. * - ErrorCodeEnum::QueryTransformationPassFailed if any of the transformation pass failed. */ [[nodiscard]] auto preprocess_query(std::shared_ptr query) @@ -199,7 +198,7 @@ class AstExprIterator { auto preprocess_query(std::shared_ptr query) -> outcome_v2::std_result> { if (nullptr == query) { - return ErrorCode{ErrorCodeEnum::QueryExpressionIsNull}; + return query; } if (nullptr != std::dynamic_pointer_cast(query)) { @@ -317,6 +316,10 @@ auto initialize_partial_resolution_from_search_ast( QueryHandlerImpl::PartialResolutionMap& auto_gen_namespace_partial_resolutions, QueryHandlerImpl::PartialResolutionMap& user_gen_namespace_partial_resolutions ) -> outcome_v2::std_result { + if (nullptr == root) { + return outcome_v2::success(); + } + std::vector ast_dfs_stack; ast_dfs_stack.emplace_back(root.get()); while (false == ast_dfs_stack.empty()) { @@ -411,10 +414,14 @@ auto QueryHandlerImpl::evaluate_node_id_value_pairs( [[maybe_unused]] KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, [[maybe_unused]] KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs ) -> outcome_v2::std_result { - if (m_is_empty_query) { + if (nullptr == m_query) { return AstEvaluationResult::True; } + if (m_is_empty_query) { + return AstEvaluationResult::False; + } + std::optional optional_evaluation_result; std::vector> ast_dfs_stack; ast_dfs_stack.emplace_back( diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 1b6d522c2d..3387428732 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -396,7 +396,7 @@ TEST_CASE("query_handler_handle_projection", "[ffi][ir_stream][search][QueryHand auto const unresolvable_projections_from_unrecognized_namespaces{ generate_projections(cReservedNamespace1, column_query_to_possible_matches).first }; - auto empty_query = clp_s::search::ast::EmptyExpr::create(); + auto null_query = std::shared_ptr{}; auto projections{resolvable_projections}; projections.insert( @@ -405,7 +405,7 @@ TEST_CASE("query_handler_handle_projection", "[ffi][ir_stream][search][QueryHand unresolvable_projections_from_unrecognized_namespaces.cend() ); - auto query_handler_impl_result{QueryHandlerImpl::create(empty_query, projections, true)}; + auto query_handler_impl_result{QueryHandlerImpl::create(null_query, projections, true)}; REQUIRE_FALSE(query_handler_impl_result.has_error()); auto& query_handler_impl{query_handler_impl_result.value()}; From e3636fe3def64631b5ecb194aa48caf5b9bd6e9a Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 6 May 2025 17:07:07 -0400 Subject: [PATCH 03/25] Implement search. --- .../clp/ffi/ir_stream/search/QueryHandler.hpp | 16 +- .../ffi/ir_stream/search/QueryHandlerImpl.cpp | 464 ++++++++++++------ .../ffi/ir_stream/search/QueryHandlerImpl.hpp | 169 ++++++- 3 files changed, 464 insertions(+), 185 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp index f995a18062..4ad1376124 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp @@ -90,21 +90,15 @@ class QueryHandler { } /** - * Evaluates the given node-ID-value pairs against the underlying query. - * @param auto_gen_node_id_value_pairs - * @param user_gen_node_id_value_pairs + * Evaluates the given kv-pair log event against the underlying query. + * @param log_event * @return A result containing the evaluation result on success, or an error code indicating * the failure: * - Forwards `QueryHandlerImpl::evaluate_node_id_value_pairs`'s return values. */ - [[nodiscard]] auto evaluate_node_id_value_pairs( - KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, - KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs - ) -> outcome_v2::std_result { - return m_query_handler_impl.evaluate_node_id_value_pairs( - auto_gen_node_id_value_pairs, - user_gen_node_id_value_pairs - ); + [[nodiscard]] auto evaluate_kv_pair_log_event(KeyValuePairLogEvent const& log_event, ) + -> outcome_v2::std_result { + return m_query_handler_impl.evaluate_kv_pair_log_event(log_event); } private: diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index a7728d361c..13b31ac9c3 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -25,103 +25,21 @@ #include "../../../../clp_s/search/ast/OrOfAndForm.hpp" #include "../../../../clp_s/search/ast/SearchUtils.hpp" #include "../../../../clp_s/search/ast/Value.hpp" +#include "../../../TraceableException.hpp" #include "../../KeyValuePairLogEvent.hpp" #include "../../SchemaTree.hpp" +#include "../../Value.hpp" #include "AstEvaluationResult.hpp" #include "ErrorCode.hpp" +#include "utils.hpp" namespace clp::ffi::ir_stream::search { namespace { -using clp_s::search::ast::AndExpr; using clp_s::search::ast::ColumnDescriptor; using clp_s::search::ast::EmptyExpr; using clp_s::search::ast::Expression; using clp_s::search::ast::FilterExpr; using clp_s::search::ast::LiteralTypeBitmask; -using clp_s::search::ast::OrExpr; - -/** - * Iterator for efficiently traversing and evaluating clp-s AST's expressions. - */ -class AstExprIterator { -public: - // Factory function - // TODO - [[nodiscard]] static auto create(clp_s::search::ast::Value* expr) - -> outcome_v2::std_result { - if (auto* and_expr{dynamic_cast(expr)}; nullptr != and_expr) { - return AstExprIterator{ExprVariant{and_expr}, and_expr->op_begin(), and_expr->op_end()}; - } - - if (auto* or_expr{dynamic_cast(expr)}; nullptr != or_expr) { - return AstExprIterator{ExprVariant{or_expr}, or_expr->op_begin(), or_expr->op_end()}; - } - - if (auto* filter_expr{dynamic_cast(expr)}; nullptr != filter_expr) { - return AstExprIterator{ - ExprVariant{filter_expr}, - filter_expr->op_begin(), - filter_expr->op_end() - }; - } - - return ErrorCode{ErrorCodeEnum::ExpressionTypeUnexpected}; - } - - // Methods - // TODO - [[nodiscard]] auto next_op() -> std::optional> { - if (m_op_end_it == m_op_next_it) { - return std::nullopt; - } - if (std::holds_alternative(m_expr)) { - return ErrorCode{ErrorCodeEnum::AttemptToIterateAstLeafExpr}; - } - return create((m_op_next_it++)->get()); - } - - // TODO - [[nodiscard]] auto as_and_expr() const -> AndExpr* { - if (std::holds_alternative(m_expr)) { - return std::get(m_expr); - } - return nullptr; - } - - // TODO - [[nodiscard]] auto as_or_expr() const -> OrExpr* { - if (std::holds_alternative(m_expr)) { - return std::get(m_expr); - } - return nullptr; - } - - // TODO - [[nodiscard]] auto as_filter_expr() const -> FilterExpr* { - if (std::holds_alternative(m_expr)) { - return std::get(m_expr); - } - return nullptr; - } - -private: - // Types - using ExprVariant = std::variant; - - // Constructor - AstExprIterator( - ExprVariant expr, - clp_s::search::ast::OpList::iterator op_next_it, - clp_s::search::ast::OpList::iterator op_end_it - ) - : m_expr{expr}, - m_op_next_it{op_next_it}, - m_op_end_it{op_end_it} {} - - ExprVariant m_expr; - clp_s::search::ast::OpList::iterator m_op_next_it; - clp_s::search::ast::OpList::iterator m_op_end_it; -}; /** * Pre-processes a search query by applying several transformation passes. @@ -195,6 +113,44 @@ class AstExprIterator { */ [[nodiscard]] auto is_auto_generated(std::string_view key_namespace) -> std::optional; +/** + * Evaluates a filter expression against the given node-ID-value pair. + * @param filter_expr + * @param node_id + * @param value + * @param schema_tree + * @param case_sensitive_match + * @return A result containing the evaluation result on success, or an error code indicating the + * failure: + * - ErrorCodeEnum::AstEvaluationInvariantViolation if a `TraceableException` is caught during + * evaluation. + * - Forwards `evaluate_filter_against_literal_type_value_pair`'s return values. + */ +[[nodiscard]] auto evaluate_filter_against_node_id_value_pair( + clp_s::search::ast::FilterExpr* filter_expr, + SchemaTree::Node::id_t node_id, + std::optional const& value, + SchemaTree const& schema_tree, + bool case_sensitive_match +) -> outcome_v2::std_result; + +/** + * Evaluates a wildcard filter expression. + * @param filter_expr + * @param node_id_value_pairs + * @param schema_tree + * @param case_sensitive_match + * @return A result containing the evaluation result on success, or an error code indicating the + * failure: + * - Forwards `evaluate_filter_against_node_id_value_pair`'s return values. + */ +[[nodiscard]] auto evaluate_wildcard_filter( + clp_s::search::ast::FilterExpr* filter_expr, + KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs, + SchemaTree const& schema_tree, + bool case_sensitive_match +) -> outcome_v2::std_result; + auto preprocess_query(std::shared_ptr query) -> outcome_v2::std_result> { if (nullptr == query) { @@ -385,6 +341,60 @@ auto is_auto_generated(std::string_view key_namespace) -> std::optional { } return std::nullopt; } + +auto evaluate_filter_against_node_id_value_pair( + clp_s::search::ast::FilterExpr* filter_expr, + SchemaTree::Node::id_t node_id, + std::optional const& value, + SchemaTree const& schema_tree, + bool case_sensitive_match +) -> outcome_v2::std_result { + try { + auto const node_type{schema_tree.get_node(node_id).get_type()}; + auto const literal_type{schema_tree_node_type_value_pair_to_literal_type(node_type, value)}; + if (false == filter_expr->get_column()->matches_type(literal_type)) { + return AstEvaluationResult::Pruned; + } + if (OUTCOME_TRYX(evaluate_filter_against_literal_type_value_pair( + filter_expr, + literal_type, + value, + case_sensitive_match + ))) + { + return AstEvaluationResult::True; + } + return AstEvaluationResult::False; + } catch (TraceableException const& ex) { + return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; + } +} + +auto evaluate_wildcard_filter( + clp_s::search::ast::FilterExpr* filter_expr, + KeyValuePairLogEvent::NodeIdValuePairs const& node_id_value_pairs, + SchemaTree const& schema_tree, + bool case_sensitive_match +) -> outcome_v2::std_result { + AstEvaluationResultBitmask evaluation_results{}; + for (auto const& [node_id, value] : node_id_value_pairs) { + auto const evaluation_result{OUTCOME_TRYX(evaluate_filter_against_node_id_value_pair( + filter_expr, + node_id, + value, + schema_tree, + case_sensitive_match + ))}; + if (AstEvaluationResult::True == evaluation_result) { + return AstEvaluationResult::True; + } + evaluation_results |= evaluation_result; + } + if ((evaluation_results & AstEvaluationResult::False) != 0) { + return AstEvaluationResult::False; + } + return AstEvaluationResult::Pruned; +} } // namespace auto QueryHandlerImpl::create( @@ -410,10 +420,8 @@ auto QueryHandlerImpl::create( }; } -auto QueryHandlerImpl::evaluate_node_id_value_pairs( - [[maybe_unused]] KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, - [[maybe_unused]] KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs -) -> outcome_v2::std_result { +auto QueryHandlerImpl::evaluate_kv_pair_log_event(KeyValuePairLogEvent const& log_event) + -> outcome_v2::std_result { if (nullptr == m_query) { return AstEvaluationResult::True; } @@ -423,99 +431,229 @@ auto QueryHandlerImpl::evaluate_node_id_value_pairs( } std::optional optional_evaluation_result; - std::vector> ast_dfs_stack; - ast_dfs_stack.emplace_back( - OUTCOME_TRYX(AstExprIterator::create(m_query.get())), - AstEvaluationResultBitmask{} - ); + m_ast_dfs_stack.clear(); + push_ast_dfs_stack(OUTCOME_TRYX(AstExprIterator::create(m_query.get()))); + while (false == m_ast_dfs_stack.empty()) { + OUTCOME_TRYV(advance_ast_dfs_evaluation(log_event, optional_evaluation_result)); + } - auto pop_stack_and_update_parent_evaluation_result - = [&](AstEvaluationResult child_expr_result, bool is_inverted = false) -> void { - ast_dfs_stack.pop_back(); - if (child_expr_result != AstEvaluationResult::Pruned && is_inverted) { - child_expr_result = (child_expr_result == AstEvaluationResult::True) - ? AstEvaluationResult::False - : AstEvaluationResult::True; + if (false == optional_evaluation_result.has_value()) { + return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; + } + + return optional_evaluation_result.value(); +} + +auto QueryHandlerImpl::AstExprIterator::create(clp_s::search::ast::Value* expr) + -> outcome_v2::std_result { + if (auto* and_expr{dynamic_cast(expr)}; nullptr != and_expr) { + return AstExprIterator{ + ExprVariant{and_expr}, + and_expr->op_begin(), + and_expr->op_end(), + and_expr->is_inverted() + }; + } + + if (auto* or_expr{dynamic_cast(expr)}; nullptr != or_expr) { + return AstExprIterator{ + ExprVariant{or_expr}, + or_expr->op_begin(), + or_expr->op_end(), + or_expr->is_inverted() + }; + } + + if (auto* filter_expr{dynamic_cast(expr)}; + nullptr != filter_expr) + { + return AstExprIterator{ + ExprVariant{filter_expr}, + filter_expr->op_begin(), + filter_expr->op_end(), + filter_expr->is_inverted() + }; + } + + return ErrorCode{ErrorCodeEnum::ExpressionTypeUnexpected}; +} + +auto QueryHandlerImpl::AstExprIterator::next_op() + -> std::optional> { + if (m_op_end_it == m_op_next_it) { + return std::nullopt; + } + if (std::holds_alternative(m_expr)) { + return ErrorCode{ErrorCodeEnum::AttemptToIterateAstLeafExpr}; + } + return create((m_op_next_it++)->get()); +} + +auto QueryHandlerImpl::evaluate_filter_expr( + clp_s::search::ast::FilterExpr* filter_expr, + KeyValuePairLogEvent const& log_event +) -> outcome_v2::std_result { + auto* col{filter_expr->get_column().get()}; + + if (col->is_pure_wildcard()) { + auto const auto_gen_evaluation_result{OUTCOME_TRYX(evaluate_wildcard_filter( + filter_expr, + log_event.get_auto_gen_node_id_value_pairs(), + log_event.get_auto_gen_keys_schema_tree(), + m_case_sensitive_match + ))}; + if (AstEvaluationResult::True == auto_gen_evaluation_result) { + return AstEvaluationResult::True; } - if (ast_dfs_stack.empty()) { - optional_evaluation_result.emplace(child_expr_result); - return; + + auto const user_gen_evaluation_result{OUTCOME_TRYX(evaluate_wildcard_filter( + filter_expr, + log_event.get_user_gen_node_id_value_pairs(), + log_event.get_user_gen_keys_schema_tree(), + m_case_sensitive_match + ))}; + if (AstEvaluationResult::True == user_gen_evaluation_result) { + return AstEvaluationResult::True; } - ast_dfs_stack.back().second |= child_expr_result; - }; - while (false == ast_dfs_stack.empty()) { - auto& [expr_it, evaluation_results] = ast_dfs_stack.back(); - if (auto* filter_expr{expr_it.as_filter_expr()}; nullptr != filter_expr) { - // Handle `FilterExpr` evaluation - // TODO: Evaluate filter expression. - // pop_stack_and_update_parent_evaluation_result(filter_result); - continue; + if (AstEvaluationResult::Pruned == auto_gen_evaluation_result + && AstEvaluationResult::Pruned == user_gen_evaluation_result) + { + return AstEvaluationResult::Pruned; } + return AstEvaluationResult::False; + } - if (auto* and_expr{expr_it.as_and_expr()}; nullptr != and_expr) { - // Handle `AndExpr` evaluation - if (0 != (evaluation_results & AstEvaluationResult::Pruned)) { - pop_stack_and_update_parent_evaluation_result(AstEvaluationResult::Pruned); - continue; - } - if (0 != (evaluation_results & AstEvaluationResult::False)) { - pop_stack_and_update_parent_evaluation_result( - AstEvaluationResult::False, - and_expr->is_inverted() - ); - continue; - } - auto const optional_next_op_it{expr_it.next_op()}; - if (optional_next_op_it.has_value()) { - ast_dfs_stack.emplace_back( - OUTCOME_TRYX(optional_next_op_it.value()), - AstEvaluationResultBitmask{} - ); - } else { - pop_stack_and_update_parent_evaluation_result( - AstEvaluationResult::True, - and_expr->is_inverted() - ); - } + if (false == m_resolved_column_to_schema_tree_node_ids.contains(col)) { + return AstEvaluationResult::Pruned; + } + + auto const optional_is_auto_gen{is_auto_generated(col->get_namespace())}; + if (false == optional_is_auto_gen.has_value()) { + return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; + } + auto const& schema_tree{ + *optional_is_auto_gen ? log_event.get_auto_gen_keys_schema_tree() + : log_event.get_user_gen_keys_schema_tree() + }; + auto const& node_id_value_pairs{ + *optional_is_auto_gen ? log_event.get_auto_gen_node_id_value_pairs() + : log_event.get_user_gen_node_id_value_pairs() + }; + auto const& matchable_node_ids{m_resolved_column_to_schema_tree_node_ids.at(col)}; + + AstEvaluationResultBitmask evaluation_results{}; + for (auto const matchable_node_id : matchable_node_ids) { + if (false == node_id_value_pairs.contains(matchable_node_id)) { continue; } + auto const evaluation_result{OUTCOME_TRYX(evaluate_filter_against_node_id_value_pair( + filter_expr, + matchable_node_id, + node_id_value_pairs.at(matchable_node_id), + schema_tree, + m_case_sensitive_match + ))}; + if (AstEvaluationResult::True == evaluation_result) { + return AstEvaluationResult::True; + } + evaluation_results |= evaluation_result; + } + + if ((evaluation_results & AstEvaluationResult::False) != 0) { + return AstEvaluationResult::False; + } + return AstEvaluationResult::Pruned; +} - // Handle `OrExpr` evaluation - auto* or_expr{expr_it.as_or_expr()}; - if (nullptr == or_expr) { - return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; +auto QueryHandlerImpl::pop_ast_dfs_stack_and_update_evaluation_results( + clp::ffi::ir_stream::search::AstEvaluationResult evaluation_result, + std::optional& query_evaluation_result +) -> void { + auto const is_inverted{m_ast_dfs_stack.back().first.is_inverted()}; + if (AstEvaluationResult::Pruned != evaluation_result && is_inverted) { + evaluation_result = AstEvaluationResult::True == evaluation_result + ? AstEvaluationResult::False + : AstEvaluationResult::True; + } + m_ast_dfs_stack.pop_back(); + if (m_ast_dfs_stack.empty()) { + query_evaluation_result.emplace(evaluation_result); + return; + } + m_ast_dfs_stack.back().second |= evaluation_result; +} + +auto QueryHandlerImpl::advance_ast_dfs_evaluation( + KeyValuePairLogEvent const& log_event, + std::optional& query_evaluation_result +) -> outcome_v2::std_result { + auto& [expr_it, evaluation_results] = m_ast_dfs_stack.back(); + if (auto* filter_expr{expr_it.as_filter_expr()}; nullptr != filter_expr) { + pop_ast_dfs_stack_and_update_evaluation_results( + OUTCOME_TRYX(evaluate_filter_expr(filter_expr, log_event)), + query_evaluation_result + ); + return outcome_v2::success(); + } + + if (auto const* and_expr{expr_it.as_and_expr()}; nullptr != and_expr) { + // Handle `AndExpr` evaluation + if (0 != (evaluation_results & AstEvaluationResult::Pruned)) { + pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult::Pruned, + query_evaluation_result + ); + return outcome_v2::success(); } - if (0 != (evaluation_results & AstEvaluationResult::True)) { - pop_stack_and_update_parent_evaluation_result( - AstEvaluationResult::True, - or_expr->is_inverted() + if (0 != (evaluation_results & AstEvaluationResult::False)) { + pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult::False, + query_evaluation_result ); - continue; + return outcome_v2::success(); } auto const optional_next_op_it{expr_it.next_op()}; if (optional_next_op_it.has_value()) { - ast_dfs_stack.emplace_back( - OUTCOME_TRYX(optional_next_op_it.value()), - AstEvaluationResultBitmask{} + push_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); + } else { + pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult::True, + query_evaluation_result ); - continue; - } - if (evaluation_results == (evaluation_results & AstEvaluationResult::Pruned)) { - // All pruned - pop_stack_and_update_parent_evaluation_result(AstEvaluationResult::Pruned); - continue; } - pop_stack_and_update_parent_evaluation_result( - AstEvaluationResult::False, - or_expr->is_inverted() - ); + return outcome_v2::success(); } - if (false == optional_evaluation_result.has_value()) { + // Handle `OrExpr` evaluation + auto const* or_expr{expr_it.as_or_expr()}; + if (nullptr == or_expr) { return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; } - - return optional_evaluation_result.value(); + if (0 != (evaluation_results & AstEvaluationResult::True)) { + pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult::True, + query_evaluation_result + ); + return outcome_v2::success(); + } + auto const optional_next_op_it{expr_it.next_op()}; + if (optional_next_op_it.has_value()) { + push_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); + return outcome_v2::success(); + } + if ((evaluation_results & AstEvaluationResult::False) != 0) { + pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult::False, + query_evaluation_result + ); + return outcome_v2::success(); + } + // All pruned + pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult::Pruned, + query_evaluation_result + ); + return outcome_v2::success(); } } // namespace clp::ffi::ir_stream::search diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 14c17bb260..e3b1954852 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -2,19 +2,25 @@ #define CLP_FFI_IR_STREAM_SEARCH_QUERYHANDLERIMPL_HPP #include +#include #include #include #include #include #include +#include #include #include +#include "../../../../clp_s/search/ast/AndExpr.hpp" #include "../../../../clp_s/search/ast/ColumnDescriptor.hpp" #include "../../../../clp_s/search/ast/EmptyExpr.hpp" #include "../../../../clp_s/search/ast/Expression.hpp" +#include "../../../../clp_s/search/ast/FilterExpr.hpp" #include "../../../../clp_s/search/ast/Literal.hpp" +#include "../../../../clp_s/search/ast/OrExpr.hpp" +#include "../../../../clp_s/search/ast/Value.hpp" #include "../../KeyValuePairLogEvent.hpp" #include "../../SchemaTree.hpp" #include "AstEvaluationResult.hpp" @@ -145,24 +151,26 @@ class QueryHandlerImpl { ~QueryHandlerImpl() = default; /** - * Implementation of `QueryHandler::evaluate_node_id_value_pairs`. - * @param auto_gen_node_id_value_pairs + * Implementation of `QueryHandler::evaluate_kv_pair_log_event`. + * @param log_event * @param user_gen_node_id_value_pairs * @return A result containing the evaluation result on success, or an error code indicating * the failure: - * - TODO + * - ErrorCodeEnum::AstEvaluationInvariantViolation if the underlying AST DFS evaluation doesn't + * return any evaluation results. + * - Forwards `AstExprIterator::create`'s return values. + * - Forwards `advance_ast_dfs_evaluation`'s return values. */ - [[nodiscard]] auto evaluate_node_id_value_pairs( - KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, - KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs - ) -> outcome_v2::std_result; + [[nodiscard]] auto evaluate_kv_pair_log_event(KeyValuePairLogEvent const& log_event) + -> outcome_v2::std_result; /** * Implementation of `QueryHandler::update_partially_resolved_columns` with new projected * schema-tree node callback given as a template parameter. * @tparam NewProjectedSchemaTreeNodeCallbackType - * @param auto_gen_node_id_value_pairs - * @param user_gen_node_id_value_pairs + * @param is_auto_generated + * @param node_locator + * @param node_id * @param new_projected_schema_tree_node_callback * @return A result containing the evaluation result on success, or an error code indicating * the failure: @@ -184,6 +192,97 @@ class QueryHandlerImpl { } private: + // Types + /** + * Iterator for efficiently traversing and evaluating clp-s AST's expressions. + */ + class AstExprIterator { + public: + // Factory function + /** + * @param expr + * @return A result containing the constructed iterator for `expr`, or an error code + * indicating the failure: + * - ErrorCodeEnum::ExpressionTypeUnexpected if the type of expression is not one of the + * following: + * - clp_s::search::ast::FilterExpr + * - clp_s::search::ast::AndExpr + * - clp_s::search::ast::OrExpr + */ + [[nodiscard]] static auto create(clp_s::search::ast::Value* expr) + -> outcome_v2::std_result; + + // Methods + /** + * @return A result containing the iterator of the next child expression operator, or an + * error code indicating the failure: + * - ErrorCodeEnum::AttemptToIterateAstLeafExpr if the current expression is a leaf + * expression + * (`clp_s::search::ast::FilterExpr`). + * - Forwards `create`'s return values. + */ + [[nodiscard]] auto next_op() -> std::optional>; + + /** + * @return The underlying expression as `clp_s::search::ast::AndExpr` if the underlying type + * matches, or nullptr otherwise. + */ + [[nodiscard]] auto as_and_expr() const -> clp_s::search::ast::AndExpr const* { + if (std::holds_alternative(m_expr)) { + return std::get(m_expr); + } + return nullptr; + } + + /** + * @return The underlying expression as `clp_s::search::ast::OrExpr` if the underlying type + * matches, or nullptr otherwise. + */ + [[nodiscard]] auto as_or_expr() const -> clp_s::search::ast::OrExpr const* { + if (std::holds_alternative(m_expr)) { + return std::get(m_expr); + } + return nullptr; + } + + /** + * @return The underlying expression as `clp_s::search::ast::FilterExpr` if the underlying + * type matches, or nullptr otherwise. + */ + [[nodiscard]] auto as_filter_expr() const -> clp_s::search::ast::FilterExpr* { + if (std::holds_alternative(m_expr)) { + return std::get(m_expr); + } + return nullptr; + } + + [[nodiscard]] auto is_inverted() const -> bool { return m_is_inverted; } + + private: + // Types + using ExprVariant = std::variant< + clp_s::search::ast::AndExpr*, + clp_s::search::ast::OrExpr*, + clp_s::search::ast::FilterExpr*>; + + // Constructor + AstExprIterator( + ExprVariant expr, + clp_s::search::ast::OpList::const_iterator op_next_it, + clp_s::search::ast::OpList::const_iterator op_end_it, + bool is_inverted + ) + : m_expr{expr}, + m_op_next_it{op_next_it}, + m_op_end_it{op_end_it}, + m_is_inverted{is_inverted} {} + + ExprVariant m_expr; + clp_s::search::ast::OpList::const_iterator m_op_next_it; + clp_s::search::ast::OpList::const_iterator m_op_end_it; + bool m_is_inverted; + }; + // Constructor QueryHandlerImpl( std::shared_ptr query, @@ -205,7 +304,7 @@ class QueryHandlerImpl { }, m_projected_columns{std::move(projected_columns)}, m_projected_column_to_original_key{std::move(projected_column_to_original_key)}, - m_case_sensitive_search{case_sensitive_match} {} + m_case_sensitive_match{case_sensitive_match} {} // Methods /** @@ -229,6 +328,53 @@ class QueryHandlerImpl { NewProjectedSchemaTreeNodeCallbackType new_projected_schema_tree_node_callback ) -> outcome_v2::std_result; + /** + * Evaluates the filter expression against the given kv-pair log event. + * @param filter_expr + * @param log_event + * @return A result containing the evaluation result on success, or an error code indicating the + * failure: + * - ErrorCodeEnum::AstEvaluationInvariantViolation if the underlying column of the filter is + * neither user-generated nor auto-generated. + * - Forwards `evaluate_wildcard_filter`'s return values. + * - Forwards `evaluate_filter_against_node_id_value_pair`'s return values. + */ + [[nodiscard]] auto evaluate_filter_expr( + clp_s::search::ast::FilterExpr* filter_expr, + KeyValuePairLogEvent const& log_event + ) -> outcome_v2::std_result; + + auto push_ast_dfs_stack(AstExprIterator ast_expr_it) -> void { + m_ast_dfs_stack.emplace_back(ast_expr_it, AstEvaluationResultBitmask{}); + } + + /** + * Pops the AST DFS stack and update the evaluation accordingly: + * - If the stack if not empty, update the parent's evaluation results. + * - Otherwise, update `query_evaluation_results`. + * @param evaluation_result + * @param query_evaluation_result Returns the query evaluation result. + */ + auto pop_ast_dfs_stack_and_update_evaluation_results( + AstEvaluationResult evaluation_result, + std::optional& query_evaluation_result + ) -> void; + + /** + * Advances AST DFA evaluation by visiting the top of `m_ast_dfs_stack`. + * @param log_event + * @param query_evaluation_result Returns the query evaluation result. + * @return A void result on success, or an error code indicating the failure: + * - ErrorCodeEnum::AstEvaluationInvariantViolation if the expression iterator on the stack top + * is an unexpected expression type. + * - Forwards `evaluate_filter_expr`'s return values. + * - Forwards `AstExprIterator::next_op`'s return values. + */ + [[nodiscard]] auto advance_ast_dfs_evaluation( + KeyValuePairLogEvent const& log_event, + std::optional& query_evaluation_result + ) -> outcome_v2::std_result; + // Variables std::shared_ptr m_query; bool m_is_empty_query; @@ -240,7 +386,8 @@ class QueryHandlerImpl { m_resolved_column_to_schema_tree_node_ids; std::vector> m_projected_columns; ProjectionMap m_projected_column_to_original_key; - bool m_case_sensitive_search; + bool m_case_sensitive_match; + std::vector> m_ast_dfs_stack; }; template From 10008b58445616b212b0978a5b0936e86ff5c533 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 6 May 2025 21:18:24 -0400 Subject: [PATCH 04/25] Ops, forget to commit this one. --- components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp index 4ad1376124..db55ddbe4a 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp @@ -96,7 +96,7 @@ class QueryHandler { * the failure: * - Forwards `QueryHandlerImpl::evaluate_node_id_value_pairs`'s return values. */ - [[nodiscard]] auto evaluate_kv_pair_log_event(KeyValuePairLogEvent const& log_event, ) + [[nodiscard]] auto evaluate_kv_pair_log_event(KeyValuePairLogEvent const& log_event) -> outcome_v2::std_result { return m_query_handler_impl.evaluate_kv_pair_log_event(log_event); } From 00e2ecc9422aae8b58c02f46bb167a7c6ac52f5e Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 6 May 2025 21:44:23 -0400 Subject: [PATCH 05/25] Fix clang-tidy warnings on the tes files. --- .../src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 3387428732..673060dbf2 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -13,7 +13,7 @@ #include #include "../../../../../clp_s/archive_constants.hpp" -#include "../../../../../clp_s/search/ast/EmptyExpr.hpp" +#include "../../../../../clp_s/search/ast/Expression.hpp" #include "../../../../../clp_s/search/ast/Literal.hpp" #include "../../../../../clp_s/search/kql/kql.hpp" #include "../../../SchemaTree.hpp" From ca87b2918cf24bef1577109aafcb371613158ead Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 7 May 2025 12:49:19 -0400 Subject: [PATCH 06/25] Ignore unsupported literal types. --- .../ffi/ir_stream/search/QueryHandlerImpl.cpp | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index 13b31ac9c3..9940dcb8e9 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -355,16 +355,21 @@ auto evaluate_filter_against_node_id_value_pair( if (false == filter_expr->get_column()->matches_type(literal_type)) { return AstEvaluationResult::Pruned; } - if (OUTCOME_TRYX(evaluate_filter_against_literal_type_value_pair( - filter_expr, - literal_type, - value, - case_sensitive_match - ))) - { - return AstEvaluationResult::True; + auto const evaluation_result{evaluate_filter_against_literal_type_value_pair( + filter_expr, + literal_type, + value, + case_sensitive_match + )}; + if (false == evaluation_result.has_error()) { + return evaluation_result.value() ? AstEvaluationResult::True + : AstEvaluationResult::False; } - return AstEvaluationResult::False; + if (ErrorCode{ErrorCodeEnum::LiteralTypeUnsupported} == evaluation_result.error()) { + // Evaluations on unsupported literal types are considered `AstEvaluationResult::False`. + return AstEvaluationResult::False; + } + return evaluation_result.error(); } catch (TraceableException const& ex) { return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; } From 8f51308f674ca430ef2ab77fb956fedf19356ff3 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 7 May 2025 13:39:03 -0400 Subject: [PATCH 07/25] WIP --- .../search/test/test_QueryHandlerImpl.cpp | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 673060dbf2..e42bcd1423 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -448,4 +448,43 @@ TEST_CASE("query_handler_handle_projection", "[ffi][ir_stream][search][QueryHand REQUIRE((expected_resolved_projections == actual_resolved_projections)); } + +TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search][QueryHandler]") { + /* + * <0:root:Obj> + * | + * |------------> <1:a:Obj> + * | | + * |--> <2:b:Int> |--> <3:b:Obj> + * | | | + * |--> <12:a:Int> | |------------> <4:c:Obj> + * | | | + * | |--> <5:d:Str> |--> <7:a:Str> + * | | | + * | |--> <6:d:Bool> |--> <8:d:Str> + * | | | + * | |--> <10:e:Obj> |--> <9:d:Float> + * | | + * |--> <13:b:Bool> |--> <11:f:Obj> + */ + auto const schema_tree{std::make_shared()}; + std::vector const locators{ + {SchemaTree::cRootId, "a", SchemaTree::Node::Type::Obj}, + {SchemaTree::cRootId, "b", SchemaTree::Node::Type::Int}, + {1, "b", SchemaTree::Node::Type::Obj}, + {3, "c", SchemaTree::Node::Type::Obj}, + {3, "d", SchemaTree::Node::Type::Str}, + {3, "d", SchemaTree::Node::Type::Bool}, + {4, "a", SchemaTree::Node::Type::Str}, + {4, "d", SchemaTree::Node::Type::Str}, + {4, "d", SchemaTree::Node::Type::Float}, + {3, "e", SchemaTree::Node::Type::Obj}, + {4, "f", SchemaTree::Node::Type::Obj}, + {SchemaTree::cRootId, "a", SchemaTree::Node::Type::Int}, + {1, "b", SchemaTree::Node::Type::Bool} + }; + for (auto const& locator : locators) { + REQUIRE_NOTHROW(schema_tree->insert_node(locator)); + } +} } // namespace clp::ffi::ir_stream::search::test From e7a578b4040b221872d5a0026edb176a9d69ef19 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 7 May 2025 15:00:56 -0400 Subject: [PATCH 08/25] Working on the unit tests... --- .../search/test/test_QueryHandlerImpl.cpp | 116 +++++++++++++++++- .../ffi/ir_stream/search/test/test_utils.cpp | 34 +---- .../clp/ffi/ir_stream/search/test/utils.hpp | 35 ++++++ 3 files changed, 147 insertions(+), 38 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index e42bcd1423..fd9872a4bb 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -16,8 +16,12 @@ #include "../../../../../clp_s/search/ast/Expression.hpp" #include "../../../../../clp_s/search/ast/Literal.hpp" #include "../../../../../clp_s/search/kql/kql.hpp" +#include "../../../../ir/types.hpp" +#include "../../../../time_types.hpp" +#include "../../../KeyValuePairLogEvent.hpp" #include "../../../SchemaTree.hpp" #include "../../../Value.hpp" +#include "../AstEvaluationResult.hpp" #include "../QueryHandlerImpl.hpp" #include "utils.hpp" @@ -28,7 +32,7 @@ using clp_s::constants::cDefaultNamespace; using clp_s::constants::cReservedNamespace1; using clp_s::search::ast::LiteralTypeBitmask; -constexpr std::string_view cRefTestStr{"*test*"}; +constexpr std::string_view cRefTestStr{"test"}; constexpr value_int_t cRefTestInt{0}; constexpr value_float_t cRefTestFloat{0.0}; constexpr value_bool_t cRefTestBool{false}; @@ -73,7 +77,13 @@ constexpr value_bool_t cRefTestBool{false}; std::map> const& column_resolutions ) -> std::string; -[[nodiscard]] auto generate_matchable_kql_expressions( +/** + * @param node_type + * @return A vector of matchable values of the given node type. + */ +[[nodiscard]] auto get_matchable_values(SchemaTree::Node::Type node_type) -> std::vector; + +auto generate_matchable_kql_expressions( std::string_view column_namespace, std::map const& column_query_to_possible_matches ) -> std::pair, std::map>> { @@ -143,7 +153,7 @@ constexpr value_bool_t cRefTestBool{false}; false == matchable_node_ids.empty()) { matchable_kql_expressions.emplace_back( - fmt::format("{}: {}", column_query_with_namespace, cRefTestStr) + fmt::format("{}: *{}*", column_query_with_namespace, cRefTestStr) ); auto [it, inserted] = expected_column_resolutions.try_emplace( column_query_with_namespace, @@ -202,6 +212,33 @@ auto serialize_column_node_ids_map( } return result; } + +auto get_matchable_values(SchemaTree::Node::Type node_type) -> std::vector { + switch (node_type) { + case SchemaTree::Node::Type::Int: + return {Value{cRefTestInt}}; + case SchemaTree::Node::Type::Float: + return {Value{cRefTestFloat}}; + case SchemaTree::Node::Type::Bool: + return {Value{cRefTestBool}}; + case SchemaTree::Node::Type::Str: { + std::vector matchable_values; + matchable_values.emplace_back(fmt::format("ThisIs{}", cRefTestStr)); + auto const long_str{fmt::format("This is {}", cRefTestStr)}; + matchable_values.emplace_back( + get_encoded_text_ast(long_str) + ); + matchable_values.emplace_back( + get_encoded_text_ast(long_str) + ); + return matchable_values; + } + case SchemaTree::Node::Type::Obj: + return {Value{}}; + default: + return {}; + } +} } // namespace TEST_CASE( @@ -412,8 +449,9 @@ TEST_CASE("query_handler_handle_projection", "[ffi][ir_stream][search][QueryHand SchemaTree::Node::id_t node_id{1}; std::map> actual_resolved_projections; auto new_projected_schema_tree_node_callback - = [&](bool is_auto_gen, SchemaTree::Node::id_t node_id, std::string_view key - ) -> outcome_v2::std_result { + = [&](bool is_auto_gen, + SchemaTree::Node::id_t node_id, + std::string_view key) -> outcome_v2::std_result { REQUIRE((is_auto_generated == is_auto_gen)); auto [column_it, column_inserted] = actual_resolved_projections.try_emplace( std::string{key}, @@ -486,5 +524,73 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search for (auto const& locator : locators) { REQUIRE_NOTHROW(schema_tree->insert_node(locator)); } + + auto const column_query_to_possible_matches{get_schema_tree_column_queries(schema_tree)}; + CAPTURE(column_query_to_possible_matches); + + auto const is_auto_generated = GENERATE(true, false); + auto const [matchable_kql_expressions, expected_column_resolutions] + = generate_matchable_kql_expressions( + is_auto_generated ? cAutogenNamespace : cDefaultNamespace, + column_query_to_possible_matches + ); + + SECTION("basic") { + auto const matchable_kql_query_str{ + fmt::format("{}", fmt::join(matchable_kql_expressions, " OR ")) + }; + CAPTURE(matchable_kql_query_str); + + auto query_stream{std::istringstream{matchable_kql_query_str}}; + auto query{clp_s::search::kql::parse_kql_expression(query_stream)}; + + auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true)}; + REQUIRE_FALSE(query_handler_impl_result.has_error()); + auto& query_handler_impl{query_handler_impl_result.value()}; + + for (auto const& locator : locators) { + auto const optional_node_id{schema_tree->try_get_node_id(locator)}; + REQUIRE(optional_node_id.has_value()); + REQUIRE_FALSE(query_handler_impl.update_partially_resolved_columns( + is_auto_generated, + locator, + *optional_node_id, + trivial_new_projected_schema_tree_node_callback + )); + } + + for (auto const& locator : locators) { + auto const optional_node_id{schema_tree->try_get_node_id(locator)}; + REQUIRE(optional_node_id.has_value()); + auto const matchable_values{get_matchable_values(locator.get_type())}; + for (auto const& matchable_value : matchable_values) { + KeyValuePairLogEvent::NodeIdValuePairs const node_id_value_pairs{ + {*optional_node_id, matchable_value} + }; + auto const auto_gen_node_id_value_pairs{ + is_auto_generated ? node_id_value_pairs + : KeyValuePairLogEvent::NodeIdValuePairs{} + }; + auto const user_gen_node_id_value_pairs{ + is_auto_generated ? KeyValuePairLogEvent::NodeIdValuePairs{} + : node_id_value_pairs + }; + auto const kv_pair_log_event_result{KeyValuePairLogEvent::create( + schema_tree, + schema_tree, + auto_gen_node_id_value_pairs, + user_gen_node_id_value_pairs, + UtcOffset{0} + )}; + REQUIRE_FALSE(kv_pair_log_event_result.has_value()); + auto const& kv_pair_log_event{kv_pair_log_event_result.value()}; + auto const evaluation_result{ + query_handler_impl.evaluate_kv_pair_log_event(kv_pair_log_event) + }; + REQUIRE_FALSE(evaluation_result.has_value()); + REQUIRE(evaluation_result.value() == AstEvaluationResult::True); + } + } + } } } // namespace clp::ffi::ir_stream::search::test diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp index cc8bb0b0fa..eb14d10542 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp @@ -23,9 +23,9 @@ #include "../../../../../clp_s/search/ast/StringLiteral.hpp" #include "../../../../ir/EncodedTextAst.hpp" #include "../../../../ir/types.hpp" -#include "../../../encoding_methods.hpp" #include "../../../Value.hpp" #include "../utils.hpp" +#include "utils.hpp" namespace clp::ffi::ir_stream::search::test { namespace { @@ -52,18 +52,6 @@ using ValueToMatchedFilterOpsPair constexpr std::string_view cRefTestString{"test"}; -/** - * Parses and encodes the given string as an instance of `EncodedTextAst`. - * @tparam encoded_variable_t - * @param text - * @return The encoded result. - */ -template -requires std::is_same_v - || std::is_same_v -[[nodiscard]] auto get_encoded_text_ast(std::string_view text) - -> clp::ir::EncodedTextAst; - /** * Asserts the filter evaluation results match the expectation on the given values. * @param filter_to_test @@ -141,26 +129,6 @@ requires std::is_same_v check_filter_evaluation(FilterExpr const* filter_to_test, LiteralType filter_operand_literal_type) -> bool; -template -requires std::is_same_v - || std::is_same_v -auto get_encoded_text_ast(std::string_view text) -> clp::ir::EncodedTextAst { - std::string logtype; - std::vector encoded_vars; - std::vector dict_var_bounds; - REQUIRE(clp::ffi::encode_message(text, logtype, encoded_vars, dict_var_bounds)); - REQUIRE(((dict_var_bounds.size() % 2) == 0)); - - std::vector dict_vars; - for (size_t i{0}; i < dict_var_bounds.size(); i += 2) { - auto const begin_pos{static_cast(dict_var_bounds[i])}; - auto const end_pos{static_cast(dict_var_bounds[i + 1])}; - dict_vars.emplace_back(text.cbegin() + begin_pos, text.cbegin() + end_pos); - } - - return clp::ir::EncodedTextAst{logtype, dict_vars, encoded_vars}; -} - auto assert_filter_evaluation_results_on_values( FilterExpr const* filter_to_test, LiteralType filter_operand_literal_type, diff --git a/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp b/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp index 6c6dc85d86..dcb798e0c6 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp @@ -10,9 +10,12 @@ #include #include +#include #include #include "../../../../../clp_s/search/ast/Literal.hpp" +#include "../../../../ir/types.hpp" +#include "../../../encoding_methods.hpp" #include "../../../SchemaTree.hpp" #include "../utils.hpp" @@ -86,6 +89,38 @@ class ColumnQueryPossibleMatches { std::ostream& os, std::map const& column_query_to_possible_matches ) -> std::ostream&; + +/** + * Parses and encodes the given string as an instance of `EncodedTextAst`. + * @tparam encoded_variable_t + * @param text + * @return The encoded result. + */ +template +requires std::is_same_v + || std::is_same_v +[[nodiscard]] auto get_encoded_text_ast(std::string_view text) + -> clp::ir::EncodedTextAst; + +template +requires std::is_same_v + || std::is_same_v +auto get_encoded_text_ast(std::string_view text) -> clp::ir::EncodedTextAst { + std::string logtype; + std::vector encoded_vars; + std::vector dict_var_bounds; + REQUIRE(clp::ffi::encode_message(text, logtype, encoded_vars, dict_var_bounds)); + REQUIRE(((dict_var_bounds.size() % 2) == 0)); + + std::vector dict_vars; + for (size_t i{0}; i < dict_var_bounds.size(); i += 2) { + auto const begin_pos{static_cast(dict_var_bounds[i])}; + auto const end_pos{static_cast(dict_var_bounds[i + 1])}; + dict_vars.emplace_back(text.cbegin() + begin_pos, text.cbegin() + end_pos); + } + + return clp::ir::EncodedTextAst{logtype, dict_vars, encoded_vars}; +} } // namespace clp::ffi::ir_stream::search::test #endif // CLP_FFI_IR_STREAM_SEARCH_TEST_UTILS_HPP From 6791912bdf09ad30782b41e61e11ee59e990f351 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 7 May 2025 17:40:17 -0400 Subject: [PATCH 09/25] Add basic --- .../search/test/test_QueryHandlerImpl.cpp | 69 ++++++++++++++----- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index fd9872a4bb..9122a3a651 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -528,17 +528,30 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search auto const column_query_to_possible_matches{get_schema_tree_column_queries(schema_tree)}; CAPTURE(column_query_to_possible_matches); - auto const is_auto_generated = GENERATE(true, false); auto const [matchable_kql_expressions, expected_column_resolutions] - = generate_matchable_kql_expressions( - is_auto_generated ? cAutogenNamespace : cDefaultNamespace, - column_query_to_possible_matches - ); + = generate_matchable_kql_expressions("", column_query_to_possible_matches); + + auto const is_auto_generated = GENERATE(true, false); + std::vector matchable_kql_expressions_with_column_resolutions; + std::vector single_wildcard_kql_expressions; + auto const namespace_id{is_auto_generated ? cAutogenNamespace : cDefaultNamespace}; + for (auto const& matchable_expression : matchable_kql_expressions) { + auto const formatted_expression{fmt::format("{}{}", namespace_id, matchable_expression)}; + if (matchable_expression.starts_with("*:")) { + single_wildcard_kql_expressions.emplace_back(formatted_expression); + } else { + matchable_kql_expressions_with_column_resolutions.emplace_back(formatted_expression); + } + } SECTION("basic") { - auto const matchable_kql_query_str{ - fmt::format("{}", fmt::join(matchable_kql_expressions, " OR ")) - }; + auto const matchable_kql_query_str = GENERATE_COPY( + fmt::format( + "{}", + fmt::join(matchable_kql_expressions_with_column_resolutions, " OR ") + ), + fmt::format("{}", fmt::join(single_wildcard_kql_expressions, " OR ")) + ); CAPTURE(matchable_kql_query_str); auto query_stream{std::istringstream{matchable_kql_query_str}}; @@ -551,21 +564,38 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search for (auto const& locator : locators) { auto const optional_node_id{schema_tree->try_get_node_id(locator)}; REQUIRE(optional_node_id.has_value()); - REQUIRE_FALSE(query_handler_impl.update_partially_resolved_columns( - is_auto_generated, - locator, - *optional_node_id, - trivial_new_projected_schema_tree_node_callback - )); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const node_id{optional_node_id.value()}; + REQUIRE_FALSE(query_handler_impl + .update_partially_resolved_columns( + is_auto_generated, + locator, + node_id, + trivial_new_projected_schema_tree_node_callback + ) + .has_error()); } for (auto const& locator : locators) { auto const optional_node_id{schema_tree->try_get_node_id(locator)}; REQUIRE(optional_node_id.has_value()); - auto const matchable_values{get_matchable_values(locator.get_type())}; + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const node_id{*optional_node_id}; + CAPTURE(fmt::format("Testing Node ID: {}", node_id)); + + auto const node_type{locator.get_type()}; + auto const matchable_values{get_matchable_values(node_type)}; for (auto const& matchable_value : matchable_values) { + if (SchemaTree::Node::Type::UnstructuredArray == node_type + || SchemaTree::Node::Type::Obj == node_type) + { + // We skip these two types because: + // - the current implementation doesn't support `UnstructuredArray` + // - we don't generate matchable queries for `Obj`. + continue; + } KeyValuePairLogEvent::NodeIdValuePairs const node_id_value_pairs{ - {*optional_node_id, matchable_value} + {node_id, matchable_value} }; auto const auto_gen_node_id_value_pairs{ is_auto_generated ? node_id_value_pairs @@ -582,13 +612,14 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search user_gen_node_id_value_pairs, UtcOffset{0} )}; - REQUIRE_FALSE(kv_pair_log_event_result.has_value()); + REQUIRE_FALSE(kv_pair_log_event_result.has_error()); auto const& kv_pair_log_event{kv_pair_log_event_result.value()}; auto const evaluation_result{ query_handler_impl.evaluate_kv_pair_log_event(kv_pair_log_event) }; - REQUIRE_FALSE(evaluation_result.has_value()); - REQUIRE(evaluation_result.value() == AstEvaluationResult::True); + REQUIRE_FALSE(evaluation_result.has_error()); + CAPTURE(evaluation_result.value()); + REQUIRE((evaluation_result.value() == AstEvaluationResult::True)); } } } From dd35aa2302cad6eefec445d14dc4df7700e9f4c2 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Wed, 7 May 2025 21:04:57 -0400 Subject: [PATCH 10/25] Finish test dev --- .../search/test/test_QueryHandlerImpl.cpp | 382 +++++++++++++++--- .../ffi/ir_stream/search/test/test_utils.cpp | 5 - .../clp/ffi/ir_stream/search/test/utils.hpp | 3 + 3 files changed, 337 insertions(+), 53 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 9122a3a651..8dceaeb463 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -83,6 +83,29 @@ constexpr value_bool_t cRefTestBool{false}; */ [[nodiscard]] auto get_matchable_values(SchemaTree::Node::Type node_type) -> std::vector; +/** + * @param node_type + * @return A vector of unmatchable values of the given node type. + */ +[[nodiscard]] auto get_unmatchable_values(SchemaTree::Node::Type node_type) -> std::vector; + +/** + * Gets the query evaluation results on the kv-pair log event constructed by the given node-ID-value + * pairs. + * @param auto_gen_schema_tree + * @param user_gen_schema_tree + * @param auto_gen_node_id_value_pairs + * @param user_gen_node_id_value_pairs + * @param query_handler_impl + */ +[[nodiscard]] auto get_query_evaluation_result( + std::shared_ptr auto_gen_schema_tree, + std::shared_ptr user_gen_schema_tree, + KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs, + QueryHandlerImpl& query_handler_impl +) -> AstEvaluationResult; + auto generate_matchable_kql_expressions( std::string_view column_namespace, std::map const& column_query_to_possible_matches @@ -233,12 +256,65 @@ auto get_matchable_values(SchemaTree::Node::Type node_type) -> std::vector std::vector { + switch (node_type) { + case SchemaTree::Node::Type::Int: + return {Value{cRefTestInt + 1}}; + case SchemaTree::Node::Type::Float: + return {Value{cRefTestFloat + 1.0}}; + case SchemaTree::Node::Type::Bool: + return {Value{false == cRefTestBool}}; + case SchemaTree::Node::Type::Str: { + std::vector matchable_values; + matchable_values.emplace_back(std::string{}); + std::string_view const unmatchable_long_str{"This is a static message"}; + REQUIRE((unmatchable_long_str.find(cRefTestStr) == std::string::npos)); + matchable_values.emplace_back( + get_encoded_text_ast(unmatchable_long_str) + ); + matchable_values.emplace_back( + get_encoded_text_ast(unmatchable_long_str) + ); + return matchable_values; + } + default: + // Unsupported types + REQUIRE(false); + // The following return should never be reached. It is added to silent clang-tidy + // warnings. + return {}; + } +} + +auto get_query_evaluation_result( + std::shared_ptr auto_gen_schema_tree, + std::shared_ptr user_gen_schema_tree, + KeyValuePairLogEvent::NodeIdValuePairs const& auto_gen_node_id_value_pairs, + KeyValuePairLogEvent::NodeIdValuePairs const& user_gen_node_id_value_pairs, + QueryHandlerImpl& query_handler_impl +) -> AstEvaluationResult { + auto const kv_pair_log_event_result{KeyValuePairLogEvent::create( + std::move(auto_gen_schema_tree), + std::move(user_gen_schema_tree), + auto_gen_node_id_value_pairs, + user_gen_node_id_value_pairs, + UtcOffset{0} + )}; + REQUIRE_FALSE(kv_pair_log_event_result.has_error()); + auto const& kv_pair_log_event{kv_pair_log_event_result.value()}; + auto const evaluation_result{query_handler_impl.evaluate_kv_pair_log_event(kv_pair_log_event)}; + REQUIRE_FALSE(evaluation_result.has_error()); + return evaluation_result.value(); +} } // namespace TEST_CASE( @@ -531,30 +607,8 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search auto const [matchable_kql_expressions, expected_column_resolutions] = generate_matchable_kql_expressions("", column_query_to_possible_matches); - auto const is_auto_generated = GENERATE(true, false); - std::vector matchable_kql_expressions_with_column_resolutions; - std::vector single_wildcard_kql_expressions; - auto const namespace_id{is_auto_generated ? cAutogenNamespace : cDefaultNamespace}; - for (auto const& matchable_expression : matchable_kql_expressions) { - auto const formatted_expression{fmt::format("{}{}", namespace_id, matchable_expression)}; - if (matchable_expression.starts_with("*:")) { - single_wildcard_kql_expressions.emplace_back(formatted_expression); - } else { - matchable_kql_expressions_with_column_resolutions.emplace_back(formatted_expression); - } - } - - SECTION("basic") { - auto const matchable_kql_query_str = GENERATE_COPY( - fmt::format( - "{}", - fmt::join(matchable_kql_expressions_with_column_resolutions, " OR ") - ), - fmt::format("{}", fmt::join(single_wildcard_kql_expressions, " OR ")) - ); - CAPTURE(matchable_kql_query_str); - - auto query_stream{std::istringstream{matchable_kql_query_str}}; + auto create_query_handler = [&](std::string const& query_str) -> QueryHandlerImpl { + auto query_stream{std::istringstream{query_str}}; auto query{clp_s::search::kql::parse_kql_expression(query_stream)}; auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true)}; @@ -568,7 +622,15 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search auto const node_id{optional_node_id.value()}; REQUIRE_FALSE(query_handler_impl .update_partially_resolved_columns( - is_auto_generated, + true, + locator, + node_id, + trivial_new_projected_schema_tree_node_callback + ) + .has_error()); + REQUIRE_FALSE(query_handler_impl + .update_partially_resolved_columns( + false, locator, node_id, trivial_new_projected_schema_tree_node_callback @@ -576,24 +638,99 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search .has_error()); } + return std::move(query_handler_impl); + }; + + SECTION("Basic test with a single matchable node-ID-value pair") { + std::vector matchable_kql_expressions_with_column_resolutions; + std::vector kql_expressions_with_unknown_namespace; + std::vector single_wildcard_kql_expressions; + + auto const is_auto_generated = GENERATE(true, false); + auto const namespace_id{is_auto_generated ? cAutogenNamespace : cDefaultNamespace}; + for (auto const& matchable_expression : matchable_kql_expressions) { + auto const formatted_expression{ + fmt::format("{}{}", namespace_id, matchable_expression) + }; + if (matchable_expression.starts_with("*:")) { + single_wildcard_kql_expressions.emplace_back(formatted_expression); + } else { + matchable_kql_expressions_with_column_resolutions.emplace_back( + formatted_expression + ); + kql_expressions_with_unknown_namespace.emplace_back( + fmt::format("{}{}", cReservedNamespace1, matchable_expression) + ); + } + } + + // Evaluate a single matchable node-ID-value pair against a list of queries. + // For queries consisting of chained AND expressions, the result can be either `Pruned` or + // `False`. Thus, `expected_evaluation_results` is a bitmask capturing all valid outcomes. + auto const [matchable_kql_query_str, expected_evaluation_results] = GENERATE_COPY( + std::make_pair( + fmt::format( + "{}", + fmt::join(matchable_kql_expressions_with_column_resolutions, " OR ") + ), + AstEvaluationResult::True + ), + std::make_pair( + fmt::format( + "{}", + fmt::join( + matchable_kql_expressions_with_column_resolutions, + " AND " + ) + ), + AstEvaluationResult::Pruned | AstEvaluationResult::False + ), + std::make_pair( + fmt::format("{}", fmt::join(single_wildcard_kql_expressions, " OR ")), + AstEvaluationResult::True + ), + std::make_pair( + fmt::format("{}", fmt::join(single_wildcard_kql_expressions, " AND ")), + AstEvaluationResult::Pruned | AstEvaluationResult::False + ), + std::make_pair( + fmt::format( + "{}", + fmt::join(kql_expressions_with_unknown_namespace, " OR ") + ), + AstEvaluationResult::Pruned + ), + std::make_pair( + fmt::format( + "{}", + fmt::join(kql_expressions_with_unknown_namespace, " AND ") + ), + AstEvaluationResult::Pruned + ) + ); + CAPTURE(matchable_kql_query_str); + CAPTURE(expected_evaluation_results); + + auto query_handler_impl{create_query_handler(matchable_kql_query_str)}; + for (auto const& locator : locators) { + auto const node_type{locator.get_type()}; + if (SchemaTree::Node::Type::UnstructuredArray == node_type + || SchemaTree::Node::Type::Obj == node_type) + { + // We skip these two types because: + // - the current implementation doesn't support `UnstructuredArray` + // - we don't generate matchable queries for `Obj`. + continue; + } + auto const optional_node_id{schema_tree->try_get_node_id(locator)}; REQUIRE(optional_node_id.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto const node_id{*optional_node_id}; CAPTURE(fmt::format("Testing Node ID: {}", node_id)); - auto const node_type{locator.get_type()}; - auto const matchable_values{get_matchable_values(node_type)}; - for (auto const& matchable_value : matchable_values) { - if (SchemaTree::Node::Type::UnstructuredArray == node_type - || SchemaTree::Node::Type::Obj == node_type) - { - // We skip these two types because: - // - the current implementation doesn't support `UnstructuredArray` - // - we don't generate matchable queries for `Obj`. - continue; - } + for (auto const& matchable_value : get_matchable_values(node_type)) { KeyValuePairLogEvent::NodeIdValuePairs const node_id_value_pairs{ {node_id, matchable_value} }; @@ -605,23 +742,172 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search is_auto_generated ? KeyValuePairLogEvent::NodeIdValuePairs{} : node_id_value_pairs }; - auto const kv_pair_log_event_result{KeyValuePairLogEvent::create( + auto const evaluation_result{get_query_evaluation_result( schema_tree, schema_tree, auto_gen_node_id_value_pairs, user_gen_node_id_value_pairs, - UtcOffset{0} + query_handler_impl )}; - REQUIRE_FALSE(kv_pair_log_event_result.has_error()); - auto const& kv_pair_log_event{kv_pair_log_event_result.value()}; - auto const evaluation_result{ - query_handler_impl.evaluate_kv_pair_log_event(kv_pair_log_event) - }; - REQUIRE_FALSE(evaluation_result.has_error()); - CAPTURE(evaluation_result.value()); - REQUIRE((evaluation_result.value() == AstEvaluationResult::True)); + CAPTURE(evaluation_result); + REQUIRE(((evaluation_result & expected_evaluation_results) != 0)); + } + } + } + + SECTION("Matchable node-ID-value pairs on both user-generated and auto-generated namespaces") { + std::vector matchable_kql_expression_pairs; + + for (auto const& matchable_expression : matchable_kql_expressions) { + if (matchable_expression.starts_with("*:")) { + // We ignore all single wildcard queries since they match both auto-gen and user-gen + // kv-pairs + continue; } + matchable_kql_expression_pairs.emplace_back( + fmt::format( + "({}{} AND {}{})", + cDefaultNamespace, + matchable_expression, + cAutogenNamespace, + matchable_expression + ) + ); } + + auto const kql_query_str{ + fmt::format("{}", fmt::join(matchable_kql_expression_pairs, " OR ")), + }; + CAPTURE(kql_query_str); + + auto query_handler_impl{create_query_handler(kql_query_str)}; + + for (auto const& locator : locators) { + auto const node_type{locator.get_type()}; + if (SchemaTree::Node::Type::UnstructuredArray == node_type + || SchemaTree::Node::Type::Obj == node_type) + { + continue; + } + auto const optional_node_id{schema_tree->try_get_node_id(locator)}; + REQUIRE(optional_node_id.has_value()); + // NOLINTNEXTLINE(bugprone-unchecked-optional-access) + auto const node_id{*optional_node_id}; + + auto const evaluation_result{get_query_evaluation_result( + schema_tree, + schema_tree, + {}, + {}, + query_handler_impl + )}; + CAPTURE(evaluation_result); + REQUIRE((evaluation_result == AstEvaluationResult::Pruned)); + + // NOTE: We use nested for loop to generated matchable/unmatchable values instead of + // using `GENERATE` since `GENERATE` in this case has a way worse performance (about + // 100 times slower). + for (auto const& matchable_value : get_matchable_values(node_type)) { + AstEvaluationResult evaluation_result{}; + evaluation_result = get_query_evaluation_result( + schema_tree, + schema_tree, + {{node_id, matchable_value}}, + {{node_id, matchable_value}}, + query_handler_impl + ); + CAPTURE(evaluation_result); + REQUIRE((evaluation_result == AstEvaluationResult::True)); + + for (auto const& unmatchable_value : get_unmatchable_values(node_type)) { + evaluation_result = get_query_evaluation_result( + schema_tree, + schema_tree, + {{node_id, unmatchable_value}}, + {{node_id, matchable_value}}, + query_handler_impl + ); + CAPTURE(evaluation_result); + REQUIRE((evaluation_result == AstEvaluationResult::False)); + + evaluation_result = get_query_evaluation_result( + schema_tree, + schema_tree, + {{node_id, matchable_value}}, + {{node_id, unmatchable_value}}, + query_handler_impl + ); + CAPTURE(evaluation_result); + REQUIRE((evaluation_result == AstEvaluationResult::False)); + + evaluation_result = get_query_evaluation_result( + schema_tree, + schema_tree, + {{node_id, unmatchable_value}}, + {{node_id, unmatchable_value}}, + query_handler_impl + ); + CAPTURE(evaluation_result); + REQUIRE((evaluation_result == AstEvaluationResult::False)); + } + } + } + } + + SECTION("Test array evaluation") { + // Array evaluation is not supported in the current implementation. To make the search still + // usable, array evaluation shouldn't fail but return `False` instead. + + /* + * Schema-tree with unstructured array: + * <0:root:Obj> + * | + * |-------------------------> <1:a:Obj> + * | | + * |--> <2:b:Int |--> <3:b:Obj> + * | | | + * |--> <12:a:Int> | |------------> <4:c:Obj> + * | | | | + * |--> <14:arr:UnstructuredArray> | |--> <5:d:Str> |--> <7:a:Str> + * | | | + * | |--> <6:d:Bool> |--> <8:d:Str> + * | | | + * | |--> <10:e:Obj> |--> <9:d:Float> + * | | + * |--> <13:b:Bool> |--> <11:f:Obj> + */ + constexpr std::string_view cArrayKeyName{"arr"}; + constexpr SchemaTree::Node::id_t cArrayNodeId{14}; + SchemaTree::NodeLocator const array_node_locator{ + SchemaTree::cRootId, + cArrayKeyName, + SchemaTree::Node::Type::UnstructuredArray + }; + REQUIRE((cArrayNodeId == schema_tree->insert_node(array_node_locator))); + auto const unstructured_array{fmt::format("[{}, {}]", cRefTestInt, cRefTestBool)}; + + auto const array_query{fmt::format("{}: {}", cArrayKeyName, cRefTestInt)}; + auto query_handler_impl{create_query_handler(array_query)}; + REQUIRE_FALSE(query_handler_impl + .update_partially_resolved_columns( + false, + array_node_locator, + cArrayNodeId, + trivial_new_projected_schema_tree_node_callback + ) + .has_error()); + auto const evaluation_result{get_query_evaluation_result( + schema_tree, + schema_tree, + {}, + {{cArrayNodeId, + Value{ + get_encoded_text_ast(unstructured_array) + }}}, + query_handler_impl + )}; + CAPTURE(evaluation_result); + REQUIRE((AstEvaluationResult::False == evaluation_result)); } } } // namespace clp::ffi::ir_stream::search::test diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp index eb14d10542..fa39bc83de 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_utils.cpp @@ -1,7 +1,5 @@ #include #include -#include -#include #include #include #include @@ -21,7 +19,6 @@ #include "../../../../../clp_s/search/ast/Integral.hpp" #include "../../../../../clp_s/search/ast/Literal.hpp" #include "../../../../../clp_s/search/ast/StringLiteral.hpp" -#include "../../../../ir/EncodedTextAst.hpp" #include "../../../../ir/types.hpp" #include "../../../Value.hpp" #include "../utils.hpp" @@ -34,9 +31,7 @@ using clp::ffi::value_bool_t; using clp::ffi::value_float_t; using clp::ffi::value_int_t; using clp::ir::eight_byte_encoded_variable_t; -using clp::ir::EightByteEncodedTextAst; using clp::ir::four_byte_encoded_variable_t; -using clp::ir::FourByteEncodedTextAst; using clp_s::search::ast::BooleanLiteral; using clp_s::search::ast::ColumnDescriptor; using clp_s::search::ast::Expression; diff --git a/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp b/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp index dcb798e0c6..371667f2a8 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp @@ -1,6 +1,8 @@ #ifndef CLP_FFI_IR_STREAM_SEARCH_TEST_UTILS_HPP #define CLP_FFI_IR_STREAM_SEARCH_TEST_UTILS_HPP +#include +#include #include #include #include @@ -14,6 +16,7 @@ #include #include "../../../../../clp_s/search/ast/Literal.hpp" +#include "../../../../ir/EncodedTextAst.hpp" #include "../../../../ir/types.hpp" #include "../../../encoding_methods.hpp" #include "../../../SchemaTree.hpp" From bfe587331a2fd5a8fb22037208094804bceb25ea Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Thu, 8 May 2025 11:16:25 -0400 Subject: [PATCH 11/25] Add inverter to the test --- .../search/test/test_QueryHandlerImpl.cpp | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 8dceaeb463..d6f68888c1 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -276,7 +276,7 @@ auto get_unmatchable_values(SchemaTree::Node::Type node_type) -> std::vector matchable_values; matchable_values.emplace_back(std::string{}); - std::string_view const unmatchable_long_str{"This is a static message"}; + std::string_view const unmatchable_long_str{"This is a static message: ID=0"}; REQUIRE((unmatchable_long_str.find(cRefTestStr) == std::string::npos)); matchable_values.emplace_back( get_encoded_text_ast(unmatchable_long_str) @@ -758,6 +758,13 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search SECTION("Matchable node-ID-value pairs on both user-generated and auto-generated namespaces") { std::vector matchable_kql_expression_pairs; + auto const with_inverter = GENERATE(true, false); + CAPTURE(with_inverter); + // NOTE: By applying De Morgan's law, `cExpressionWithInverter` should be equivalent to + // `cExpressionWithoutInverter`. + constexpr std::string_view cExpressionWithInverter{"NOT (NOT {}{} OR NOT {}{})"}; + constexpr std::string_view cExpressionWithoutInverter{"({}{} AND {}{})"}; + for (auto const& matchable_expression : matchable_kql_expressions) { if (matchable_expression.starts_with("*:")) { // We ignore all single wildcard queries since they match both auto-gen and user-gen @@ -765,18 +772,25 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search continue; } matchable_kql_expression_pairs.emplace_back( - fmt::format( - "({}{} AND {}{})", - cDefaultNamespace, - matchable_expression, - cAutogenNamespace, - matchable_expression - ) + with_inverter ? fmt::format( + cExpressionWithInverter, + cDefaultNamespace, + matchable_expression, + cAutogenNamespace, + matchable_expression + ) + : fmt::format( + cExpressionWithoutInverter, + cDefaultNamespace, + matchable_expression, + cAutogenNamespace, + matchable_expression + ) ); } auto const kql_query_str{ - fmt::format("{}", fmt::join(matchable_kql_expression_pairs, " OR ")), + fmt::format("{}", fmt::join(matchable_kql_expression_pairs, " OR ")) }; CAPTURE(kql_query_str); @@ -793,16 +807,17 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search REQUIRE(optional_node_id.has_value()); // NOLINTNEXTLINE(bugprone-unchecked-optional-access) auto const node_id{*optional_node_id}; + CAPTURE(node_id); - auto const evaluation_result{get_query_evaluation_result( + auto const pruned_evaluation_result{get_query_evaluation_result( schema_tree, schema_tree, {}, {}, query_handler_impl )}; - CAPTURE(evaluation_result); - REQUIRE((evaluation_result == AstEvaluationResult::Pruned)); + CAPTURE(pruned_evaluation_result); + REQUIRE((pruned_evaluation_result == AstEvaluationResult::Pruned)); // NOTE: We use nested for loop to generated matchable/unmatchable values instead of // using `GENERATE` since `GENERATE` in this case has a way worse performance (about From ed633bf603e62ad2513da72687d1d857fa057358 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 11 May 2025 00:44:29 -0400 Subject: [PATCH 12/25] Update test case. --- .../search/test/test_QueryHandlerImpl.cpp | 51 ++++++++----------- 1 file changed, 20 insertions(+), 31 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index d6f68888c1..351187b959 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -756,14 +756,8 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search } SECTION("Matchable node-ID-value pairs on both user-generated and auto-generated namespaces") { - std::vector matchable_kql_expression_pairs; - - auto const with_inverter = GENERATE(true, false); - CAPTURE(with_inverter); - // NOTE: By applying De Morgan's law, `cExpressionWithInverter` should be equivalent to - // `cExpressionWithoutInverter`. - constexpr std::string_view cExpressionWithInverter{"NOT (NOT {}{} OR NOT {}{})"}; - constexpr std::string_view cExpressionWithoutInverter{"({}{} AND {}{})"}; + std::vector xor_matchable_expressions; + constexpr std::string_view cXorExpression{"(({}{} AND NOT {}{}) OR (NOT {}{} AND {}{}))"}; for (auto const& matchable_expression : matchable_kql_expressions) { if (matchable_expression.starts_with("*:")) { @@ -771,27 +765,22 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search // kv-pairs continue; } - matchable_kql_expression_pairs.emplace_back( - with_inverter ? fmt::format( - cExpressionWithInverter, - cDefaultNamespace, - matchable_expression, - cAutogenNamespace, - matchable_expression - ) - : fmt::format( - cExpressionWithoutInverter, - cDefaultNamespace, - matchable_expression, - cAutogenNamespace, - matchable_expression - ) + xor_matchable_expressions.emplace_back( + fmt::format( + cXorExpression, + cDefaultNamespace, + matchable_expression, + cAutogenNamespace, + matchable_expression, + cDefaultNamespace, + matchable_expression, + cAutogenNamespace, + matchable_expression + ) ); } - auto const kql_query_str{ - fmt::format("{}", fmt::join(matchable_kql_expression_pairs, " OR ")) - }; + auto const kql_query_str{fmt::format("{}", fmt::join(xor_matchable_expressions, " OR "))}; CAPTURE(kql_query_str); auto query_handler_impl{create_query_handler(kql_query_str)}; @@ -817,7 +806,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search query_handler_impl )}; CAPTURE(pruned_evaluation_result); - REQUIRE((pruned_evaluation_result == AstEvaluationResult::Pruned)); + REQUIRE((AstEvaluationResult::Pruned == pruned_evaluation_result)); // NOTE: We use nested for loop to generated matchable/unmatchable values instead of // using `GENERATE` since `GENERATE` in this case has a way worse performance (about @@ -832,7 +821,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search query_handler_impl ); CAPTURE(evaluation_result); - REQUIRE((evaluation_result == AstEvaluationResult::True)); + REQUIRE((AstEvaluationResult::False == evaluation_result)); for (auto const& unmatchable_value : get_unmatchable_values(node_type)) { evaluation_result = get_query_evaluation_result( @@ -843,7 +832,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search query_handler_impl ); CAPTURE(evaluation_result); - REQUIRE((evaluation_result == AstEvaluationResult::False)); + REQUIRE((AstEvaluationResult::True == evaluation_result)); evaluation_result = get_query_evaluation_result( schema_tree, @@ -853,7 +842,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search query_handler_impl ); CAPTURE(evaluation_result); - REQUIRE((evaluation_result == AstEvaluationResult::False)); + REQUIRE((AstEvaluationResult::True == evaluation_result)); evaluation_result = get_query_evaluation_result( schema_tree, @@ -863,7 +852,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search query_handler_impl ); CAPTURE(evaluation_result); - REQUIRE((evaluation_result == AstEvaluationResult::False)); + REQUIRE((AstEvaluationResult::False == evaluation_result)); } } } From 8dda41bff5f8d4b0f48037d76fd0344a9b3123e4 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 11 May 2025 17:32:04 -0400 Subject: [PATCH 13/25] Add comments for xor. --- .../ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 351187b959..77f6a84e67 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -757,7 +757,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search SECTION("Matchable node-ID-value pairs on both user-generated and auto-generated namespaces") { std::vector xor_matchable_expressions; - constexpr std::string_view cXorExpression{"(({}{} AND NOT {}{}) OR (NOT {}{} AND {}{}))"}; + constexpr std::string_view cXorExpression{"(({} AND NOT @{}) OR (NOT {} AND @{}))"}; for (auto const& matchable_expression : matchable_kql_expressions) { if (matchable_expression.starts_with("*:")) { @@ -768,18 +768,17 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search xor_matchable_expressions.emplace_back( fmt::format( cXorExpression, - cDefaultNamespace, matchable_expression, - cAutogenNamespace, matchable_expression, - cDefaultNamespace, matchable_expression, - cAutogenNamespace, matchable_expression ) ); } + // Combine all XOR expressions into a single KQL query string joined by "OR". + // Each XOR expression matches either the default namespace or the auto-generated namespace + // of the same column, but not both. auto const kql_query_str{fmt::format("{}", fmt::join(xor_matchable_expressions, " OR "))}; CAPTURE(kql_query_str); From fd0c79ae595ddb72a9b3c4aaacf8306ed1797af2 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Sun, 11 May 2025 17:53:18 -0400 Subject: [PATCH 14/25] Update components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp Co-authored-by: Devin Gibson --- components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp index 135d5bbc86..12b351f7fc 100644 --- a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp @@ -23,7 +23,7 @@ auto ErrorCategory::message(ErrorCodeEnum error_enum) const -> std::string { return "Internal invariant violated during AST evaluation. This indicates a serious " "bug in the evaluation logic."; case ErrorCodeEnum::AttemptToIterateAstLeafExpr: - return "Attempted to iterate an leaf expression of an AST."; + return "Attempted to iterate a leaf expression of an AST."; case ErrorCodeEnum::ColumnDescriptorTokenIteratorOutOfBounds: return "Attempted to access a token beyond the end of the column descriptor."; case ErrorCodeEnum::ColumnTokenizationFailure: From d9d27b5e1258c45af0b4548ab043a517fb4e129b Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Sun, 11 May 2025 18:17:08 -0400 Subject: [PATCH 15/25] Apply code review comments. --- .../clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 4 ++-- .../ir_stream/search/test/test_QueryHandlerImpl.cpp | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index e3b1954852..fc451823ab 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -334,8 +334,8 @@ class QueryHandlerImpl { * @param log_event * @return A result containing the evaluation result on success, or an error code indicating the * failure: - * - ErrorCodeEnum::AstEvaluationInvariantViolation if the underlying column of the filter is - * neither user-generated nor auto-generated. + * - ErrorCodeEnum::AstEvaluationInvariantViolation if the underlying column of the filter has + * been resolved, but is neither user-generated nor auto-generated. * - Forwards `evaluate_wildcard_filter`'s return values. * - Forwards `evaluate_filter_against_node_id_value_pair`'s return values. */ diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 77f6a84e67..296dba2fc1 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -274,17 +274,17 @@ auto get_unmatchable_values(SchemaTree::Node::Type node_type) -> std::vector matchable_values; - matchable_values.emplace_back(std::string{}); + std::vector unmatchable_values; + unmatchable_values.emplace_back(std::string{}); std::string_view const unmatchable_long_str{"This is a static message: ID=0"}; REQUIRE((unmatchable_long_str.find(cRefTestStr) == std::string::npos)); - matchable_values.emplace_back( + unmatchable_values.emplace_back( get_encoded_text_ast(unmatchable_long_str) ); - matchable_values.emplace_back( + unmatchable_values.emplace_back( get_encoded_text_ast(unmatchable_long_str) ); - return matchable_values; + return unmatchable_values; } default: // Unsupported types @@ -610,6 +610,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search auto create_query_handler = [&](std::string const& query_str) -> QueryHandlerImpl { auto query_stream{std::istringstream{query_str}}; auto query{clp_s::search::kql::parse_kql_expression(query_stream)}; + REQUIRE((nullptr != query)); auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true)}; REQUIRE_FALSE(query_handler_impl_result.has_error()); @@ -638,7 +639,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search .has_error()); } - return std::move(query_handler_impl); + return std::move(query_handler_impl_result.value()); }; SECTION("Basic test with a single matchable node-ID-value pair") { From 1a6694d22384b16f4f7be8d8f2dc075ae679ee2e Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 12 May 2025 16:11:02 -0400 Subject: [PATCH 16/25] Fix doc strings. --- .../src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp | 9 +++++---- .../src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 4 ++-- .../core/src/clp/ffi/ir_stream/search/test/utils.hpp | 4 +++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index 9940dcb8e9..b565bb8dae 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -44,14 +44,15 @@ using clp_s::search::ast::LiteralTypeBitmask; /** * Pre-processes a search query by applying several transformation passes. * @param query - * @return A result containing the transformed + * @return A result containing the transformed query on success, or an error code indicating the + * failure: * - ErrorCodeEnum::QueryTransformationPassFailed if any of the transformation pass failed. */ [[nodiscard]] auto preprocess_query(std::shared_ptr query) -> outcome_v2::std_result>; /** - * Creates projected columns and column-to-original-key map from the given projections. + * Creates column descriptors and column-to-original-key map from the given projections. * @param projections * @return A result containing a pair or an error code indicating the failure: * - The pair: @@ -565,7 +566,7 @@ auto QueryHandlerImpl::evaluate_filter_expr( evaluation_results |= evaluation_result; } - if ((evaluation_results & AstEvaluationResult::False) != 0) { + if (0 != (evaluation_results & AstEvaluationResult::False)) { return AstEvaluationResult::False; } return AstEvaluationResult::Pruned; @@ -647,7 +648,7 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( push_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); return outcome_v2::success(); } - if ((evaluation_results & AstEvaluationResult::False) != 0) { + if (0 != (evaluation_results & AstEvaluationResult::False)) { pop_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::False, query_evaluation_result diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index fc451823ab..7a1ff2e4f5 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -217,8 +217,7 @@ class QueryHandlerImpl { * @return A result containing the iterator of the next child expression operator, or an * error code indicating the failure: * - ErrorCodeEnum::AttemptToIterateAstLeafExpr if the current expression is a leaf - * expression - * (`clp_s::search::ast::FilterExpr`). + * expression (`clp_s::search::ast::FilterExpr`). * - Forwards `create`'s return values. */ [[nodiscard]] auto next_op() -> std::optional>; @@ -277,6 +276,7 @@ class QueryHandlerImpl { m_op_end_it{op_end_it}, m_is_inverted{is_inverted} {} + // Variables ExprVariant m_expr; clp_s::search::ast::OpList::const_iterator m_op_next_it; clp_s::search::ast::OpList::const_iterator m_op_end_it; diff --git a/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp b/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp index 371667f2a8..376c285b14 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/utils.hpp @@ -70,7 +70,9 @@ class ColumnQueryPossibleMatches { * Trivial implementation of `NewProjectedSchemaTreeNodeCallback` that always return success without * doing anything. * @param is_auto_generated - * @param + * @param node_id + * @param projected_key_path + * @return A void result. */ [[nodiscard]] auto trivial_new_projected_schema_tree_node_callback( bool is_auto_generated, From b24af9cb6ffb00fbec7700b4002b56a4d02f1481 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 12 May 2025 16:52:14 -0400 Subject: [PATCH 17/25] Apply code review comments to move pruned check out of loop. --- .../search/test/test_QueryHandlerImpl.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 296dba2fc1..d7e0523e6d 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -785,6 +785,12 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search auto query_handler_impl{create_query_handler(kql_query_str)}; + auto const pruned_evaluation_result{ + get_query_evaluation_result(schema_tree, schema_tree, {}, {}, query_handler_impl) + }; + CAPTURE(pruned_evaluation_result); + REQUIRE((AstEvaluationResult::Pruned == pruned_evaluation_result)); + for (auto const& locator : locators) { auto const node_type{locator.get_type()}; if (SchemaTree::Node::Type::UnstructuredArray == node_type @@ -798,16 +804,6 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search auto const node_id{*optional_node_id}; CAPTURE(node_id); - auto const pruned_evaluation_result{get_query_evaluation_result( - schema_tree, - schema_tree, - {}, - {}, - query_handler_impl - )}; - CAPTURE(pruned_evaluation_result); - REQUIRE((AstEvaluationResult::Pruned == pruned_evaluation_result)); - // NOTE: We use nested for loop to generated matchable/unmatchable values instead of // using `GENERATE` since `GENERATE` in this case has a way worse performance (about // 100 times slower). From 57568f2d9b61f631b438c0271dd9a1b9b0a13c4f Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Mon, 12 May 2025 23:34:07 -0400 Subject: [PATCH 18/25] Fix docstrings. --- .../core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 6 +++--- .../clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 7a1ff2e4f5..71b22f7582 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -349,9 +349,9 @@ class QueryHandlerImpl { } /** - * Pops the AST DFS stack and update the evaluation accordingly: + * Pops the AST DFS stack and updates the evaluation result accordingly: * - If the stack if not empty, update the parent's evaluation results. - * - Otherwise, update `query_evaluation_results`. + * - Otherwise, update `query_evaluation_result`. * @param evaluation_result * @param query_evaluation_result Returns the query evaluation result. */ @@ -361,7 +361,7 @@ class QueryHandlerImpl { ) -> void; /** - * Advances AST DFA evaluation by visiting the top of `m_ast_dfs_stack`. + * Advances AST DFS evaluation by visiting the top of `m_ast_dfs_stack`. * @param log_event * @param query_evaluation_result Returns the query evaluation result. * @return A void result on success, or an error code indicating the failure: diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index d7e0523e6d..e1fd9ff982 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -90,13 +90,13 @@ constexpr value_bool_t cRefTestBool{false}; [[nodiscard]] auto get_unmatchable_values(SchemaTree::Node::Type node_type) -> std::vector; /** - * Gets the query evaluation results on the kv-pair log event constructed by the given node-ID-value - * pairs. * @param auto_gen_schema_tree * @param user_gen_schema_tree * @param auto_gen_node_id_value_pairs * @param user_gen_node_id_value_pairs * @param query_handler_impl + * @return The query evaluation result on the kv-pair log event constructed by the given + * schema-trees and node-ID-value pairs. */ [[nodiscard]] auto get_query_evaluation_result( std::shared_ptr auto_gen_schema_tree, From 2cad470be705e062b883a1601b15c8cae16697e6 Mon Sep 17 00:00:00 2001 From: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> Date: Tue, 13 May 2025 23:20:46 -0400 Subject: [PATCH 19/25] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- .../clp/ffi/ir_stream/search/ErrorCode.cpp | 4 ++-- .../clp/ffi/ir_stream/search/QueryHandler.hpp | 2 +- .../ffi/ir_stream/search/QueryHandlerImpl.hpp | 12 ++++++------ .../search/test/test_QueryHandlerImpl.cpp | 19 ++++++++++--------- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp index 12b351f7fc..8e8669786b 100644 --- a/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/ErrorCode.cpp @@ -20,8 +20,8 @@ auto ErrorCategory::message(ErrorCodeEnum error_enum) const -> std::string { case ErrorCodeEnum::AstDynamicCastFailure: return "Failed to dynamically cast an AST node to the expected type."; case ErrorCodeEnum::AstEvaluationInvariantViolation: - return "Internal invariant violated during AST evaluation. This indicates a serious " - "bug in the evaluation logic."; + return "Internal invariant violated during AST evaluation. This indicates a serious" + " bug in the evaluation logic."; case ErrorCodeEnum::AttemptToIterateAstLeafExpr: return "Attempted to iterate a leaf expression of an AST."; case ErrorCodeEnum::ColumnDescriptorTokenIteratorOutOfBounds: diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp index db55ddbe4a..de510f3011 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp @@ -94,7 +94,7 @@ class QueryHandler { * @param log_event * @return A result containing the evaluation result on success, or an error code indicating * the failure: - * - Forwards `QueryHandlerImpl::evaluate_node_id_value_pairs`'s return values. + * - Forwards `QueryHandlerImpl::evaluate_kv_pair_log_event`'s return values. */ [[nodiscard]] auto evaluate_kv_pair_log_event(KeyValuePairLogEvent const& log_event) -> outcome_v2::std_result { diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 71b22f7582..996a8d549c 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -349,9 +349,9 @@ class QueryHandlerImpl { } /** - * Pops the AST DFS stack and updates the evaluation result accordingly: - * - If the stack if not empty, update the parent's evaluation results. - * - Otherwise, update `query_evaluation_result`. + * Pops from the AST DFS stack and updates the evaluation result accordingly: + * - If the stack still has elements, updates the parent's evaluation results. + * - Otherwise, updates `query_evaluation_result`. * @param evaluation_result * @param query_evaluation_result Returns the query evaluation result. */ @@ -361,12 +361,12 @@ class QueryHandlerImpl { ) -> void; /** - * Advances AST DFS evaluation by visiting the top of `m_ast_dfs_stack`. + * Advances the AST DFS evaluation by visiting the top of `m_ast_dfs_stack`. * @param log_event * @param query_evaluation_result Returns the query evaluation result. * @return A void result on success, or an error code indicating the failure: - * - ErrorCodeEnum::AstEvaluationInvariantViolation if the expression iterator on the stack top - * is an unexpected expression type. + * - ErrorCodeEnum::AstEvaluationInvariantViolation if the expression iterator at the top of the + * stack is an unexpected expression type. * - Forwards `evaluate_filter_expr`'s return values. * - Forwards `AstExprIterator::next_op`'s return values. */ diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index e1fd9ff982..4c23c9f012 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -259,7 +259,8 @@ auto get_matchable_values(SchemaTree::Node::Type node_type) -> std::vector std::vector Date: Tue, 13 May 2025 23:21:26 -0400 Subject: [PATCH 20/25] Update components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- .../core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 996a8d549c..8db0eebdde 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -153,7 +153,6 @@ class QueryHandlerImpl { /** * Implementation of `QueryHandler::evaluate_kv_pair_log_event`. * @param log_event - * @param user_gen_node_id_value_pairs * @return A result containing the evaluation result on success, or an error code indicating * the failure: * - ErrorCodeEnum::AstEvaluationInvariantViolation if the underlying AST DFS evaluation doesn't From f32419f757cc6ac0b60aa802a00b431cd05c8ad2 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 13 May 2025 23:24:10 -0400 Subject: [PATCH 21/25] Rename result bitmask type. --- .../clp/ffi/ir_stream/search/AstEvaluationResult.hpp | 2 +- .../clp/ffi/ir_stream/search/QueryHandlerImpl.cpp | 4 ++-- .../clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 4 ++-- .../ir_stream/search/test/test_QueryHandlerImpl.cpp | 12 ++++++------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp b/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp index 5cf887b0f6..b3a7e0ef58 100644 --- a/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/AstEvaluationResult.hpp @@ -17,7 +17,7 @@ enum AstEvaluationResult : uint8_t { Pruned = 1 << 2, }; -using AstEvaluationResultBitmask = std::underlying_type_t; +using ast_evaluation_result_bitmask_t = std::underlying_type_t; } // namespace clp::ffi::ir_stream::search #endif // CLP_FFI_IR_STREAM_SEARCH_ASTEVALUATIONRESULT_HPP diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index b565bb8dae..dec5225e3d 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -382,7 +382,7 @@ auto evaluate_wildcard_filter( SchemaTree const& schema_tree, bool case_sensitive_match ) -> outcome_v2::std_result { - AstEvaluationResultBitmask evaluation_results{}; + ast_evaluation_result_bitmask_t evaluation_results{}; for (auto const& [node_id, value] : node_id_value_pairs) { auto const evaluation_result{OUTCOME_TRYX(evaluate_filter_against_node_id_value_pair( filter_expr, @@ -548,7 +548,7 @@ auto QueryHandlerImpl::evaluate_filter_expr( }; auto const& matchable_node_ids{m_resolved_column_to_schema_tree_node_ids.at(col)}; - AstEvaluationResultBitmask evaluation_results{}; + ast_evaluation_result_bitmask_t evaluation_results{}; for (auto const matchable_node_id : matchable_node_ids) { if (false == node_id_value_pairs.contains(matchable_node_id)) { continue; diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 8db0eebdde..90b3df92d3 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -344,7 +344,7 @@ class QueryHandlerImpl { ) -> outcome_v2::std_result; auto push_ast_dfs_stack(AstExprIterator ast_expr_it) -> void { - m_ast_dfs_stack.emplace_back(ast_expr_it, AstEvaluationResultBitmask{}); + m_ast_dfs_stack.emplace_back(ast_expr_it, ast_evaluation_result_bitmask_t{}); } /** @@ -386,7 +386,7 @@ class QueryHandlerImpl { std::vector> m_projected_columns; ProjectionMap m_projected_column_to_original_key; bool m_case_sensitive_match; - std::vector> m_ast_dfs_stack; + std::vector> m_ast_dfs_stack; }; template diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index 4c23c9f012..b0ab6767fb 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -671,14 +671,14 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search // For queries consisting of chained AND expressions, the result can be either `Pruned` or // `False`. Thus, `expected_evaluation_results` is a bitmask capturing all valid outcomes. auto const [matchable_kql_query_str, expected_evaluation_results] = GENERATE_COPY( - std::make_pair( + std::make_pair( fmt::format( "{}", fmt::join(matchable_kql_expressions_with_column_resolutions, " OR ") ), AstEvaluationResult::True ), - std::make_pair( + std::make_pair( fmt::format( "{}", fmt::join( @@ -688,22 +688,22 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search ), AstEvaluationResult::Pruned | AstEvaluationResult::False ), - std::make_pair( + std::make_pair( fmt::format("{}", fmt::join(single_wildcard_kql_expressions, " OR ")), AstEvaluationResult::True ), - std::make_pair( + std::make_pair( fmt::format("{}", fmt::join(single_wildcard_kql_expressions, " AND ")), AstEvaluationResult::Pruned | AstEvaluationResult::False ), - std::make_pair( + std::make_pair( fmt::format( "{}", fmt::join(kql_expressions_with_unknown_namespace, " OR ") ), AstEvaluationResult::Pruned ), - std::make_pair( + std::make_pair( fmt::format( "{}", fmt::join(kql_expressions_with_unknown_namespace, " AND ") From 66ed01f9fcc3140a9f871f8dcd23e21954647094 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 13 May 2025 23:26:17 -0400 Subject: [PATCH 22/25] Renaming push and pop according to the review comments. --- .../core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp | 6 +++--- .../core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index dec5225e3d..e361205e9e 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -438,7 +438,7 @@ auto QueryHandlerImpl::evaluate_kv_pair_log_event(KeyValuePairLogEvent const& lo std::optional optional_evaluation_result; m_ast_dfs_stack.clear(); - push_ast_dfs_stack(OUTCOME_TRYX(AstExprIterator::create(m_query.get()))); + push_to_ast_dfs_stack(OUTCOME_TRYX(AstExprIterator::create(m_query.get()))); while (false == m_ast_dfs_stack.empty()) { OUTCOME_TRYV(advance_ast_dfs_evaluation(log_event, optional_evaluation_result)); } @@ -621,7 +621,7 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( } auto const optional_next_op_it{expr_it.next_op()}; if (optional_next_op_it.has_value()) { - push_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); + push_to_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); } else { pop_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::True, @@ -645,7 +645,7 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( } auto const optional_next_op_it{expr_it.next_op()}; if (optional_next_op_it.has_value()) { - push_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); + push_to_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); return outcome_v2::success(); } if (0 != (evaluation_results & AstEvaluationResult::False)) { diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 90b3df92d3..2291dcf71c 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -343,7 +343,7 @@ class QueryHandlerImpl { KeyValuePairLogEvent const& log_event ) -> outcome_v2::std_result; - auto push_ast_dfs_stack(AstExprIterator ast_expr_it) -> void { + auto push_to_ast_dfs_stack(AstExprIterator ast_expr_it) -> void { m_ast_dfs_stack.emplace_back(ast_expr_it, ast_evaluation_result_bitmask_t{}); } @@ -354,7 +354,7 @@ class QueryHandlerImpl { * @param evaluation_result * @param query_evaluation_result Returns the query evaluation result. */ - auto pop_ast_dfs_stack_and_update_evaluation_results( + auto pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult evaluation_result, std::optional& query_evaluation_result ) -> void; From 60679988cfd9953b51c848491f07588a2f3b958b Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 13 May 2025 23:36:15 -0400 Subject: [PATCH 23/25] Fix docstring. --- .../src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp index 2291dcf71c..d98deab134 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp @@ -213,11 +213,13 @@ class QueryHandlerImpl { // Methods /** - * @return A result containing the iterator of the next child expression operator, or an - * error code indicating the failure: + * Retrieves the next child expression operator to visit as an `AstExprIterator`. + * @return A result containing the next `AstExprIterator` to visit on success, or an error + * code indicating the failure: * - ErrorCodeEnum::AttemptToIterateAstLeafExpr if the current expression is a leaf - * expression (`clp_s::search::ast::FilterExpr`). + * expression (`clp_s::search::ast::FilterExpr`) with no child operators. * - Forwards `create`'s return values. + * @return std::nullopt if there are no more child operators to visit. */ [[nodiscard]] auto next_op() -> std::optional>; From ea1a28c7547e711bc95a7dd44f8250cb563205e4 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 13 May 2025 23:38:08 -0400 Subject: [PATCH 24/25] Use constexpr. --- .../ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp index b0ab6767fb..a4670fefd9 100644 --- a/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/test/test_QueryHandlerImpl.cpp @@ -277,13 +277,13 @@ auto get_unmatchable_values(SchemaTree::Node::Type node_type) -> std::vector unmatchable_values; unmatchable_values.emplace_back(std::string{}); - std::string_view const unmatchable_long_str{"This is a static message: ID=0"}; - REQUIRE((unmatchable_long_str.find(cRefTestStr) == std::string::npos)); + constexpr std::string_view cUnmatchableLongStr{"This is a static message: ID=0"}; + REQUIRE((cUnmatchableLongStr.find(cRefTestStr) == std::string::npos)); unmatchable_values.emplace_back( - get_encoded_text_ast(unmatchable_long_str) + get_encoded_text_ast(cUnmatchableLongStr) ); unmatchable_values.emplace_back( - get_encoded_text_ast(unmatchable_long_str) + get_encoded_text_ast(cUnmatchableLongStr) ); return unmatchable_values; } From 620abca253f6bd3daa92dbd1fdbaadb416764309 Mon Sep 17 00:00:00 2001 From: LinZhihao-723 Date: Tue, 13 May 2025 23:54:10 -0400 Subject: [PATCH 25/25] Fix... --- .../ffi/ir_stream/search/QueryHandlerImpl.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp index e361205e9e..b8708bc671 100644 --- a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp +++ b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp @@ -572,7 +572,7 @@ auto QueryHandlerImpl::evaluate_filter_expr( return AstEvaluationResult::Pruned; } -auto QueryHandlerImpl::pop_ast_dfs_stack_and_update_evaluation_results( +auto QueryHandlerImpl::pop_from_ast_dfs_stack_and_update_evaluation_results( clp::ffi::ir_stream::search::AstEvaluationResult evaluation_result, std::optional& query_evaluation_result ) -> void { @@ -596,7 +596,7 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( ) -> outcome_v2::std_result { auto& [expr_it, evaluation_results] = m_ast_dfs_stack.back(); if (auto* filter_expr{expr_it.as_filter_expr()}; nullptr != filter_expr) { - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( OUTCOME_TRYX(evaluate_filter_expr(filter_expr, log_event)), query_evaluation_result ); @@ -606,14 +606,14 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( if (auto const* and_expr{expr_it.as_and_expr()}; nullptr != and_expr) { // Handle `AndExpr` evaluation if (0 != (evaluation_results & AstEvaluationResult::Pruned)) { - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::Pruned, query_evaluation_result ); return outcome_v2::success(); } if (0 != (evaluation_results & AstEvaluationResult::False)) { - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::False, query_evaluation_result ); @@ -623,7 +623,7 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( if (optional_next_op_it.has_value()) { push_to_ast_dfs_stack(OUTCOME_TRYX(optional_next_op_it.value())); } else { - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::True, query_evaluation_result ); @@ -637,7 +637,7 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( return ErrorCode{ErrorCodeEnum::AstEvaluationInvariantViolation}; } if (0 != (evaluation_results & AstEvaluationResult::True)) { - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::True, query_evaluation_result ); @@ -649,14 +649,15 @@ auto QueryHandlerImpl::advance_ast_dfs_evaluation( return outcome_v2::success(); } if (0 != (evaluation_results & AstEvaluationResult::False)) { - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::False, query_evaluation_result ); return outcome_v2::success(); } + // All pruned - pop_ast_dfs_stack_and_update_evaluation_results( + pop_from_ast_dfs_stack_and_update_evaluation_results( AstEvaluationResult::Pruned, query_evaluation_result );