A joint patch for:

- MDEV-5689 ExtractValue(xml, 'substring(/x,/y)') crashes
- MDEV-5709 ExtractValue() with XPath variable references returns wrong result.

Description:

1. The main problem was that that nodeset_func->fix_fields() was
called in Item_func_xml_extractvalue::val_str() and
Item_func_xml_update::val_str(), which led in some cases to
execution of the XPath engine *before* having a parsed XML value.
Moved to Item_xml_str_func::fix_fields().

2. Cleanup: added a new method Item_xml_str_func::fix_fields() and moved
most of the code from Item_xml_str_func::fix_length_and_dec()
to Item_xml_str_func::fix_fields(), to follow the usual Item layout.

3. Cleanup: a parsed XML value is useless without the raw XML value
it was built from.

Previously the parsed and the raw values where stored in separate String
instances. It was hard to follow how they are synchronized.
Added a helper class XML which contains both parsed and raw values.
Makes things easier to read and modify.

4. MDEV-5709: const_item() could incorrectly return a "true"
result when XPath expression contains users/SP variable references.
Now nodeset_func->const_item() is also taken into account to
catch such cases.

5. Minor code enhancements.

1 files changed, 53 insertions, 2 deletions

diff --git a/sql/item_xmlfunc.h b/sql/item_xmlfunc.h
index e818a6da408..637f505e12e 100644
--- a/sql/item_xmlfunc.h
+++ b/sql/item_xmlfunc.h
@@ -26,11 +26,55 @@
 #endif
 
 
+typedef struct my_xml_node_st MY_XML_NODE;
+
+
 class Item_xml_str_func: public Item_str_func
 {
 protected:
-  String tmp_value, pxml;
+  /*
+    A helper class to store raw and parsed XML.
+  */
+  class XML
+  {
+    bool m_cached;
+    String *m_raw_ptr;   // Pointer to text representation
+    String m_raw_buf;    // Cached text representation
+    String m_parsed_buf; // Array of MY_XML_NODEs, pointing to raw_buffer
+    bool parse();
+    void reset()
+    {
+      m_cached= false;
+      m_raw_ptr= (String *) 0;
+    }
+  public:
+    XML() { reset(); }
+    void set_charset(CHARSET_INFO *cs) { m_parsed_buf.set_charset(cs); }
+    String *raw() { return m_raw_ptr; }
+    String *parsed() { return &m_parsed_buf; }
+    const MY_XML_NODE *node(uint idx);
+    bool cached() { return m_cached; }
+    bool parse(String *raw, bool cache);
+    bool parse(Item *item, bool cache)
+    {
+      String *res;
+      if (!(res= item->val_str(&m_raw_buf)))
+      {
+        m_raw_ptr= (String *) 0;
+        m_cached= cache;
+        return true;
+      }
+      return parse(res, cache);
+    }
+  };
   Item *nodeset_func;
+  XML xml;
+  bool get_xml(XML *xml, bool cache= false)
+  {
+    if (!cache && xml->cached())
+      return xml->raw() == 0;
+    return xml->parse(args[0], cache);
+  }
 public:
   Item_xml_str_func(Item *a, Item *b): 
     Item_str_func(a,b) 
@@ -42,8 +86,12 @@ public:
   {
     maybe_null= TRUE;
   }
+  bool fix_fields(THD *thd, Item **ref);
   void fix_length_and_dec();
-  String *parse_xml(String *raw_xml, String *parsed_xml_buf);
+  bool const_item() const
+  {
+    return const_item_cache && (!nodeset_func || nodeset_func->const_item());
+  }
   bool check_vcol_func_processor(uchar *int_arg) 
   {
     return trace_unsupported_by_check_vcol_func_processor(func_name());
@@ -63,6 +111,9 @@ public:
 class Item_func_xml_update: public Item_xml_str_func
 {
   String tmp_value2, tmp_value3;
+  bool collect_result(String *str,
+                      const MY_XML_NODE *cut,
+                      const String *replace);
 public:
   Item_func_xml_update(Item *a,Item *b,Item *c) :Item_xml_str_func(a,b,c) {}
   const char *func_name() const { return "updatexml"; }