This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new ec3d9032 fix(search): misidentify DESC token in SQL parser (#2439)
ec3d9032 is described below
commit ec3d9032deb7aef3e238570cfa9eeb607491e2e6
Author: Twice <[email protected]>
AuthorDate: Sun Jul 21 17:37:24 2024 +0900
fix(search): misidentify DESC token in SQL parser (#2439)
---
src/search/ir.h | 4 ++--
src/search/passes/manager.h | 14 ++++++++++++++
src/search/passes/recorder.h | 41 ++++++++++++++++++++++++++++++++++++++++
src/search/sql_parser.h | 4 ++--
src/search/sql_transformer.h | 3 +--
tests/cppunit/sql_parser_test.cc | 1 +
6 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/src/search/ir.h b/src/search/ir.h
index 72cb7351..116fe9a7 100644
--- a/src/search/ir.h
+++ b/src/search/ir.h
@@ -432,8 +432,8 @@ struct SearchExpr : Node {
std::unique_ptr<Node> Clone() const override {
return std::make_unique<SearchExpr>(
Node::MustAs<IndexRef>(index->Clone()),
Node::MustAs<QueryExpr>(query_expr->Clone()),
- Node::MustAs<LimitClause>(limit->Clone()),
Node::MustAs<SortByClause>(sort_by->Clone()),
- Node::MustAs<SelectClause>(select->Clone()));
+ limit ? Node::MustAs<LimitClause>(limit->Clone()) : nullptr,
+ sort_by ? Node::MustAs<SortByClause>(sort_by->Clone()) : nullptr,
Node::MustAs<SelectClause>(select->Clone()));
}
};
diff --git a/src/search/passes/manager.h b/src/search/passes/manager.h
index e94a12e9..57f317d2 100644
--- a/src/search/passes/manager.h
+++ b/src/search/passes/manager.h
@@ -31,6 +31,7 @@
#include "search/passes/interval_analysis.h"
#include "search/passes/lower_to_plan.h"
#include "search/passes/push_down_not_expr.h"
+#include "search/passes/recorder.h"
#include "search/passes/simplify_and_or_expr.h"
#include "search/passes/simplify_boolean.h"
#include "search/passes/sort_limit_fuse.h"
@@ -60,6 +61,18 @@ struct PassManager {
return result;
}
+ static PassSequence FullRecord(PassSequence &&seq,
std::vector<std::unique_ptr<Node>> &results) {
+ PassSequence res_seq;
+ res_seq.push_back(std::make_unique<Recorder>(results));
+
+ for (auto &p : seq) {
+ res_seq.push_back(std::move(p));
+ res_seq.push_back(std::make_unique<Recorder>(results));
+ }
+
+ return res_seq;
+ }
+
template <typename... PassSeqs>
static PassSequence Merge(PassSeqs &&...seqs) {
static_assert(std::conjunction_v<std::negation<std::is_reference<PassSeqs>>...>);
@@ -79,6 +92,7 @@ struct PassManager {
static PassSequence PlanPasses() { return Create(LowerToPlan{},
IndexSelection{}, SortLimitFuse{}); }
static PassSequence Default() { return Merge(ExprPasses(), NumericPasses(),
PlanPasses()); }
+ static PassSequence Debug(std::vector<std::unique_ptr<Node>> &recorded) {
return FullRecord(Default(), recorded); }
};
} // namespace kqir
diff --git a/src/search/passes/recorder.h b/src/search/passes/recorder.h
new file mode 100644
index 00000000..e68fa673
--- /dev/null
+++ b/src/search/passes/recorder.h
@@ -0,0 +1,41 @@
+/*
+ * 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 <memory>
+
+#include "search/ir.h"
+#include "search/ir_pass.h"
+
+namespace kqir {
+
+struct Recorder : Pass {
+ std::vector<std::unique_ptr<Node>> &results;
+
+ explicit Recorder(std::vector<std::unique_ptr<Node>> &results) :
results(results) {}
+
+ std::unique_ptr<Node> Transform(std::unique_ptr<Node> node) override {
+ results.push_back(node->Clone());
+ return node;
+ }
+};
+
+} // namespace kqir
diff --git a/src/search/sql_parser.h b/src/search/sql_parser.h
index 26e3da6d..9a611d9b 100644
--- a/src/search/sql_parser.h
+++ b/src/search/sql_parser.h
@@ -79,8 +79,8 @@ struct Desc : string<'d', 'e', 's', 'c'> {};
struct Limit : string<'l', 'i', 'm', 'i', 't'> {};
struct WhereClause : seq<Where, QueryExpr> {};
-struct AscOrDesc : WSPad<sor<Asc, Desc>> {};
-struct OrderByClause : seq<OrderBy, WSPad<Identifier>, opt<AscOrDesc>> {};
+struct AscOrDesc : sor<Asc, Desc> {};
+struct OrderByClause : seq<OrderBy, WSPad<Identifier>, opt<WSPad<AscOrDesc>>>
{};
struct LimitClause : seq<Limit, opt<seq<WSPad<UnsignedInteger>, one<','>>>,
WSPad<UnsignedInteger>> {};
struct SearchStmt
diff --git a/src/search/sql_transformer.h b/src/search/sql_transformer.h
index 972ae894..72aa0de7 100644
--- a/src/search/sql_transformer.h
+++ b/src/search/sql_transformer.h
@@ -133,8 +133,7 @@ struct Transformer : ir::TreeTransformer {
}
return Node::Create<ir::LimitClause>(offset, count);
- }
- if (Is<OrderByClause>(node)) {
+ } else if (Is<OrderByClause>(node)) {
CHECK(node->children.size() == 1 || node->children.size() == 2);
auto field = std::make_unique<FieldRef>(node->children[0]->string());
diff --git a/tests/cppunit/sql_parser_test.cc b/tests/cppunit/sql_parser_test.cc
index a566d965..c6fc96ba 100644
--- a/tests/cppunit/sql_parser_test.cc
+++ b/tests/cppunit/sql_parser_test.cc
@@ -125,6 +125,7 @@ TEST(SQLParserTest, Simple) {
AssertIR(Parse("select a from b limit 2, 3"), "select a from b where true
limit 2, 3");
AssertIR(Parse("select a from b order by a"), "select a from b where true
sortby a, asc");
AssertIR(Parse("select a from b order by c desc"), "select a from b where
true sortby c, desc");
+ AssertIR(Parse("select a from b order by c desc limit 10"), "select a from b
where true sortby c, desc limit 0, 10");
AssertIR(Parse("select a from b order by a limit 10"), "select a from b
where true sortby a, asc limit 0, 10");
AssertIR(Parse("select a from b where c = 1 limit 10"), "select a from b
where c = 1 limit 0, 10");
AssertIR(Parse("select a from b where c = 1 and d hastag \"x\" order by e"),
