← Back to team overview

maria-developers team mailing list archive

Re: A fix for MDEV-5689 ExtractValue(xml, 'substring(/x, /y)') crashes

 

Hi, Alexander!

On Feb 20, Alexander Barkov wrote:
> please review a fix for MDEV-5689.
> 
> It also fixes
> 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.

Good, please put this in a changeset comment.

> === modified file 'sql/item_xmlfunc.h'
> --- sql/item_xmlfunc.h	2013-11-28 21:35:59 +0000
> +++ sql/item_xmlfunc.h	2014-02-20 13:45:36 +0000
> @@ -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

How's that an "Array of MY_XML_NODEs", if it's just a String?

> +    bool parse();

Ok. I'm confused. In some places it looks like m_parsed_buf is, indeed,
an array of MY_XML_NODEs. In other places it looks like it's a string.
How comes?

Regards,
Sergei



Follow ups

References