rgw: support more meaningful compilation error string

in es module

Signed-off-by: Yehuda Sadeh <yehuda@redhat.com>

diff --git a/src/rgw/rgw_es_main.cc b/src/rgw/rgw_es_main.cc
index fb705193624e13cbd34cc7959ac88d4754bc6daf..2eda54dfa966c61e511a2becf5007b5debd469a0 100644 (file)
--- a/src/rgw/rgw_es_main.cc
+++ b/src/rgw/rgw_es_main.cc
@@ -46,10 +46,12 @@ int main(int argc, char *argv[])
                                                           {"date", ESEntityTypeMap::ES_ENTITY_DATE} };
   ESEntityTypeMap em(custom_map);
   es_query.set_custom_type_map(&em);
+
+  string err;
   
-  bool valid = es_query.compile();
+  bool valid = es_query.compile(&err);
   if (!valid) {
-    cout << "invalid query, failed generating request json" << std::endl;
+    cout << "failed to compile query: " << err << std::endl;
     return EINVAL;
   }
 

diff --git a/src/rgw/rgw_es_query.cc b/src/rgw/rgw_es_query.cc
index 58c2b5eb77d5b1404f3077270a2a7a4b3d3d8ab8..bff26ffe6d215bb866bedacffac2666cff87d47b 100644 (file)
--- a/src/rgw/rgw_es_query.cc
+++ b/src/rgw/rgw_es_query.cc
@@ -113,12 +113,12 @@ public:
   ESQueryNode(ESQueryCompiler *_compiler) : compiler(_compiler) {}
   virtual ~ESQueryNode() {}
 
-  virtual bool init(ESQueryStack *s, ESQueryNode **pnode) = 0;
+  virtual bool init(ESQueryStack *s, ESQueryNode **pnode, string *perr) = 0;
 
   virtual void dump(Formatter *f) const = 0;
 };
 
-static bool alloc_node(ESQueryCompiler *compiler, ESQueryStack *s, ESQueryNode **pnode);
+static bool alloc_node(ESQueryCompiler *compiler, ESQueryStack *s, ESQueryNode **pnode, string *perr);
 
 class ESQueryNode_Bool : public ESQueryNode {
   string op;
@@ -127,13 +127,14 @@ class ESQueryNode_Bool : public ESQueryNode {
 public:
   ESQueryNode_Bool(ESQueryCompiler *compiler) : ESQueryNode(compiler) {}
   ESQueryNode_Bool(ESQueryCompiler *compiler, const string& _op, ESQueryNode *_first, ESQueryNode *_second) :ESQueryNode(compiler), op(_op), first(_first), second(_second) {}
-  bool init(ESQueryStack *s, ESQueryNode **pnode) override {
+  bool init(ESQueryStack *s, ESQueryNode **pnode, string *perr) override {
     bool valid = s->pop(&op);
     if (!valid) {
+      *perr = "incorrect expression";
       return false;
     }
-    valid = alloc_node(compiler, s, &first) &&
-      alloc_node(compiler, s, &second);
+    valid = alloc_node(compiler, s, &first, perr) &&
+      alloc_node(compiler, s, &second, perr);
     if (!valid) {
       return false;
     }
@@ -162,7 +163,7 @@ public:
   ESQueryNodeLeafVal() = default;
   virtual ~ESQueryNodeLeafVal() {}
 
-  virtual bool init(const string& str_val) = 0;
+  virtual bool init(const string& str_val, string *perr) = 0;
   virtual void encode_json(const string& field, Formatter *f) const  = 0;
 };
 
@@ -170,7 +171,7 @@ class ESQueryNodeLeafVal_Str : public ESQueryNodeLeafVal {
   string val;
 public:
   ESQueryNodeLeafVal_Str() {}
-  bool init(const string& str_val) override {
+  bool init(const string& str_val, string *perr) override {
     val = str_val;
     return true;
   }
@@ -183,10 +184,11 @@ class ESQueryNodeLeafVal_Int : public ESQueryNodeLeafVal {
   int64_t val;
 public:
   ESQueryNodeLeafVal_Int() {}
-  bool init(const string& str_val) override {
+  bool init(const string& str_val, string *perr) override {
     string err;
     val = strict_strtoll(str_val.c_str(), 10, &err);
     if (!err.empty()) {
+      *perr = string("failed to parse integer: ") + err;
       return false;
     }
     return true;
@@ -200,8 +202,9 @@ class ESQueryNodeLeafVal_Date : public ESQueryNodeLeafVal {
   ceph::real_time val;
 public:
   ESQueryNodeLeafVal_Date() {}
-  bool init(const string& str_val) override {
+  bool init(const string& str_val, string *perr) override {
     if (parse_time(str_val.c_str(), &val) < 0) {
+      *perr = string("failed to parse date: ") + str_val;
       return false;
     }
     return true;
@@ -219,7 +222,7 @@ protected:
   ESQueryNodeLeafVal *val{nullptr};
   ESEntityTypeMap::EntityType entity_type{ESEntityTypeMap::ES_ENTITY_NONE};
 
-  bool val_from_str(const string& str_val) {
+  bool val_from_str(const string& str_val, string *perr) {
     switch (entity_type) {
       case ESEntityTypeMap::ES_ENTITY_DATE:
         val = new ESQueryNodeLeafVal_Date;
@@ -230,32 +233,33 @@ protected:
       default:
         val = new ESQueryNodeLeafVal_Str;
     }
-    return val->init(str_val);
+    return val->init(str_val, perr);
   }
 public:
   ESQueryNode_Op(ESQueryCompiler *compiler) : ESQueryNode(compiler) {}
   ~ESQueryNode_Op() {
     delete val;
   }
-  virtual bool init(ESQueryStack *s, ESQueryNode **pnode) override {
+  virtual bool init(ESQueryStack *s, ESQueryNode **pnode, string *perr) override {
     string str_val;
     bool valid = s->pop(&op) &&
       s->pop(&str_val) &&
       s->pop(&field);
     if (!valid) {
+      *perr = "invalid expression";
       return false;
     }
     ESQueryNode *effective_node;
-    if (!handle_nested(&effective_node)) {
+    if (!handle_nested(&effective_node, perr)) {
       return false;
     }
-    if (!val_from_str(str_val)) {
+    if (!val_from_str(str_val, perr)) {
       return false;
     }
     *pnode = effective_node;
     return true;
   }
-  bool handle_nested(ESQueryNode **pnode);
+  bool handle_nested(ESQueryNode **pnode, string *perr);
 
   virtual void dump(Formatter *f) const = 0;
 };
@@ -270,11 +274,11 @@ public:
     str_val = v;
   }
 
-  bool init(ESQueryStack *s, ESQueryNode **pnode) override {
+  bool init(ESQueryStack *s, ESQueryNode **pnode, string *perr) override {
     if (op.empty()) {
-      return ESQueryNode_Op::init(s, pnode);
+      return ESQueryNode_Op::init(s, pnode, perr);
     }
-    return val_from_str(str_val);
+    return val_from_str(str_val, perr);
   }
 
   virtual void dump(Formatter *f) const {
@@ -357,7 +361,7 @@ string ESQueryNode_Op_Nested<ceph::real_time>::type_str() const {
   return "date";
 }
 
-bool ESQueryNode_Op::handle_nested(ESQueryNode **pnode)
+bool ESQueryNode_Op::handle_nested(ESQueryNode **pnode, string *perr)
 {
   string field_name = field;
   const string& custom_prefix = compiler->get_custom_prefix();
@@ -365,8 +369,13 @@ bool ESQueryNode_Op::handle_nested(ESQueryNode **pnode)
     *pnode = this;
     auto m = compiler->get_generic_type_map();
     if (m) {
-      return m->find(field_name, &entity_type);
+      bool found = m->find(field_name, &entity_type);
+      if (!found) {
+        *perr = string("unexpected generic field '") + field_name + "'";
+      }
+      return found;
     }
+    *perr = "query parser does not support generic types";
     return false;
   }
 
@@ -400,11 +409,12 @@ static bool is_bool_op(const string& str)
   return (str == "or" || str == "and");
 }
 
-static bool alloc_node(ESQueryCompiler *compiler, ESQueryStack *s, ESQueryNode **pnode)
+static bool alloc_node(ESQueryCompiler *compiler, ESQueryStack *s, ESQueryNode **pnode, string *perr)
 {
   string op;
   bool valid = s->peek(&op);
   if (!valid) {
+    *perr = "incorrect expression";
     return false;
   }
 
@@ -424,13 +434,14 @@ static bool alloc_node(ESQueryCompiler *compiler, ESQueryStack *s, ESQueryNode *
 
     auto iter = range_op_map.find(op);
     if (iter == range_op_map.end()) {
+      *perr = string("invalid operator: ") + op;
       return false;
     }
 
     node = new ESQueryNode_Op_Range(compiler, iter->second);
   }
 
-  if (!node->init(s, pnode)) {
+  if (!node->init(s, pnode, perr)) {
     delete node;
     return false;
   }
@@ -583,16 +594,18 @@ bool ESInfixQueryParser::parse(list<string> *result) {
   return true;
 }
 
-bool ESQueryCompiler::convert(list<string>& infix) {
+bool ESQueryCompiler::convert(list<string>& infix, string *perr) {
   list<string> prefix;
   if (!infix_to_prefix(infix, &prefix)) {
+    *perr = "invalid query";
     return false;
   }
   stack.assign(prefix);
-  if (!alloc_node(this, &stack, &query_root)) {
+  if (!alloc_node(this, &stack, &query_root, perr)) {
     return false;
   }
   if (!stack.done()) {
+    *perr = "invalid query";
     return false;
   }
   return true;
@@ -602,13 +615,14 @@ ESQueryCompiler::~ESQueryCompiler() {
   delete query_root;
 }
 
-bool ESQueryCompiler::compile() {
+bool ESQueryCompiler::compile(string *perr) {
   list<string> infix;
   if (!parser.parse(&infix)) {
+    *perr = "failed to parse query";
     return false;
   }
 
-  if (!convert(infix)) {
+  if (!convert(infix, perr)) {
     return false;
   }
 

diff --git a/src/rgw/rgw_es_query.h b/src/rgw/rgw_es_query.h
index 913c95e097ca6e8a11b21533897b809b185c3867..d162d2568f03ff5e183ed8f2661754b0103ebcfe 100644 (file)
--- a/src/rgw/rgw_es_query.h
+++ b/src/rgw/rgw_es_query.h
@@ -93,7 +93,7 @@ class ESQueryCompiler {
 
   string custom_prefix;
 
-  bool convert(list<string>& infix);
+  bool convert(list<string>& infix, string *perr);
 
   list<pair<string, string> > eq_conds;
 
@@ -108,7 +108,7 @@ public:
   }
   ~ESQueryCompiler();
 
-  bool compile();
+  bool compile(string *perr);
   void dump(Formatter *f) const;
   
   void set_generic_type_map(ESEntityTypeMap *entity_map) {

diff --git a/src/rgw/rgw_sync_module_es_rest.cc b/src/rgw/rgw_sync_module_es_rest.cc
index 2d7077d4a9c3e29cccf8d91b6f2bfcee1e01a628..20d8702451dde4827f76f63b936d09fd9b1f93ac 100644 (file)
--- a/src/rgw/rgw_sync_module_es_rest.cc
+++ b/src/rgw/rgw_sync_module_es_rest.cc
@@ -109,6 +109,7 @@ protected:
   uint64_t marker{0};
   string next_marker;
   bool is_truncated{false};
+  string err;
 
   es_search_response response;
 
@@ -149,7 +150,7 @@ void RGWMetadataSearchOp::execute()
 
   ESQueryCompiler es_query(expression, &conds, custom_prefix);
   
-  bool valid = es_query.compile();
+  bool valid = es_query.compile(&err);
   if (!valid) {
     ldout(s->cct, 10) << "invalid query, failed generating request json" << dendl;
     op_ret = -EINVAL;
@@ -240,8 +241,10 @@ public:
     return 0;
   }
   void send_response() override {
-    if (op_ret)
+    if (op_ret) {
+      s->err.message = err;
       set_req_state_err(s, op_ret);
+    }
     dump_errno(s);
     end_header(s, this, "application/xml");