cherry pick from nebula : full text bugfix (vesoft-inc#2829)

* show analyzer when show ft index (vesoft-inc#5587) * show analyzer when show ft index * fix tck * fix ft (vesoft-inc#5594) * fix conflict * Fix expression issues when using ES_QUERY in Lookup clause (vesoft-inc#5597) * Add cherry pick missed codes * Restore permission check logics --------- Co-authored-by: hs.zhang <[email protected]> Co-authored-by: Yee <[email protected]>
yixinglu · Jun 16, 2023 · 6739028 · 6739028
1 parent 00cb1b6
commit 6739028
Show file tree

Hide file tree

Showing 30 changed files with 553 additions and 155 deletions.
diff --git a/src/common/datatypes/Edge.cpp b/src/common/datatypes/Edge.cpp
@@ -82,9 +82,8 @@ const Value& Edge::value(const std::string& key) const {
   auto find = props.find(key);
   if (find != props.end()) {
     return find->second;
-  } else {
-    return Value::kNullValue;
   }
+  return Value::kNullValue;
 }
 
 bool Edge::operator<(const Edge& rhs) const {

diff --git a/src/common/datatypes/Vertex.cpp b/src/common/datatypes/Vertex.cpp
@@ -61,6 +61,18 @@ const Value& Vertex::value(const std::string& key) const {
   return Value::kNullValue;
 }
 
+const Value& Vertex::getTagProp(const std::string& tagName, const std::string& prop) const {
+  for (const auto& tag : tags) {
+    if (tag.name == tagName) {
+      auto find = tag.props.find(prop);
+      if (find != tag.props.end()) {
+        return find->second;
+      }
+    }
+  }
+  return Value::kNullValue;
+}
+
 std::string Vertex::toString() const {
   std::stringstream os;
   os << "(" << vid << ")";

diff --git a/src/common/datatypes/Vertex.h b/src/common/datatypes/Vertex.h
@@ -107,6 +107,7 @@ struct Vertex {
   bool contains(const Value& key) const;
 
   const Value& value(const std::string& key) const;
+  const Value& getTagProp(const std::string& tagName, const std::string& prop) const;
 };
 
 inline void swap(Vertex& a, Vertex& b) {

diff --git a/src/common/plugin/fulltext/elasticsearch/ESAdapter.cpp b/src/common/plugin/fulltext/elasticsearch/ESAdapter.cpp
@@ -22,7 +22,7 @@ void ESBulk::put(const std::string& indexName,
                  const std::string& src,
                  const std::string& dst,
                  int64_t rank,
-                 const std::string& text) {
+                 std::map<std::string, std::string> data) {
   folly::dynamic action = folly::dynamic::object();
   folly::dynamic metadata = folly::dynamic::object();
   folly::dynamic body = folly::dynamic::object();
@@ -35,7 +35,9 @@ void ESBulk::put(const std::string& indexName,
   body["src"] = src;
   body["dst"] = dst;
   body["rank"] = rank;
-  body["text"] = text;
+  for (auto& [key, value] : data) {
+    body[key] = std::move(value);
+  }
   documents_[indexName].emplace_back(std::move(action));
   documents_[indexName].emplace_back(std::move(body));
 }

diff --git a/src/common/plugin/fulltext/elasticsearch/ESAdapter.h b/src/common/plugin/fulltext/elasticsearch/ESAdapter.h
@@ -44,7 +44,7 @@ class ESBulk {
            const std::string& src,
            const std::string& dst,
            int64_t rank,
-           const std::string& text);
+           std::map<std::string, std::string> data);
   void delete_(const std::string& indexName,
                const std::string& vid,
                const std::string& src,

diff --git a/src/common/plugin/fulltext/test/ElasticsearchTest.cpp b/src/common/plugin/fulltext/test/ElasticsearchTest.cpp
@@ -324,7 +324,8 @@ TEST_F(ESTest, bulk) {
   plugin::ESClient client(mockHttpClient, "http", "127.0.0.1:9200", "", "");
   plugin::ESAdapter adapter(std::vector<plugin::ESClient>({client}));
   plugin::ESBulk bulk;
-  bulk.put("nebula_index_1", "1", "", "", 0, "vertex text");
+  std::map<std::string, std::string> data{{"text", "vretex text"}};
+  bulk.put("nebula_index_1", "1", "", "", 0, data);
   bulk.delete_("nebula_index_2", "", "a", "b", 10);
   {
     auto result = adapter.bulk(bulk, true);

diff --git a/src/graph/context/iterator/SequentialIter.cpp b/src/graph/context/iterator/SequentialIter.cpp
@@ -126,6 +126,25 @@ StatusOr<std::size_t> SequentialIter::getColumnIndex(const std::string& col) con
   return index->second;
 }
 
+const Value& SequentialIter::getTagProp(const std::string& tag, const std::string& prop) const {
+  const auto& val = this->getColumn("VERTEX");
+  if (val.isVertex()) {
+    const auto& vertex = val.getVertex();
+    return vertex.getTagProp(tag, prop);
+  }
+  return Value::kNullValue;
+}
+
+const Value& SequentialIter::getEdgeProp(const std::string& edge, const std::string& prop) const {
+  DCHECK(!edge.empty());
+  const auto& val = this->getColumn("EDGE");
+  if (val.isEdge()) {
+    const auto& e = val.getEdge();
+    return e.value(prop);
+  }
+  return Value::kNullValue;
+}
+
 Value SequentialIter::getVertex(const std::string& name) {
   return getColumn(name);
 }

diff --git a/src/graph/context/iterator/SequentialIter.h b/src/graph/context/iterator/SequentialIter.h
@@ -78,6 +78,10 @@ class SequentialIter : public Iterator {
 
   StatusOr<std::size_t> getColumnIndex(const std::string& col) const override;
 
+  const Value& getTagProp(const std::string& tag, const std::string& prop) const override;
+
+  const Value& getEdgeProp(const std::string& edge, const std::string& prop) const override;
+
   Value getVertex(const std::string& name = "") override;
 
   Value getEdge() const override;

diff --git a/src/graph/executor/maintain/FTIndexExecutor.cpp b/src/graph/executor/maintain/FTIndexExecutor.cpp
@@ -70,7 +70,7 @@ folly::Future<Status> ShowFTIndexesExecutor::execute() {
 
         auto indexes = std::move(resp).value();
         DataSet dataSet;
-        dataSet.colNames = {"Name", "Schema Type", "Schema Name", "Fields"};
+        dataSet.colNames = {"Name", "Schema Type", "Schema Name", "Fields", "Analyzer"};
         for (auto &index : indexes) {
           if (index.second.get_space_id() != spaceId) {
             continue;
@@ -92,6 +92,11 @@ folly::Future<Status> ShowFTIndexesExecutor::execute() {
           row.values.emplace_back(isEdge ? "Edge" : "Tag");
           row.values.emplace_back(std::move(shmNameRet).value());
           row.values.emplace_back(std::move(fields));
+          std::string analyzer = index.second.get_analyzer();
+          if (analyzer.empty()) {
+            analyzer = "default";
+          }
+          row.values.emplace_back(analyzer);
           dataSet.rows.emplace_back(std::move(row));
         }
         return finish(ResultBuilder()

diff --git a/src/graph/executor/query/FulltextIndexScanExecutor.cpp b/src/graph/executor/query/FulltextIndexScanExecutor.cpp
@@ -7,6 +7,7 @@
 #include "common/datatypes/DataSet.h"
 #include "common/datatypes/Edge.h"
 #include "graph/planner/plan/Query.h"
+#include "graph/util/Constants.h"
 #include "graph/util/FTIndexUtils.h"
 
 using nebula::storage::StorageClient;
@@ -31,7 +32,7 @@ folly::Future<Status> FulltextIndexScanExecutor::execute() {
   const auto& space = qctx()->rctx()->session()->space();
   if (!isIntVidType(space)) {
     if (ftIndexScan->isEdge()) {
-      DataSet edges({"edge"});
+      DataSet edges({"id", kScore});
       for (auto& item : esResultValue.items) {
         Edge edge;
         edge.src = item.src;
@@ -42,25 +43,26 @@ folly::Future<Status> FulltextIndexScanExecutor::execute() {
       }
       finish(ResultBuilder().value(Value(std::move(edges))).iter(Iterator::Kind::kProp).build());
     } else {
-      DataSet vertices({kVid});
+      DataSet vertices({"id", kScore});
       for (auto& item : esResultValue.items) {
         vertices.emplace_back(Row({item.vid, item.score}));
       }
       finish(ResultBuilder().value(Value(std::move(vertices))).iter(Iterator::Kind::kProp).build());
     }
   } else {
     if (ftIndexScan->isEdge()) {
-      DataSet edges({kSrc, kRank, kDst});
+      DataSet edges({"id", kScore});
       for (auto& item : esResultValue.items) {
-        std::string srcStr = item.src;
-        std::string dstStr = item.dst;
-        int64_t src = *reinterpret_cast<int64_t*>(srcStr.data());
-        int64_t dst = *reinterpret_cast<int64_t*>(dstStr.data());
-        edges.emplace_back(Row({src, item.rank, dst}));
+        Edge edge;
+        edge.src = item.src;
+        edge.dst = item.dst;
+        edge.ranking = item.rank;
+        edge.type = ftIndexScan->schemaId();
+        edges.emplace_back(Row({std::move(edge), item.score}));
       }
       finish(ResultBuilder().value(Value(std::move(edges))).iter(Iterator::Kind::kProp).build());
     } else {
-      DataSet vertices({kVid});
+      DataSet vertices({"id", kScore});
       for (auto& item : esResultValue.items) {
         std::string vidStr = item.vid;
         int64_t vid = *reinterpret_cast<int64_t*>(vidStr.data());
@@ -82,8 +84,12 @@ StatusOr<plugin::ESQueryResult> FulltextIndexScanExecutor::accessFulltextIndex(
       auto arg = tsExpr->arg();
       auto index = arg->index();
       auto query = arg->query();
-      int64_t offset = ftIndexScan->offset();
-      int64_t count = ftIndexScan->limit();
+      int64_t offset = ftIndexScan->getValidOffset();
+      auto limit = ftIndexScan->limit();
+      if (limit > std::numeric_limits<int32_t>::max()) {
+        limit = std::numeric_limits<int32_t>::max();
+      }
+      int64_t count = limit - offset;
       execFunc = [=, &esAdapter]() { return esAdapter.queryString(index, query, offset, count); };
       break;
     }

diff --git a/src/graph/executor/query/GetEdgesExecutor.cpp b/src/graph/executor/query/GetEdgesExecutor.cpp
@@ -93,7 +93,7 @@ folly::Future<Status> GetEdgesExecutor::getEdges() {
                  ge->exprs(),
                  ge->dedup(),
                  ge->orderBy(),
-                 ge->limit(qctx()),
+                 ge->getValidLimit(),
                  ge->filter())
       .via(runner())
       .ensure([this, getPropsTime]() {

diff --git a/src/graph/executor/query/GetVerticesExecutor.cpp b/src/graph/executor/query/GetVerticesExecutor.cpp
@@ -67,7 +67,7 @@ folly::Future<Status> GetVerticesExecutor::getVertices() {
                  gv->exprs(),
                  gv->dedup(),
                  gv->orderBy(),
-                 gv->limit(qctx()),
+                 gv->getValidLimit(),
                  gv->filter())
       .via(runner())
       .ensure([this, getPropsTime]() {

diff --git a/src/graph/planner/ngql/LookupPlanner.cpp b/src/graph/planner/ngql/LookupPlanner.cpp
@@ -65,22 +65,30 @@ StatusOr<SubPlan> LookupPlanner::transform(AstContext* astCtx) {
       auto rank = FunctionCallExpression::make(pool, "rank", {ColumnExpression::make(pool, 0)});
       auto dst = FunctionCallExpression::make(pool, "dst", {ColumnExpression::make(pool, 0)});
 
-      plan.root =
+      auto ge =
           GetEdges::make(qctx, plan.root, spaceId, src, type, rank, dst, std::move(edgeProps));
 
+      auto yieldColumns = pool->makeAndAdd<YieldColumns>();
+      yieldColumns->addColumn(new YieldColumn(EdgeExpression::make(pool)));
+      plan.root = Project::make(qctx, ge, yieldColumns);
+
       hashKeys = {ColumnExpression::make(pool, 0)};
-      probeKeys = {ColumnExpression::make(pool, 0)};
+      probeKeys = {EdgeExpression::make(pool)};
     } else {
       storage::cpp2::VertexProp vertexProp;
       vertexProp.tag_ref() = lookupCtx->schemaId;
       vertexProp.props_ref() = lookupCtx->idxReturnCols;
       auto vertexProps = std::make_unique<std::vector<storage::cpp2::VertexProp>>();
       vertexProps->emplace_back(std::move(vertexProp));
-      plan.root = GetVertices::make(
+      auto gv = GetVertices::make(
           qctx, plan.root, spaceId, ColumnExpression::make(pool, 0), std::move(vertexProps));
 
+      auto yieldColumns = pool->makeAndAdd<YieldColumns>();
+      yieldColumns->addColumn(new YieldColumn(VertexExpression::make(pool)));
+      plan.root = Project::make(qctx, gv, yieldColumns);
+
       hashKeys = {ColumnExpression::make(pool, 0)};
-      probeKeys = {FunctionCallExpression::make(pool, "id", {ColumnExpression::make(pool, 0)})};
+      probeKeys = {FunctionCallExpression::make(pool, "id", {VertexExpression::make(pool)})};
     }
 
     if (lookupCtx->hasScore) {

diff --git a/src/graph/planner/plan/Query.cpp b/src/graph/planner/plan/Query.cpp
@@ -19,10 +19,18 @@ using folly::stringPrintf;
 namespace nebula {
 namespace graph {
 
+int64_t Explore::getValidLimit() const {
+  auto limit = this->limit();
+  if (limit < 0) {
+    limit = std::numeric_limits<int64_t>::max();
+  }
+  return limit;
+}
+
 int64_t Explore::limit(QueryContext* qctx) const {
   DCHECK(ExpressionUtils::isEvaluableExpr(limit_, qctx));
   QueryExpressionContext ctx(qctx ? qctx->ectx() : nullptr);
-  return DCHECK_NOTNULL(limit_)->eval(ctx).getInt();
+  return limit(ctx);
 }
 
 std::unique_ptr<PlanNodeDescription> Explore::explain() const {

diff --git a/src/graph/planner/plan/Query.h b/src/graph/planner/plan/Query.h
@@ -22,6 +22,7 @@ namespace graph {
 //  GetVertices,
 //  GetEdges,
 //  IndexScan
+//  FulltextIndexScan
 class Explore : public SingleInputNode {
  public:
   GraphSpaceID space() const {
@@ -36,6 +37,8 @@ class Explore : public SingleInputNode {
     return dedup_;
   }
 
+  int64_t getValidLimit() const;
+
   // Get the constant limit value
   int64_t limit(QueryContext* qctx = nullptr) const;
 
@@ -441,7 +444,7 @@ class GetVertices : public Explore {
                            std::unique_ptr<std::vector<Expr>>&& exprs = nullptr,
                            bool dedup = false,
                            std::vector<storage::cpp2::OrderBy> orderBy = {},
-                           int64_t limit = std::numeric_limits<int64_t>::max(),
+                           int64_t limit = -1,
                            Expression* filter = nullptr) {
     return qctx->objPool()->makeAndAdd<GetVertices>(qctx,
                                                     Kind::kGetVertices,
@@ -547,7 +550,7 @@ class GetEdges final : public Explore {
                         std::unique_ptr<std::vector<EdgeProp>>&& props = nullptr,
                         std::unique_ptr<std::vector<Expr>>&& exprs = nullptr,
                         bool dedup = false,
-                        int64_t limit = std::numeric_limits<int64_t>::max(),
+                        int64_t limit = -1,
                         std::vector<storage::cpp2::OrderBy> orderBy = {},
                         Expression* filter = nullptr) {
     return qctx->objPool()->makeAndAdd<GetEdges>(qctx,
@@ -767,6 +770,10 @@ class FulltextIndexScan : public Explore {
     return isEdge_;
   }
 
+  int64_t getValidOffset() const {
+    return offset_ >= 0 ? offset_ : 0;
+  }
+
   int64_t offset() const {
     return offset_;
   }

diff --git a/src/graph/util/ExpressionUtils.cpp b/src/graph/util/ExpressionUtils.cpp
@@ -190,6 +190,13 @@ Expression *ExpressionUtils::rewriteEdgePropFunc2LabelAttribute(
   return RewriteVisitor::transform(expr, std::move(matcher), std::move(rewriter));
 }
 
+Expression *ExpressionUtils::rewriteLabelAttr2PropExpr(const Expression *expr, bool isEdge) {
+  if (isEdge) {
+    return rewriteLabelAttr2EdgeProp(expr);
+  }
+  return rewriteLabelAttr2TagProp(expr);
+}
+
 Expression *ExpressionUtils::rewriteLabelAttr2TagProp(const Expression *expr) {
   ObjectPool *pool = expr->getObjPool();
   auto matcher = [](const Expression *e) -> bool {

diff --git a/src/graph/util/ExpressionUtils.h b/src/graph/util/ExpressionUtils.h
@@ -71,6 +71,9 @@ class ExpressionUtils {
   static Expression* rewriteEdgePropFunc2LabelAttribute(
       const Expression* expr, const std::unordered_map<std::string, AliasType>& aliasTypeMap);
 
+  // rewrite LabelAttr expr to property expr
+  static Expression* rewriteLabelAttr2PropExpr(const Expression* expr, bool isEdge);
+
   // rewrite LabelAttr to tagProp
   static Expression* rewriteLabelAttr2TagProp(const Expression* expr);