feat(kv-ir): Add support for duplicate columns in projections. (#1245)

gibber9809 · LinZhihao-723 · web-flow · commit a8444fffdce6 · 2025-09-03T08:40:57.000-04:00
Co-authored-by: Lin Zhihao &lt;59785146+LinZhihao-723@users.noreply.github.com&gt;
diff --git a/components/core/src/clp/ffi/ir_stream/search/NewProjectedSchemaTreeNodeCallbackReq.hpp b/components/core/src/clp/ffi/ir_stream/search/NewProjectedSchemaTreeNodeCallbackReq.hpp
@@ -1,8 +1,10 @@
 #ifndef CLP_FFI_IR_STREAM_SEARCH_NEWPROJECTEDSCHEMATREENODECALLBACKREQ_HPP
 #define CLP_FFI_IR_STREAM_SEARCH_NEWPROJECTEDSCHEMATREENODECALLBACKREQ_HPP
 
+#include <cstddef>
 #include <string_view>
 #include <type_traits>
+#include <utility>
 
 #include <ystdlib/error_handling/Result.hpp>
 
@@ -18,7 +20,7 @@ namespace clp::ffi::ir_stream::search {
  * @tparam NewProjectedSchemaTreeNodeCallbackType
  * @param arg_0 Whether the schema-tree node is for an auto-generated kv-pair.
  * @param arg_1 The ID of the schema-tree node.
- * @param arg_2 The projected key path.
+ * @param arg_2 The projected key path and its original index in the list of projections.
  * @return A void result on success or a user-defined error code indicating the failure.
  */
 template <typename NewProjectedSchemaTreeNodeCallbackType>
@@ -27,7 +29,7 @@ concept NewProjectedSchemaTreeNodeCallbackReq = std::is_invocable_r_v<
         NewProjectedSchemaTreeNodeCallbackType,
         bool,
         SchemaTree::Node::id_t,
-        std::string_view>;
+        std::pair<std::string_view, size_t>>;
 }  // namespace clp::ffi::ir_stream::search
 
 #endif  // CLP_FFI_IR_STREAM_SEARCH_NEWPROJECTEDSCHEMATREENODECALLBACKREQ_HPP
diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandler.hpp
@@ -32,7 +32,11 @@ class QueryHandler {
 public:
     // Factory function
     /**
-     * @param new_projected_schema_tree_node_callback
+     * @param new_projected_schema_tree_node_callback Callback for newly resolved projections.
+     * @param query The search query.
+     * @param projections The columns to project.
+     * @param case_sensitive_match Whether to use case-sensitive match for string comparison.
+     * @param allow_duplicate_projected_columns Whether to allow duplicate projected columns.
      * @return A result containing the newly constructed `QueryHandler` on success, or an error code
      * indicating the failure:
      * - Forwards `QueryHandlerImpl::create`'s return values.
@@ -42,15 +46,17 @@ class QueryHandler {
             std::shared_ptr<clp_s::search::ast::Expression> query,
             std::vector<std::pair<std::string, clp_s::search::ast::literal_type_bitmask_t>> const&
                     projections,
-            bool case_sensitive_match
+            bool case_sensitive_match,
+            bool allow_duplicate_projected_columns = false
     ) -> ystdlib::error_handling::Result<QueryHandler> {
         return QueryHandler{
                 new_projected_schema_tree_node_callback,
                 YSTDLIB_ERROR_HANDLING_TRYX(
                         QueryHandlerImpl::create(
                                 std::move(query),
                                 projections,
-                                case_sensitive_match
+                                case_sensitive_match,
+                                allow_duplicate_projected_columns
                         )
                 )
         };
diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.cpp
@@ -1,5 +1,6 @@
 #include "QueryHandlerImpl.hpp"
 
+#include <cstddef>
 #include <exception>
 #include <memory>
 #include <optional>
@@ -54,6 +55,7 @@ using clp_s::search::ast::literal_type_bitmask_t;
 /**
  * Creates column descriptors and column-to-original-key map from the given projections.
  * @param projections
+ * @param allow_duplicate_projected_columns Whether to allow duplicate projected columns.
  * @return A result containing a pair or an error code indicating the failure:
  * - The pair:
  *   - A vector of projected columns.
@@ -65,7 +67,8 @@ using clp_s::search::ast::literal_type_bitmask_t;
  *     descriptor for the projected key.
  */
 [[nodiscard]] auto create_projected_columns_and_projection_map(
-        std::vector<std::pair<std::string, literal_type_bitmask_t>> const& projections
+        std::vector<std::pair<std::string, literal_type_bitmask_t>> const& projections,
+        bool allow_duplicate_projected_columns
 )
         -> ystdlib::error_handling::Result<std::pair<
                 std::vector<std::shared_ptr<ColumnDescriptor>>,
@@ -74,7 +77,7 @@ using clp_s::search::ast::literal_type_bitmask_t;
 /**
  * Creates initial partial resolutions for the given query and projections.
  * @param query
- * @param projected_column_to_original_key
+ * @param projected_column_to_original_key_and_index
  * @return A result containing a pair or an error code indicating the failure:
  * - The pair:
  *   - The partial resolution for auto-generated keys.
@@ -86,7 +89,7 @@ using clp_s::search::ast::literal_type_bitmask_t;
  */
 [[nodiscard]] auto create_initial_partial_resolutions(
         std::shared_ptr<Expression> const& query,
-        QueryHandlerImpl::ProjectionMap const& projected_column_to_original_key
+        QueryHandlerImpl::ProjectionMap const& projected_column_to_original_key_and_index
 )
         -> ystdlib::error_handling::Result<std::pair<
                 QueryHandlerImpl::PartialResolutionMap,
@@ -182,20 +185,24 @@ auto preprocess_query(std::shared_ptr<Expression> query)
 }
 
 auto create_projected_columns_and_projection_map(
-        std::vector<std::pair<std::string, literal_type_bitmask_t>> const& projections
+        std::vector<std::pair<std::string, literal_type_bitmask_t>> const& projections,
+        bool allow_duplicate_projected_columns
 )
         -> ystdlib::error_handling::Result<std::pair<
                 std::vector<std::shared_ptr<ColumnDescriptor>>,
                 QueryHandlerImpl::ProjectionMap>> {
     std::unordered_set<std::string_view> unique_projected_columns;
     std::vector<std::shared_ptr<ColumnDescriptor>> projected_columns;
-    QueryHandlerImpl::ProjectionMap projected_column_to_original_key;
+    QueryHandlerImpl::ProjectionMap projected_column_to_original_key_and_index;
 
-    for (auto const& [key, types] : projections) {
-        if (unique_projected_columns.contains(key)) {
-            return ErrorCode{ErrorCodeEnum::DuplicateProjectedColumn};
+    for (size_t index{0}; index < projections.size(); ++index) {
+        auto const& [key, types] = projections[index];
+        if (false == allow_duplicate_projected_columns) {
+            if (unique_projected_columns.contains(key)) {
+                return ErrorCode{ErrorCodeEnum::DuplicateProjectedColumn};
+            }
+            unique_projected_columns.emplace(key);
         }
-        unique_projected_columns.emplace(key);
 
         std::vector<std::string> descriptor_tokens;
         std::string descriptor_namespace;
@@ -220,26 +227,29 @@ auto create_projected_columns_and_projection_map(
             {
                 return ErrorCode{ErrorCodeEnum::ProjectionColumnDescriptorCreationFailure};
             }
-            projected_column_to_original_key.emplace(column_descriptor.get(), key);
+            projected_column_to_original_key_and_index.emplace(
+                    column_descriptor.get(),
+                    std::make_pair(key, index)
+            );
             projected_columns.emplace_back(std::move(column_descriptor));
         } catch (std::exception const& e) {
             return ErrorCode{ErrorCodeEnum::ProjectionColumnDescriptorCreationFailure};
         }
     }
 
-    return {std::move(projected_columns), std::move(projected_column_to_original_key)};
+    return {std::move(projected_columns), std::move(projected_column_to_original_key_and_index)};
 }
 
 auto create_initial_partial_resolutions(
         std::shared_ptr<Expression> const& query,
-        QueryHandlerImpl::ProjectionMap const& projected_column_to_original_key
+        QueryHandlerImpl::ProjectionMap const& projected_column_to_original_key_and_index
 )
         -> ystdlib::error_handling::Result<std::pair<
                 QueryHandlerImpl::PartialResolutionMap,
                 QueryHandlerImpl::PartialResolutionMap>> {
     QueryHandlerImpl::PartialResolutionMap auto_gen_namespace_partial_resolutions;
     QueryHandlerImpl::PartialResolutionMap user_gen_namespace_partial_resolutions;
-    for (auto const& [col, key] : projected_column_to_original_key) {
+    for (auto const& [col, _] : projected_column_to_original_key_and_index) {
         auto const optional_is_auto_gen{is_auto_generated(col->get_namespace())};
         if (false == optional_is_auto_gen.has_value()) {
             // Ignore unrecognized namespace
@@ -408,22 +418,27 @@ auto evaluate_wildcard_filter(
 auto QueryHandlerImpl::create(
         std::shared_ptr<Expression> query,
         std::vector<std::pair<std::string, literal_type_bitmask_t>> const& projections,
-        bool case_sensitive_match
+        bool case_sensitive_match,
+        bool allow_duplicate_projected_columns
 ) -> ystdlib::error_handling::Result<QueryHandlerImpl> {
     query = YSTDLIB_ERROR_HANDLING_TRYX(preprocess_query(query));
-    auto [projected_columns, projected_column_to_original_key]
-            = YSTDLIB_ERROR_HANDLING_TRYX(create_projected_columns_and_projection_map(projections));
+    auto [projected_columns, projected_column_to_original_key_and_index]
+            = YSTDLIB_ERROR_HANDLING_TRYX(create_projected_columns_and_projection_map(
+                    projections,
+                    allow_duplicate_projected_columns
+            ));
     auto [auto_gen_namespace_partial_resolutions, user_gen_namespace_partial_resolutions]
-            = YSTDLIB_ERROR_HANDLING_TRYX(
-                    create_initial_partial_resolutions(query, projected_column_to_original_key)
-            );
+            = YSTDLIB_ERROR_HANDLING_TRYX(create_initial_partial_resolutions(
+                    query,
+                    projected_column_to_original_key_and_index
+            ));
 
     return QueryHandlerImpl{
             std::move(query),
             std::move(auto_gen_namespace_partial_resolutions),
             std::move(user_gen_namespace_partial_resolutions),
             std::move(projected_columns),
-            std::move(projected_column_to_original_key),
+            std::move(projected_column_to_original_key_and_index),
             case_sensitive_match
     };
 }
diff --git a/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp b/components/core/src/clp/ffi/ir_stream/search/QueryHandlerImpl.hpp
@@ -1,6 +1,7 @@
 #ifndef CLP_FFI_IR_STREAM_SEARCH_QUERYHANDLERIMPL_HPP
 #define CLP_FFI_IR_STREAM_SEARCH_QUERYHANDLERIMPL_HPP
 
+#include <cstddef>
 #include <memory>
 #include <optional>
 #include <string>
@@ -117,7 +118,8 @@ class QueryHandlerImpl {
         clp_s::search::ast::DescriptorList::iterator m_next_token_it;
     };
 
-    using ProjectionMap = std::unordered_map<clp_s::search::ast::ColumnDescriptor*, std::string>;
+    using ProjectionMap = std::
+            unordered_map<clp_s::search::ast::ColumnDescriptor*, std::pair<std::string, size_t>>;
 
     using PartialResolutionMap = std::
             unordered_map<SchemaTree::Node::id_t, std::vector<ColumnDescriptorTokenIterator>>;
@@ -127,6 +129,7 @@ class QueryHandlerImpl {
      * @param query The search query.
      * @param projections The columns to project.
      * @param case_sensitive_match Whether to use case-sensitive match for string comparison.
+     * @param allow_duplicate_projected_columns Whether to allow duplicate projected columns.
      * @return A result containing the newly constructed `QueryHandler` on success, or an error code
      * indicating the failure:
      * - Forwards `preprocess_query`'s return values.
@@ -137,7 +140,8 @@ class QueryHandlerImpl {
             std::shared_ptr<clp_s::search::ast::Expression> query,
             std::vector<std::pair<std::string, clp_s::search::ast::literal_type_bitmask_t>> const&
                     projections,
-            bool case_sensitive_match
+            bool case_sensitive_match,
+            bool allow_duplicate_projected_columns
     ) -> ystdlib::error_handling::Result<QueryHandlerImpl>;
 
     // Delete copy constructor and assignment operator
@@ -292,7 +296,7 @@ class QueryHandlerImpl {
             PartialResolutionMap auto_gen_namespace_partial_resolutions,
             PartialResolutionMap user_gen_namespace_partial_resolutions,
             std::vector<std::shared_ptr<clp_s::search::ast::ColumnDescriptor>> projected_columns,
-            ProjectionMap projected_column_to_original_key,
+            ProjectionMap projected_column_to_original_key_and_index,
             bool case_sensitive_match
     )
             : m_query{std::move(query)},
@@ -306,7 +310,9 @@ class QueryHandlerImpl {
                       std::move(user_gen_namespace_partial_resolutions)
               },
               m_projected_columns{std::move(projected_columns)},
-              m_projected_column_to_original_key{std::move(projected_column_to_original_key)},
+              m_projected_column_to_original_key_and_index{
+                      std::move(projected_column_to_original_key_and_index)
+              },
               m_case_sensitive_match{case_sensitive_match} {}
 
     // Methods
@@ -388,7 +394,7 @@ class QueryHandlerImpl {
             std::unordered_set<SchemaTree::Node::id_t>>
             m_resolved_column_to_schema_tree_node_ids;
     std::vector<std::shared_ptr<clp_s::search::ast::ColumnDescriptor>> m_projected_columns;
-    ProjectionMap m_projected_column_to_original_key;
+    ProjectionMap m_projected_column_to_original_key_and_index;
     bool m_case_sensitive_match;
     std::vector<std::pair<AstExprIterator, ast_evaluation_result_bitmask_t>> m_ast_dfs_stack;
 };
@@ -492,11 +498,13 @@ auto QueryHandlerImpl::handle_column_resolution_on_new_schema_tree_node(
     }
 
     auto* col{token_it.get_column_descriptor()};
-    if (m_projected_column_to_original_key.contains(col)) {
+    auto const original_key_and_index_it = m_projected_column_to_original_key_and_index.find(col);
+    if (m_projected_column_to_original_key_and_index.end() != original_key_and_index_it) {
+        auto const& [original_key, projected_index] = original_key_and_index_it->second;
         YSTDLIB_ERROR_HANDLING_TRYV(new_projected_schema_tree_node_callback(
                 is_auto_generated,
                 node_id,
-                m_projected_column_to_original_key.at(col)
+                std::make_pair(original_key, projected_index)
         ));
         return ystdlib::error_handling::success();
     }
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
@@ -1,3 +1,4 @@
+#include <cstddef>
 #include <map>
 #include <memory>
 #include <set>
@@ -410,7 +411,7 @@ TEST_CASE(
     auto query_stream{std::istringstream{kql_query_str}};
     auto query{clp_s::search::kql::parse_kql_expression(query_stream)};
 
-    auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true)};
+    auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true, false)};
     REQUIRE_FALSE(query_handler_impl_result.has_error());
     auto& query_handler_impl{query_handler_impl_result.value()};
 
@@ -521,19 +522,21 @@ TEST_CASE("query_handler_handle_projection", "[ffi][ir_stream][search][QueryHand
             unresolvable_projections_from_unrecognized_namespaces.cend()
     );
 
-    auto query_handler_impl_result{QueryHandlerImpl::create(null_query, projections, true)};
+    auto query_handler_impl_result{QueryHandlerImpl::create(null_query, projections, true, false)};
     REQUIRE_FALSE(query_handler_impl_result.has_error());
     auto& query_handler_impl{query_handler_impl_result.value()};
 
     SchemaTree::Node::id_t node_id{1};
     std::map<std::string, std::set<SchemaTree::Node::id_t>> actual_resolved_projections;
     auto new_projected_schema_tree_node_callback
-            = [&](bool is_auto_gen,
-                  SchemaTree::Node::id_t node_id,
-                  std::string_view key) -> ystdlib::error_handling::Result<void> {
+            = [&](
+                      bool is_auto_gen,
+                      SchemaTree::Node::id_t node_id,
+                      std::pair<std::string_view, size_t> key_and_index
+              ) -> ystdlib::error_handling::Result<void> {
         REQUIRE((is_auto_generated == is_auto_gen));
         auto [column_it, column_inserted] = actual_resolved_projections.try_emplace(
-                std::string{key},
+                std::string{key_and_index.first},
                 std::set<SchemaTree::Node::id_t>{}
         );
         auto [node_id_it, node_id_inserted] = column_it->second.emplace(node_id);
@@ -615,7 +618,7 @@ TEST_CASE("query_handler_evaluation_kv_pair_log_event", "[ffi][ir_stream][search
         auto query{clp_s::search::kql::parse_kql_expression(query_stream)};
         REQUIRE((nullptr != query));
 
-        auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true)};
+        auto query_handler_impl_result{QueryHandlerImpl::create(query, {}, true, false)};
         REQUIRE_FALSE(query_handler_impl_result.has_error());
         auto& query_handler_impl{query_handler_impl_result.value()};
 
diff --git a/components/core/src/clp/ffi/ir_stream/search/test/utils.cpp b/components/core/src/clp/ffi/ir_stream/search/test/utils.cpp
@@ -67,7 +67,7 @@ auto ColumnQueryPossibleMatches::serialize() const -> std::string {
 auto trivial_new_projected_schema_tree_node_callback(
         [[maybe_unused]] bool is_auto_generated,
         [[maybe_unused]] SchemaTree::Node::id_t node_id,
-        [[maybe_unused]] std::string_view projected_key_path
+        [[maybe_unused]] std::pair<std::string_view, size_t> projected_key_path_and_index
 ) -> ystdlib::error_handling::Result<void> {
     return ystdlib::error_handling::success();
 }
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
@@ -74,13 +74,13 @@ class ColumnQueryPossibleMatches {
  * doing anything.
  * @param is_auto_generated
  * @param node_id
- * @param projected_key_path
+ * @param projected_key_path_and_index
  * @return A void result.
  */
 [[nodiscard]] auto trivial_new_projected_schema_tree_node_callback(
         bool is_auto_generated,
         SchemaTree::Node::id_t node_id,
-        std::string_view projected_key_path
+        std::pair<std::string_view, size_t> projected_key_path_and_index
 ) -> ystdlib::error_handling::Result<void>;
 
 /**
diff --git a/components/core/src/clp_s/kv_ir_search.cpp b/components/core/src/clp_s/kv_ir_search.cpp
@@ -200,7 +200,7 @@ auto deserialize_and_search_kv_ir_stream(
     auto trivial_new_projected_schema_tree_node_callback
             = []([[maybe_unused]] bool is_auto_generated,
                  [[maybe_unused]] SchemaTree::Node::id_t node_id,
-                 [[maybe_unused]] std::string_view projected_key_path)
+                 [[maybe_unused]] std::pair<std::string_view, size_t> projected_key_and_index)
             -> ystdlib::error_handling::Result<void> { return ystdlib::error_handling::success(); };
     using QueryHandlerType = clp::ffi::ir_stream::search::QueryHandler<
             decltype(trivial_new_projected_schema_tree_node_callback)>;

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ auto ColumnQueryPossibleMatches::serialize() const -> std::string {`
`67`	`67`	`auto trivial_new_projected_schema_tree_node_callback(`
`68`	`68`	`[[maybe_unused]] bool is_auto_generated,`
`69`	`69`	`[[maybe_unused]] SchemaTree::Node::id_t node_id,`
`70`		`- [[maybe_unused]] std::string_view projected_key_path`
	`70`	`+ [[maybe_unused]] std::pair<std::string_view, size_t> projected_key_path_and_index`
`71`	`71`	`) -> ystdlib::error_handling::Result<void> {`
`72`	`72`	`return ystdlib::error_handling::success();`
`73`	`73`	`}`