From f9775e14b00768ec713d05849f1669e1ead693e1 Mon Sep 17 00:00:00 2001
From: PragmaTwice <twice@apache.org>
Date: Sat, 23 Mar 2024 13:56:29 +0900
Subject: [PATCH 1/3] Add Redis query parser via PEGTL for search module

---
 src/search/common_parser.h               | 56 +++++++++++++++
 src/search/common_transformer.h          | 91 ++++++++++++++++++++++++
 src/search/redis_query_parser.h          | 65 +++++++++++++++++
 src/search/redis_query_transformer.h     | 70 ++++++++++++++++++
 src/search/sql_parser.h                  | 36 ++--------
 src/search/sql_transformer.h             | 68 ++----------------
 tests/cppunit/redis_query_parser_test.cc | 42 +++++++++++
 tests/cppunit/sql_parser_test.cc         |  1 -
 8 files changed, 336 insertions(+), 93 deletions(-)
 create mode 100644 src/search/common_parser.h
 create mode 100644 src/search/common_transformer.h
 create mode 100644 src/search/redis_query_parser.h
 create mode 100644 src/search/redis_query_transformer.h
 create mode 100644 tests/cppunit/redis_query_parser_test.cc

diff --git a/src/search/common_parser.h b/src/search/common_parser.h
new file mode 100644
index 00000000000..15f5bc36aff
--- /dev/null
+++ b/src/search/common_parser.h
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#pragma once
+
+#include <tao/pegtl.hpp>
+
+namespace kqir {
+
+namespace peg = tao::pegtl;
+
+struct True : peg::string<'t', 'r', 'u', 'e'> {};
+struct False : peg::string<'f', 'a', 'l', 's', 'e'> {};
+struct Boolean : peg::sor<True, False> {};
+
+struct Digits : peg::plus<peg::digit> {};
+struct NumberExp : peg::seq<peg::one<'e', 'E'>, peg::opt<peg::one<'-', '+'>>, Digits> {};
+struct NumberFrac : peg::seq<peg::one<'.'>, Digits> {};
+struct Number : peg::seq<peg::opt<peg::one<'-'>>, Digits, peg::opt<NumberFrac>, peg::opt<NumberExp>> {};
+
+struct UnicodeXDigit : peg::list<peg::seq<peg::one<'u'>, peg::rep<4, peg::xdigit>>, peg::one<'\\'>> {};
+struct EscapedSingleChar : peg::one<'"', '\\', 'b', 'f', 'n', 'r', 't'> {};
+struct EscapedChar : peg::sor<EscapedSingleChar, UnicodeXDigit> {};
+struct UnescapedChar : peg::utf8::range<0x20, 0x10FFFF> {};
+struct Char : peg::if_then_else<peg::one<'\\'>, EscapedChar, UnescapedChar> {};
+
+struct StringContent : peg::until<peg::at<peg::one<'"'>>, Char> {};
+struct String : peg::seq<peg::one<'"'>, StringContent, peg::any> {};
+
+struct Identifier : peg::identifier {};
+
+struct WhiteSpace : peg::one<' ', '\t', '\n', '\r'> {};
+template <typename T>
+struct WSPad : peg::pad<T, WhiteSpace> {};
+
+struct UnsignedInteger : Digits {};
+struct Integer : peg::seq<peg::opt<peg::one<'-'>>, Digits> {};
+
+}  // namespace kqir
diff --git a/src/search/common_transformer.h b/src/search/common_transformer.h
new file mode 100644
index 00000000000..8febbb4ce73
--- /dev/null
+++ b/src/search/common_transformer.h
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#pragma once
+
+#include <tao/pegtl/contrib/parse_tree.hpp>
+#include <tao/pegtl/contrib/unescape.hpp>
+#include <tao/pegtl/demangle.hpp>
+
+#include "common_parser.h"
+#include "status.h"
+
+namespace kqir {
+
+struct TreeTransformer {
+  using TreeNode = std::unique_ptr<peg::parse_tree::node>;
+
+  template <typename T>
+  static bool Is(const TreeNode& node) {
+    return node->type == peg::demangle<T>();
+  }
+
+  static bool IsRoot(const TreeNode& node) { return node->type.empty(); }
+
+  static StatusOr<std::string> UnescapeString(std::string_view str) {
+    str = str.substr(1, str.size() - 2);
+
+    std::string result;
+    while (!str.empty()) {
+      if (str[0] == '\\') {
+        str.remove_prefix(1);
+        switch (str[0]) {
+          case '\\':
+          case '"':
+            result.push_back(str[0]);
+            break;
+          case 'b':
+            result.push_back('\b');
+            break;
+          case 'f':
+            result.push_back('\f');
+            break;
+          case 'n':
+            result.push_back('\n');
+            break;
+          case 'r':
+            result.push_back('\r');
+            break;
+          case 't':
+            result.push_back('\t');
+            break;
+          case 'u':
+            if (!peg::unescape::utf8_append_utf32(
+                    result, peg::unescape::unhex_string<unsigned>(str.data() + 1, str.data() + 5))) {
+              return {Status::NotOK,
+                      fmt::format("invalid Unicode code point '{}' in string literal", std::string(str.data() + 1, 4))};
+            }
+            str.remove_prefix(4);
+            break;
+          default:
+            __builtin_unreachable();
+        };
+        str.remove_prefix(1);
+      } else {
+        result.push_back(str[0]);
+        str.remove_prefix(1);
+      }
+    }
+
+    return result;
+  }
+};
+
+}  // namespace kqir
diff --git a/src/search/redis_query_parser.h b/src/search/redis_query_parser.h
new file mode 100644
index 00000000000..07eac327e5d
--- /dev/null
+++ b/src/search/redis_query_parser.h
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#pragma once
+
+#include <tao/pegtl.hpp>
+
+#include "common_parser.h"
+
+namespace kqir {
+
+namespace redis_query {
+
+using namespace peg;
+
+struct Field : seq<one<'@'>, Identifier> {};
+
+struct Tag : sor<Identifier, String> {};
+struct TagList : seq<one<'{'>, WSPad<Tag>, star<seq<one<'|'>, WSPad<Tag>>>, one<'}'>> {};
+
+struct Inf : seq<opt<one<'+', '-'>>, string<'i', 'n', 'f'>> {};
+struct ExclusiveNumber: seq<one<'('>, Number> {};
+struct NumericRangePart : sor<Inf, ExclusiveNumber, Number> {};
+struct NumericRange : seq<one<'['>, WSPad<NumericRangePart>, WSPad<NumericRangePart>, one<']'>> {};
+
+struct FieldQuery : seq<WSPad<Field>, one<':'>, WSPad<sor<TagList, NumericRange>>> {};
+
+struct QueryExpr;
+
+struct ParenExpr : WSPad<seq<one<'('>, QueryExpr, one<')'>>> {};
+
+struct NotExpr;
+
+struct BooleanExpr : sor<FieldQuery, ParenExpr, NotExpr> {};
+
+struct NotExpr : seq<WSPad<one<'-'>>, BooleanExpr> {};
+
+struct AndExpr : seq<BooleanExpr, plus<BooleanExpr>> {};
+struct AndExprP : sor<AndExpr, BooleanExpr> {};
+
+struct OrExpr : seq<AndExprP, plus<seq<one<'|'>, AndExprP>>> {};
+struct OrExprP : sor<OrExpr, AndExprP> {};
+
+struct QueryExpr : seq<OrExprP> {};
+
+}  // namespace redis_query
+
+}  // namespace kqir
diff --git a/src/search/redis_query_transformer.h b/src/search/redis_query_transformer.h
new file mode 100644
index 00000000000..7411f7fb960
--- /dev/null
+++ b/src/search/redis_query_transformer.h
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#pragma once
+
+#include "common_transformer.h"
+#include "ir.h"
+#include "parse_util.h"
+#include "redis_query_parser.h"
+
+namespace kqir {
+
+namespace redis_query {
+
+namespace ir = kqir;
+
+template <typename Rule>
+using TreeSelector = parse_tree::selector<
+    Rule,
+    parse_tree::store_content::on<Number, String, Identifier, Inf>,
+    parse_tree::remove_content::on<TagList, NumericRange, ExclusiveNumber, FieldQuery, NotExpr, AndExpr, OrExpr>>;
+
+template <typename Input>
+StatusOr<std::unique_ptr<parse_tree::node>> ParseToTree(Input&& in) {
+  if (auto root = parse_tree::parse<seq<QueryExpr, eof>, TreeSelector>(std::forward<Input>(in))) {
+    return root;
+  } else {
+    // TODO: improve parse error message, with source location
+    return {Status::NotOK, "invalid syntax"};
+  }
+}
+
+struct Transformer : ir::TreeTransformer {
+  static auto Transform(const TreeNode& node) -> StatusOr<std::unique_ptr<Node>> {
+    if (Is<Number>(node)) {
+      return Node::Create<ir::NumericLiteral>(*ParseFloat(node->string()));
+    } else if (Is<TagList>(node)) {
+      
+    } else {
+      // UNREACHABLE CODE, just for debugging here
+      return {Status::NotOK, fmt::format("encountered invalid node type: {}", node->type)};
+    }
+  }
+};
+
+template <typename Input>
+StatusOr<std::unique_ptr<ir::Node>> ParseToIR(Input&& in) {
+  return Transformer::Transform(GET_OR_RET(ParseToTree(std::forward<Input>(in))));
+}
+
+}  // namespace redis_query
+
+}  // namespace kqir
diff --git a/src/search/sql_parser.h b/src/search/sql_parser.h
index dffe051d132..981ea0accde 100644
--- a/src/search/sql_parser.h
+++ b/src/search/sql_parser.h
@@ -18,40 +18,18 @@
  *
  */
 
+#pragma once
+
 #include <tao/pegtl.hpp>
 
-namespace kqir {
+#include "common_parser.h"
 
-namespace peg = tao::pegtl;
+namespace kqir {
 
 namespace sql {
 
 using namespace peg;
 
-struct True : string<'t', 'r', 'u', 'e'> {};
-struct False : string<'f', 'a', 'l', 's', 'e'> {};
-struct Boolean : sor<True, False> {};
-
-struct Digits : plus<digit> {};
-struct NumberExp : seq<one<'e', 'E'>, opt<one<'-', '+'>>, Digits> {};
-struct NumberFrac : seq<one<'.'>, Digits> {};
-struct Number : seq<opt<one<'-'>>, Digits, opt<NumberFrac>, opt<NumberExp>> {};
-
-struct UnicodeXDigit : list<seq<one<'u'>, rep<4, xdigit>>, one<'\\'>> {};
-struct EscapedSingleChar : one<'"', '\\', 'b', 'f', 'n', 'r', 't'> {};
-struct EscapedChar : sor<EscapedSingleChar, UnicodeXDigit> {};
-struct UnescapedChar : utf8::range<0x20, 0x10FFFF> {};
-struct Char : if_then_else<one<'\\'>, EscapedChar, UnescapedChar> {};
-
-struct StringContent : until<at<one<'"'>>, Char> {};
-struct String : seq<one<'"'>, StringContent, any> {};
-
-struct Identifier : identifier {};
-
-struct WhiteSpace : one<' ', '\t', '\n', '\r'> {};
-template <typename T>
-struct WSPad : pad<T, WhiteSpace> {};
-
 struct HasTag : string<'h', 'a', 's', 't', 'a', 'g'> {};
 struct HasTagExpr : WSPad<seq<Identifier, WSPad<HasTag>, String>> {};
 
@@ -74,7 +52,7 @@ struct NotExpr : seq<WSPad<Not>, BooleanExpr> {};
 
 struct And : string<'a', 'n', 'd'> {};
 // left recursion elimination
-// struct AndExpr : sor<seq<AndExpr, And, NotExpr>, NotExpr> {};
+// struct AndExpr : sor<seq<AndExpr, And, BooleanExpr>, BooleanExpr> {};
 struct AndExpr : seq<BooleanExpr, plus<seq<And, BooleanExpr>>> {};
 struct AndExprP : sor<AndExpr, BooleanExpr> {};
 
@@ -100,12 +78,10 @@ struct Asc : string<'a', 's', 'c'> {};
 struct Desc : string<'d', 'e', 's', 'c'> {};
 struct Limit : string<'l', 'i', 'm', 'i', 't'> {};
 
-struct Integer : Digits {};
-
 struct WhereClause : seq<Where, QueryExpr> {};
 struct AscOrDesc : WSPad<sor<Asc, Desc>> {};
 struct OrderByClause : seq<OrderBy, WSPad<Identifier>, opt<AscOrDesc>> {};
-struct LimitClause : seq<Limit, opt<seq<WSPad<Integer>, one<','>>>, WSPad<Integer>> {};
+struct LimitClause : seq<Limit, opt<seq<WSPad<UnsignedInteger>, one<','>>>, WSPad<UnsignedInteger>> {};
 
 struct SearchStmt
     : WSPad<seq<Select, SelectExpr, From, FromExpr, opt<WhereClause>, opt<OrderByClause>, opt<LimitClause>>> {};
diff --git a/src/search/sql_transformer.h b/src/search/sql_transformer.h
index 76fac35ce7b..5c300654c28 100644
--- a/src/search/sql_transformer.h
+++ b/src/search/sql_transformer.h
@@ -18,13 +18,13 @@
  *
  */
 
+#pragma once
+
 #include <limits>
 #include <memory>
-#include <tao/pegtl/contrib/parse_tree.hpp>
-#include <tao/pegtl/contrib/unescape.hpp>
-#include <tao/pegtl/demangle.hpp>
 #include <variant>
 
+#include "common_transformer.h"
 #include "ir.h"
 #include "parse_util.h"
 #include "sql_parser.h"
@@ -37,7 +37,8 @@ namespace ir = kqir;
 
 template <typename Rule>
 using TreeSelector = parse_tree::selector<
-    Rule, parse_tree::store_content::on<Boolean, Number, String, Identifier, NumericCompareOp, AscOrDesc, Integer>,
+    Rule,
+    parse_tree::store_content::on<Boolean, Number, String, Identifier, NumericCompareOp, AscOrDesc, UnsignedInteger>,
     parse_tree::remove_content::on<HasTagExpr, NumericCompareExpr, NotExpr, AndExpr, OrExpr, Wildcard, SelectExpr,
                                    FromExpr, WhereClause, OrderByClause, LimitClause, SearchStmt>>;
 
@@ -51,64 +52,7 @@ StatusOr<std::unique_ptr<parse_tree::node>> ParseToTree(Input&& in) {
   }
 }
 
-struct Transformer {
-  using TreeNode = std::unique_ptr<parse_tree::node>;
-
-  template <typename T>
-  static bool Is(const TreeNode& node) {
-    return node->type == demangle<T>();
-  }
-
-  static bool IsRoot(const TreeNode& node) { return node->type.empty(); }
-
-  static StatusOr<std::string> UnescapeString(std::string_view str) {
-    str = str.substr(1, str.size() - 2);
-
-    std::string result;
-    while (!str.empty()) {
-      if (str[0] == '\\') {
-        str.remove_prefix(1);
-        switch (str[0]) {
-          case '\\':
-          case '"':
-            result.push_back(str[0]);
-            break;
-          case 'b':
-            result.push_back('\b');
-            break;
-          case 'f':
-            result.push_back('\f');
-            break;
-          case 'n':
-            result.push_back('\n');
-            break;
-          case 'r':
-            result.push_back('\r');
-            break;
-          case 't':
-            result.push_back('\t');
-            break;
-          case 'u':
-            if (!unescape::utf8_append_utf32(result,
-                                             unescape::unhex_string<unsigned>(str.data() + 1, str.data() + 5))) {
-              return {Status::NotOK,
-                      fmt::format("invalid Unicode code point '{}' in string literal", std::string(str.data() + 1, 4))};
-            }
-            str.remove_prefix(4);
-            break;
-          default:
-            __builtin_unreachable();
-        };
-        str.remove_prefix(1);
-      } else {
-        result.push_back(str[0]);
-        str.remove_prefix(1);
-      }
-    }
-
-    return result;
-  }
-
+struct Transformer : ir::TreeTransformer {
   static auto Transform(const TreeNode& node) -> StatusOr<std::unique_ptr<Node>> {
     if (Is<Boolean>(node)) {
       return Node::Create<ir::BoolLiteral>(node->string_view() == "true");
diff --git a/tests/cppunit/redis_query_parser_test.cc b/tests/cppunit/redis_query_parser_test.cc
new file mode 100644
index 00000000000..d2c7fc378d5
--- /dev/null
+++ b/tests/cppunit/redis_query_parser_test.cc
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+#include <gtest/gtest.h>
+#include <search/redis_query_transformer.h>
+
+#include "tao/pegtl/contrib/parse_tree_to_dot.hpp"
+#include "tao/pegtl/string_input.hpp"
+
+using namespace kqir::redis_query;
+
+static auto Parse(const std::string& in) { return ParseToTree(string_input(in, "test")); }
+
+#define AssertSyntaxError(node) ASSERT_EQ(node.Msg(), "invalid syntax");  // NOLINT
+
+// NOLINTNEXTLINE
+#define AssertIR(node, val)              \
+  ASSERT_EQ(node.Msg(), Status::ok_msg); \
+  ASSERT_EQ(node.GetValue()->Dump(), val);
+
+TEST(RedisQueryParserTest, Simple) {
+    if (auto root = Parse("@a:{ hello | hi } @b:[1 2] | @c:[(3 +inf]")) {
+        parse_tree::print_dot(std::cout, **root);
+    }
+}
diff --git a/tests/cppunit/sql_parser_test.cc b/tests/cppunit/sql_parser_test.cc
index 3a99746d0f8..34fd0f3c307 100644
--- a/tests/cppunit/sql_parser_test.cc
+++ b/tests/cppunit/sql_parser_test.cc
@@ -21,7 +21,6 @@
 #include <gtest/gtest.h>
 #include <search/sql_transformer.h>
 
-#include "tao/pegtl/contrib/parse_tree_to_dot.hpp"
 #include "tao/pegtl/string_input.hpp"
 
 using namespace kqir::sql;

From 7bd32f2af86143d65fe8c4dea0e1d739dc53c182 Mon Sep 17 00:00:00 2001
From: PragmaTwice <twice@apache.org>
Date: Sat, 23 Mar 2024 15:06:51 +0900
Subject: [PATCH 2/3] finish transformer & add tests

---
 src/search/redis_query_parser.h          |  2 +-
 src/search/redis_query_transformer.h     | 94 +++++++++++++++++++++++-
 tests/cppunit/redis_query_parser_test.cc | 57 ++++++++++++--
 3 files changed, 143 insertions(+), 10 deletions(-)

diff --git a/src/search/redis_query_parser.h b/src/search/redis_query_parser.h
index 07eac327e5d..43e525d8a8a 100644
--- a/src/search/redis_query_parser.h
+++ b/src/search/redis_query_parser.h
@@ -36,7 +36,7 @@ struct Tag : sor<Identifier, String> {};
 struct TagList : seq<one<'{'>, WSPad<Tag>, star<seq<one<'|'>, WSPad<Tag>>>, one<'}'>> {};
 
 struct Inf : seq<opt<one<'+', '-'>>, string<'i', 'n', 'f'>> {};
-struct ExclusiveNumber: seq<one<'('>, Number> {};
+struct ExclusiveNumber : seq<one<'('>, Number> {};
 struct NumericRangePart : sor<Inf, ExclusiveNumber, Number> {};
 struct NumericRange : seq<one<'['>, WSPad<NumericRangePart>, WSPad<NumericRangePart>, one<']'>> {};
 
diff --git a/src/search/redis_query_transformer.h b/src/search/redis_query_transformer.h
index 7411f7fb960..1409544b2c9 100644
--- a/src/search/redis_query_transformer.h
+++ b/src/search/redis_query_transformer.h
@@ -20,10 +20,13 @@
 
 #pragma once
 
+#include <memory>
+
 #include "common_transformer.h"
 #include "ir.h"
 #include "parse_util.h"
 #include "redis_query_parser.h"
+#include "search/common_parser.h"
 
 namespace kqir {
 
@@ -33,8 +36,7 @@ namespace ir = kqir;
 
 template <typename Rule>
 using TreeSelector = parse_tree::selector<
-    Rule,
-    parse_tree::store_content::on<Number, String, Identifier, Inf>,
+    Rule, parse_tree::store_content::on<Number, String, Identifier, Inf>,
     parse_tree::remove_content::on<TagList, NumericRange, ExclusiveNumber, FieldQuery, NotExpr, AndExpr, OrExpr>>;
 
 template <typename Input>
@@ -51,8 +53,92 @@ struct Transformer : ir::TreeTransformer {
   static auto Transform(const TreeNode& node) -> StatusOr<std::unique_ptr<Node>> {
     if (Is<Number>(node)) {
       return Node::Create<ir::NumericLiteral>(*ParseFloat(node->string()));
-    } else if (Is<TagList>(node)) {
-      
+    } else if (Is<FieldQuery>(node)) {
+      CHECK(node->children.size() == 2);
+
+      auto field = node->children[0]->string();
+      const auto& query = node->children[1];
+
+      if (Is<TagList>(query)) {
+        std::vector<std::unique_ptr<ir::QueryExpr>> exprs;
+
+        for (const auto& tag : query->children) {
+          auto tag_str = Is<Identifier>(tag) ? tag->string() : GET_OR_RET(UnescapeString(tag->string()));
+          exprs.push_back(std::make_unique<ir::TagContainExpr>(std::make_unique<FieldRef>(field),
+                                                               std::make_unique<StringLiteral>(tag_str)));
+        }
+
+        if (exprs.size() == 1) {
+          return std::move(exprs[0]);
+        } else {
+          return std::make_unique<ir::OrExpr>(std::move(exprs));
+        }
+      } else {  // NumericRange
+        std::vector<std::unique_ptr<ir::QueryExpr>> exprs;
+
+        const auto& lhs = query->children[0];
+        const auto& rhs = query->children[1];
+
+        if (Is<ExclusiveNumber>(lhs)) {
+          exprs.push_back(
+              std::make_unique<NumericCompareExpr>(NumericCompareExpr::GT, std::make_unique<FieldRef>(field),
+                                                   Node::As<NumericLiteral>(GET_OR_RET(Transform(lhs->children[0])))));
+        } else if (Is<Number>(lhs)) {
+          exprs.push_back(std::make_unique<NumericCompareExpr>(NumericCompareExpr::GET,
+                                                               std::make_unique<FieldRef>(field),
+                                                               Node::As<NumericLiteral>(GET_OR_RET(Transform(lhs)))));
+        } else {  // Inf
+          if (lhs->string_view() == "+inf") {
+            return {Status::NotOK, "it's not allowed to set the lower bound as positive infinity"};
+          }
+        }
+
+        if (Is<ExclusiveNumber>(rhs)) {
+          exprs.push_back(
+              std::make_unique<NumericCompareExpr>(NumericCompareExpr::LT, std::make_unique<FieldRef>(field),
+                                                   Node::As<NumericLiteral>(GET_OR_RET(Transform(rhs->children[0])))));
+        } else if (Is<Number>(rhs)) {
+          exprs.push_back(std::make_unique<NumericCompareExpr>(NumericCompareExpr::LET,
+                                                               std::make_unique<FieldRef>(field),
+                                                               Node::As<NumericLiteral>(GET_OR_RET(Transform(rhs)))));
+        } else {  // Inf
+          if (rhs->string_view() == "-inf") {
+            return {Status::NotOK, "it's not allowed to set the upper bound as negative infinity"};
+          }
+        }
+
+        if (exprs.empty()) {
+          return std::make_unique<BoolLiteral>(true);
+        } else if (exprs.size() == 1) {
+          return std::move(exprs[0]);
+        } else {
+          return std::make_unique<ir::AndExpr>(std::move(exprs));
+        }
+      }
+    } else if (Is<NotExpr>(node)) {
+      CHECK(node->children.size() == 1);
+
+      return Node::Create<ir::NotExpr>(Node::As<ir::QueryExpr>(GET_OR_RET(Transform(node->children[0]))));
+    } else if (Is<AndExpr>(node)) {
+      std::vector<std::unique_ptr<ir::QueryExpr>> exprs;
+
+      for (const auto& child : node->children) {
+        exprs.push_back(Node::As<ir::QueryExpr>(GET_OR_RET(Transform(child))));
+      }
+
+      return Node::Create<ir::AndExpr>(std::move(exprs));
+    } else if (Is<OrExpr>(node)) {
+      std::vector<std::unique_ptr<ir::QueryExpr>> exprs;
+
+      for (const auto& child : node->children) {
+        exprs.push_back(Node::As<ir::QueryExpr>(GET_OR_RET(Transform(child))));
+      }
+
+      return Node::Create<ir::OrExpr>(std::move(exprs));
+    } else if (IsRoot(node)) {
+      CHECK(node->children.size() == 1);
+
+      return Transform(node->children[0]);
     } else {
       // UNREACHABLE CODE, just for debugging here
       return {Status::NotOK, fmt::format("encountered invalid node type: {}", node->type)};
diff --git a/tests/cppunit/redis_query_parser_test.cc b/tests/cppunit/redis_query_parser_test.cc
index d2c7fc378d5..1c794ffedcc 100644
--- a/tests/cppunit/redis_query_parser_test.cc
+++ b/tests/cppunit/redis_query_parser_test.cc
@@ -21,12 +21,11 @@
 #include <gtest/gtest.h>
 #include <search/redis_query_transformer.h>
 
-#include "tao/pegtl/contrib/parse_tree_to_dot.hpp"
 #include "tao/pegtl/string_input.hpp"
 
 using namespace kqir::redis_query;
 
-static auto Parse(const std::string& in) { return ParseToTree(string_input(in, "test")); }
+static auto Parse(const std::string& in) { return ParseToIR(string_input(in, "test")); }
 
 #define AssertSyntaxError(node) ASSERT_EQ(node.Msg(), "invalid syntax");  // NOLINT
 
@@ -36,7 +35,55 @@ static auto Parse(const std::string& in) { return ParseToTree(string_input(in, "
   ASSERT_EQ(node.GetValue()->Dump(), val);
 
 TEST(RedisQueryParserTest, Simple) {
-    if (auto root = Parse("@a:{ hello | hi } @b:[1 2] | @c:[(3 +inf]")) {
-        parse_tree::print_dot(std::cout, **root);
-    }
+  AssertSyntaxError(Parse(""));
+  AssertSyntaxError(Parse("a"));
+  AssertSyntaxError(Parse("@a"));
+  AssertSyntaxError(Parse("a:"));
+  AssertSyntaxError(Parse("@a:"));
+  AssertSyntaxError(Parse("@a:[]"));
+  AssertSyntaxError(Parse("@a:[1 2"));
+  AssertSyntaxError(Parse("@a:[(inf 1]"));
+  AssertSyntaxError(Parse("@a:[((1 2]"));
+  AssertSyntaxError(Parse("@a:[1]"));
+  AssertSyntaxError(Parse("@a:[1 2 3]"));
+  AssertSyntaxError(Parse("@a:{}"));
+  AssertSyntaxError(Parse("@a:{x"));
+  AssertSyntaxError(Parse("@a:{|}"));
+  AssertSyntaxError(Parse("@a:{x|}"));
+  AssertSyntaxError(Parse("@a:{|y}"));
+  AssertSyntaxError(Parse("@a:{x|y|}"));
+  AssertSyntaxError(Parse("@a:{x}|"));
+  AssertSyntaxError(Parse("@a:{x} -"));
+  AssertSyntaxError(Parse("@a:{x}|@a:{x}|"));
+
+  AssertIR(Parse("@a:[1 2]"), "(and a >= 1, a <= 2)");
+  AssertIR(Parse("@a : [1 2]"), "(and a >= 1, a <= 2)");
+  AssertIR(Parse("@a:[(1 2]"), "(and a > 1, a <= 2)");
+  AssertIR(Parse("@a:[1 (2]"), "(and a >= 1, a < 2)");
+  AssertIR(Parse("@a:[(1 (2]"), "(and a > 1, a < 2)");
+  AssertIR(Parse("@a:[inf 2]"), "a <= 2");
+  AssertIR(Parse("@a:[-inf 2]"), "a <= 2");
+  AssertIR(Parse("@a:[1 inf]"), "a >= 1");
+  AssertIR(Parse("@a:[1 +inf]"), "a >= 1");
+  AssertIR(Parse("@a:[(1 +inf]"), "a > 1");
+  AssertIR(Parse("@a:[-inf +inf]"), "true");
+  AssertIR(Parse("@a:{x}"), "a hastag \"x\"");
+  AssertIR(Parse("@a:{x|y}"), R"((or a hastag "x", a hastag "y"))");
+  AssertIR(Parse("@a:{x|y|z}"), R"((or a hastag "x", a hastag "y", a hastag "z"))");
+  AssertIR(Parse(R"(@a:{"x"|y})"), R"((or a hastag "x", a hastag "y"))");
+  AssertIR(Parse(R"(@a:{"x" | "y"})"), R"((or a hastag "x", a hastag "y"))");
+  AssertIR(Parse("@a:{x} @b:[1 inf]"), "(and a hastag \"x\", b >= 1)");
+  AssertIR(Parse("@a:{x} | @b:[1 inf]"), "(or a hastag \"x\", b >= 1)");
+  AssertIR(Parse("@a:{x} @b:[1 inf] @c:{y}"), "(and a hastag \"x\", b >= 1, c hastag \"y\")");
+  AssertIR(Parse("@a:{x}|@b:[1 inf] | @c:{y}"), "(or a hastag \"x\", b >= 1, c hastag \"y\")");
+  AssertIR(Parse("@a:[1 inf] @b:[inf 2]| @c:[(3 inf]"), "(or (and a >= 1, b <= 2), c > 3)");
+  AssertIR(Parse("@a:[1 inf] | @b:[inf 2] @c:[(3 inf]"), "(or a >= 1, (and b <= 2, c > 3))");
+  AssertIR(Parse("(@a:[1 inf] @b:[inf 2])| @c:[(3 inf]"), "(or (and a >= 1, b <= 2), c > 3)");
+  AssertIR(Parse("@a:[1 inf] | (@b:[inf 2] @c:[(3 inf])"), "(or a >= 1, (and b <= 2, c > 3))");
+  AssertIR(Parse("@a:[1 inf] (@b:[inf 2]| @c:[(3 inf])"), "(and a >= 1, (or b <= 2, c > 3))");
+  AssertIR(Parse("(@a:[1 inf] | @b:[inf 2]) @c:[(3 inf]"), "(and (or a >= 1, b <= 2), c > 3)");
+  AssertIR(Parse("-@a:{x}"), "not a hastag \"x\"");
+  AssertIR(Parse("-@a:[(1 +inf]"), "not a > 1");
+  AssertIR(Parse("-@a:[1 inf] @b:[inf 2]| -@c:[(3 inf]"), "(or (and not a >= 1, b <= 2), not c > 3)");
+  AssertIR(Parse("@a:[1 inf] -(@b:[inf 2]| @c:[(3 inf])"), "(and a >= 1, not (or b <= 2, c > 3))");
 }

From 532bb535444b4877126a474c78ad66b15c86cad4 Mon Sep 17 00:00:00 2001
From: PragmaTwice <twice@apache.org>
Date: Sat, 23 Mar 2024 16:39:12 +0900
Subject: [PATCH 3/3] fix

---
 src/search/redis_query_parser.h          |  6 ++++--
 src/search/redis_query_transformer.h     | 11 +++++++----
 tests/cppunit/redis_query_parser_test.cc |  3 +++
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/src/search/redis_query_parser.h b/src/search/redis_query_parser.h
index 43e525d8a8a..fd373830f32 100644
--- a/src/search/redis_query_parser.h
+++ b/src/search/redis_query_parser.h
@@ -42,17 +42,19 @@ struct NumericRange : seq<one<'['>, WSPad<NumericRangePart>, WSPad<NumericRangeP
 
 struct FieldQuery : seq<WSPad<Field>, one<':'>, WSPad<sor<TagList, NumericRange>>> {};
 
+struct Wildcard : one<'*'> {};
+
 struct QueryExpr;
 
 struct ParenExpr : WSPad<seq<one<'('>, QueryExpr, one<')'>>> {};
 
 struct NotExpr;
 
-struct BooleanExpr : sor<FieldQuery, ParenExpr, NotExpr> {};
+struct BooleanExpr : sor<FieldQuery, ParenExpr, NotExpr, WSPad<Wildcard>> {};
 
 struct NotExpr : seq<WSPad<one<'-'>>, BooleanExpr> {};
 
-struct AndExpr : seq<BooleanExpr, plus<BooleanExpr>> {};
+struct AndExpr : seq<BooleanExpr, plus<seq<BooleanExpr>>> {};
 struct AndExprP : sor<AndExpr, BooleanExpr> {};
 
 struct OrExpr : seq<AndExprP, plus<seq<one<'|'>, AndExprP>>> {};
diff --git a/src/search/redis_query_transformer.h b/src/search/redis_query_transformer.h
index 1409544b2c9..ebd051430c3 100644
--- a/src/search/redis_query_transformer.h
+++ b/src/search/redis_query_transformer.h
@@ -35,9 +35,10 @@ namespace redis_query {
 namespace ir = kqir;
 
 template <typename Rule>
-using TreeSelector = parse_tree::selector<
-    Rule, parse_tree::store_content::on<Number, String, Identifier, Inf>,
-    parse_tree::remove_content::on<TagList, NumericRange, ExclusiveNumber, FieldQuery, NotExpr, AndExpr, OrExpr>>;
+using TreeSelector =
+    parse_tree::selector<Rule, parse_tree::store_content::on<Number, String, Identifier, Inf>,
+                         parse_tree::remove_content::on<TagList, NumericRange, ExclusiveNumber, FieldQuery, NotExpr,
+                                                        AndExpr, OrExpr, Wildcard>>;
 
 template <typename Input>
 StatusOr<std::unique_ptr<parse_tree::node>> ParseToTree(Input&& in) {
@@ -53,6 +54,8 @@ struct Transformer : ir::TreeTransformer {
   static auto Transform(const TreeNode& node) -> StatusOr<std::unique_ptr<Node>> {
     if (Is<Number>(node)) {
       return Node::Create<ir::NumericLiteral>(*ParseFloat(node->string()));
+    } else if (Is<Wildcard>(node)) {
+      return Node::Create<ir::BoolLiteral>(true);
     } else if (Is<FieldQuery>(node)) {
       CHECK(node->children.size() == 2);
 
@@ -143,7 +146,7 @@ struct Transformer : ir::TreeTransformer {
       // UNREACHABLE CODE, just for debugging here
       return {Status::NotOK, fmt::format("encountered invalid node type: {}", node->type)};
     }
-  }
+  }  // NOLINT
 };
 
 template <typename Input>
diff --git a/tests/cppunit/redis_query_parser_test.cc b/tests/cppunit/redis_query_parser_test.cc
index 1c794ffedcc..a31051a58bf 100644
--- a/tests/cppunit/redis_query_parser_test.cc
+++ b/tests/cppunit/redis_query_parser_test.cc
@@ -86,4 +86,7 @@ TEST(RedisQueryParserTest, Simple) {
   AssertIR(Parse("-@a:[(1 +inf]"), "not a > 1");
   AssertIR(Parse("-@a:[1 inf] @b:[inf 2]| -@c:[(3 inf]"), "(or (and not a >= 1, b <= 2), not c > 3)");
   AssertIR(Parse("@a:[1 inf] -(@b:[inf 2]| @c:[(3 inf])"), "(and a >= 1, not (or b <= 2, c > 3))");
+  AssertIR(Parse("*"), "true");
+  AssertIR(Parse("* *"), "(and true, true)");
+  AssertIR(Parse("*|*"), "(or true, true)");
 }