fix(clp-s): Handle pure wildcards and unexpected literal types correctly in EvaluateTimestampIndex (fixes #1096). (#1277)

Co-authored-by: kirkrodrigues <[email protected]>

Author: gibber9809

components/core/src/clp_s/search/EvaluateTimestampIndex.cpp

@@ -3,13 +3,15 @@
 #include "ast/AndExpr.hpp"
 #include "ast/Expression.hpp"
 #include "ast/FilterExpr.hpp"
+#include "ast/FilterOperation.hpp"
 #include "ast/Integral.hpp"
 #include "ast/Literal.hpp"
 #include "ast/OrExpr.hpp"
 
 using clp_s::search::ast::AndExpr;
 using clp_s::search::ast::Expression;
 using clp_s::search::ast::FilterExpr;
+using clp_s::search::ast::FilterOperation;
 using clp_s::search::ast::Integral;
 using clp_s::search::ast::Integral64;
 using clp_s::search::ast::literal_type_bitmask_t;
@@ -63,16 +65,19 @@ EvaluatedValue EvaluateTimestampIndex::run(std::shared_ptr<Expression> const& ex
              range_it != m_timestamp_dict->tokenized_column_to_range_end();
              range_it++)
         {
+            // Don't attempt to evaluate the timestamp index against columns with wildcard tokens.
+            if (column->is_unresolved_descriptor()) {
+                continue;
+            }
+
             std::vector<std::string> const& tokens = range_it->first;
             auto const& descriptors = column->get_descriptor_list();
-            // TODO: handle wildcard matching; the initial check on timestamp index happens
-            // before schema matching, so
             if (tokens.size() != descriptors.size()) {
                 continue;
             }
 
             bool matched = true;
-            for (size_t i = 0; i < descriptors.size(); ++i) {
+            for (size_t i{0ULL}; i < descriptors.size(); ++i) {
                 if (tokens[i] != descriptors[i].get_token()) {
                     matched = false;
                     break;
@@ -82,20 +87,21 @@ EvaluatedValue EvaluateTimestampIndex::run(std::shared_ptr<Expression> const& ex
                 continue;
             }
 
-            EvaluatedValue ret;
-            // this is safe after type narrowing because all DateType literals are either
-            // Integral or a derived class of Integral
-            Integral64 literal = std::static_pointer_cast<Integral>(filter->get_operand())->get();
-            if (std::holds_alternative<int64_t>(literal)) {
-                ret = range_it->second->evaluate_filter(
-                        filter->get_operation(),
-                        std::get<int64_t>(literal)
-                );
+            auto const literal{filter->get_operand()};
+            if (nullptr == literal) {
+                return EvaluatedValue::Unknown;
+            }
+
+            EvaluatedValue ret{EvaluatedValue::Unknown};
+            auto const filter_op{filter->get_operation()};
+            int64_t int_literal{};
+            double float_literal{};
+            if (literal->as_int(int_literal, filter_op)) {
+                ret = range_it->second->evaluate_filter(filter_op, int_literal);
+            } else if (literal->as_float(float_literal, filter_op)) {
+                ret = range_it->second->evaluate_filter(filter_op, float_literal);
             } else {
-                ret = range_it->second->evaluate_filter(
-                        filter->get_operation(),
-                        std::get<double>(literal)
-                );
+                return EvaluatedValue::Unknown;
             }
 
             if (ret == EvaluatedValue::True) {

components/core/tests/clp_s_test_utils.cpp

@@ -1,6 +1,7 @@
 #include "clp_s_test_utils.hpp"
 
 #include <filesystem>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -13,6 +14,7 @@
 auto compress_archive(
         std::string const& file_path,
         std::string const& archive_directory,
+        std::optional<std::string> timestamp_key,
         bool single_file_archive,
         bool structurize_arrays,
         clp_s::FileType file_type
@@ -39,6 +41,9 @@ auto compress_archive(
     parser_option.structurize_arrays = structurize_arrays;
     parser_option.single_file_archive = single_file_archive;
     parser_option.input_file_type = file_type;
+    if (timestamp_key.has_value()) {
+        parser_option.timestamp_key = std::move(timestamp_key.value());
+    }
 
     clp_s::JsonParser parser{parser_option};
     std::vector<clp_s::ArchiveStats> archive_stats;

components/core/tests/clp_s_test_utils.hpp

@@ -1,6 +1,7 @@
 #ifndef CLP_S_TEST_UTILS_HPP
 #define CLP_S_TEST_UTILS_HPP
 
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -22,6 +23,7 @@
 [[nodiscard]] auto compress_archive(
         std::string const& file_path,
         std::string const& archive_directory,
+        std::optional<std::string> timestamp_key,
         bool single_file_archive,
         bool structurize_arrays,
         clp_s::FileType file_type

components/core/tests/test-clp_s-delta-encode-log-order.cpp

@@ -2,6 +2,7 @@
 #include <cstdint>
 #include <filesystem>
 #include <memory>
+#include <optional>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -68,6 +69,7 @@ TEST_CASE("clp-s-delta-encode-log-order", "[clp-s][delta-encode-log-order]") {
     REQUIRE_NOTHROW(compress_archive(
             get_test_input_local_path(),
             std::string{cTestDeltaEncodeOrderArchiveDirectory},
+            std::nullopt,
             true,
             false,
             clp_s::FileType::Json

components/core/tests/test-clp_s-end_to_end.cpp

@@ -2,6 +2,7 @@
 
 #include <cstdlib>
 #include <filesystem>
+#include <optional>
 #include <string>
 #include <string_view>
 #include <utility>
@@ -108,6 +109,7 @@ TEST_CASE("clp-s-compress-extract-no-floats", "[clp-s][end-to-end]") {
             std::ignore = compress_archive(
                     get_test_input_local_path(),
                     std::string{cTestEndToEndArchiveDirectory},
+                    std::nullopt,
                     single_file_archive,
                     structurize_arrays,
                     clp_s::FileType::Json

components/core/tests/test-clp_s-range_index.cpp

@@ -1,5 +1,6 @@
 #include <cstdlib>
 #include <filesystem>
+#include <optional>
 #include <string>
 #include <string_view>
 
@@ -224,6 +225,7 @@ TEST_CASE("clp-s-range-index", "[clp-s][range-index]") {
             archive_stats = compress_archive(
                     input_file,
                     std::string{cTestRangeIndexArchiveDirectory},
+                    std::nullopt,
                     single_file_archive,
                     false,
                     input_file_type

components/core/tests/test-clp_s-search.cpp

@@ -2,6 +2,7 @@
 #include <exception>
 #include <filesystem>
 #include <memory>
+#include <optional>
 #include <set>
 #include <sstream>
 #include <string>
@@ -223,7 +224,8 @@ TEST_CASE("clp-s-search", "[clp-s][search]") {
              R"aa(idx: 0 OR idx: 1)aa",
              {1}},
             {R"aa(ambiguous_varstring: "a*e")aa", {10, 11, 12}},
-            {R"aa(ambiguous_varstring: "a\*e")aa", {12}}
+            {R"aa(ambiguous_varstring: "a\*e")aa", {12}},
+            {R"aa(idx: * AND NOT idx: null AND idx: 0)aa", {0}}
     };
     auto structurize_arrays = GENERATE(true, false);
     auto single_file_archive = GENERATE(true, false);
@@ -234,6 +236,7 @@ TEST_CASE("clp-s-search", "[clp-s][search]") {
             std::ignore = compress_archive(
                     get_test_input_local_path(),
                     std::string{cTestSearchArchiveDirectory},
+                    std::string{cTestIdxKey},
                     single_file_archive,
                     structurize_arrays,
                     clp_s::FileType::Json