Re: [HACKERS] xpath improvement suggestion

[Arie Bikker]

Robert Haas wrote: On Tue, Jan 5, 2010 at 6:09 PM, Arie Bikker <a...@abikker.nl> wrote: Hi all, Well I had  to burn some midnight oil trying to figure out why a construct like SELECT xpath('name()','<a/>'); doesn't give the expected result. Kept getting an empty array:  xpath ------------- {} instead of the expected "{a}" BugID 4294 and the TODO item "better handling of XPath data types" pointed in the right direction. whithin src/backend/utils/adt/xml.c in the function xpath the result of the call to xmlXPathCompiledEval is not handled optimally. In fact, the result is assumed to be a nodeset without consulting the ->type member of the result. I've made some minor changes to xml.c to handle some non-nodeset results of xmlXPathCompiledEval. Essentially, the revised code makes an array of all the nodes in the xpathobj result in case this is a nodeset, or an array with a single element in case the reult is a number/string/boolean. The problem cases mentioned in http://archives.postgresql.org/pgsql-hackers/2008-06/msg00616.php now work as expected. Revision of the code involves: - A switch statement to handle the result type of xmlXPathCompiledEval. - an additional function xmlpathobjtoxmltype. diff of the revisioned code with respect to original is in attached file. kind regards, Arie Bikker Hi, Could you please resend this as a context diff and add it to our patch management application? http://wiki.postgresql.org/wiki/Submitting_a_Patch https://commitfest.postgresql.org/action/commitfest_view/open Thanks! ...Robert Hope this is the right attachement type (I'm new at this) BTW. here a some nice examples: - Get the number of attributes of the first childnode: select ( xpath('count(@*)',(xpath('*[1]','<a b="c"><d e="f" g="j"/></a>'))[1]))[1]; - an alternative for xpath_exist('/a/d') select (xpath('boolean(/a/d)','<a b="c"><d e="f" g="j"/></a>'))[1]; - fixes bug 4206 select xpath('//text()',xmlparse(document '<?xml version="1.0"?><elem1><elem2>one</elem2><elem2>two</elem2><elem2>three</elem2><elem3att="2"/></elem1>')); - fixes bug 4294 select xpath('name(/my:a/*[last()])', '<a xmlns="http://myns.com/ns"><b>text1</b><c>text2</c></a>', ARRAY[ARRAY['my','http://myns.com/ns']]); kind regards, Arie Bikker

*** src/backend/utils/adt/xml.c.old	Fri Sep  4 12:49:43 2009
--- src/backend/utils/adt/xml.c	Wed Jan  6 21:32:22 2010
***************
*** 111,116 ****
--- 111,117 ----
  static xmlDocPtr xml_parse(text *data, XmlOptionType xmloption_arg,
  		  bool preserve_whitespace, int encoding);
  static text *xml_xmlnodetoxmltype(xmlNodePtr cur);
+ static text *xml_xmlpathobjtoxmltype(xmlXPathObjectPtr cur);
  #endif   /* USE_LIBXML */
  
  static StringInfo query_to_xml_internal(const char *query, char *tablename,
***************
*** 3265,3270 ****
--- 3266,3312 ----
  
  	return result;
  }
+ 
+ /*
+  * Convert XML pathobject to text for non-nodeset objects
+  */
+ static text *
+ xml_xmlpathobjtoxmltype(xmlXPathObjectPtr cur)
+ {
+ 	xmltype    *result;
+ 
+ 	if (cur->type == XPATH_BOOLEAN)
+ 	{
+ 		PG_TRY();
+ 		{
+ 			result = cstring_to_text((char *)(xmlXPathCastToBoolean(cur)?"t":"f"));
+ 		}
+ 		PG_CATCH();
+ 		{
+ 			PG_RE_THROW();
+ 		}
+ 		PG_END_TRY();
+ 	}
+ 	else
+ 	{
+ 		xmlChar    *str;
+ 
+ 		str = xmlXPathCastToString(cur);
+ 		PG_TRY();
+ 		{
+ 			result = (xmltype *) cstring_to_text((char *) str);
+ 		}
+ 		PG_CATCH();
+ 		{
+ 			xmlFree(str);
+ 			PG_RE_THROW();
+ 		}
+ 		PG_END_TRY();
+ 		xmlFree(str);
+ 	}
+ 
+ 	return result;
+ }
  #endif
  
  
***************
*** 3418,3442 ****
  			xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
  						"could not create XPath object");
  
! 		/* return empty array in cases when nothing is found */
! 		if (xpathobj->nodesetval == NULL)
! 			res_nitems = 0;
! 		else
! 			res_nitems = xpathobj->nodesetval->nodeNr;
! 
! 		if (res_nitems)
! 		{
! 			for (i = 0; i < xpathobj->nodesetval->nodeNr; i++)
  			{
! 				Datum		elem;
! 				bool		elemisnull = false;
! 
! 				elem = PointerGetDatum(xml_xmlnodetoxmltype(xpathobj->nodesetval->nodeTab[i]));
! 				astate = accumArrayResult(astate, elem,
! 										  elemisnull, XMLOID,
! 										  CurrentMemoryContext);
  			}
  		}
  	}
  	PG_CATCH();
  	{
--- 3460,3504 ----
  			xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
  						"could not create XPath object");
  
! 		switch (xpathobj->type) {
! 		case XPATH_NODESET: {
! 			/* return empty array in cases when nothing is found */
! 			if (xpathobj->nodesetval == NULL)
! 				res_nitems = 0;
! 			else
! 				res_nitems = xpathobj->nodesetval->nodeNr;
! 	
! 			if (res_nitems)
  			{
! 				for (i = 0; i < xpathobj->nodesetval->nodeNr; i++)
! 				{
! 					Datum		elem;
! 					bool		elemisnull = false;
! 	
! 					elem = PointerGetDatum(xml_xmlnodetoxmltype(xpathobj->nodesetval->nodeTab[i]));
! 					astate = accumArrayResult(astate, elem,
! 										  	elemisnull, XMLOID,
! 										  	CurrentMemoryContext);
! 				}
  			}
+ 			break;
  		}
+ 		case XPATH_BOOLEAN:
+ 		case XPATH_NUMBER:
+ 		case XPATH_STRING: {
+ 			Datum		elem;
+ 			bool		elemisnull = false;
+ 	
+ 			elem = PointerGetDatum(xml_xmlpathobjtoxmltype(xpathobj));
+ 			astate = accumArrayResult(astate, elem,
+ 						  	elemisnull, XMLOID,
+ 						  	CurrentMemoryContext);
+ 			break;
+ 		}
+ 		default: {
+ 			ereport(WARNING, (errmsg("Unknown PathObjectType (%d)", xpathobj->type)));
+ 		}
+ 		}
  	}
  	PG_CATCH();
  	{
***************
*** 3460,3466 ****
  	xmlFreeDoc(doc);
  	xmlFreeParserCtxt(ctxt);
  
! 	if (res_nitems == 0)
  		PG_RETURN_ARRAYTYPE_P(construct_empty_array(XMLOID));
  	else
  		PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate, CurrentMemoryContext));
--- 3522,3528 ----
  	xmlFreeDoc(doc);
  	xmlFreeParserCtxt(ctxt);
  
! 	if (astate == NULL)
  		PG_RETURN_ARRAYTYPE_P(construct_empty_array(XMLOID));
  	else
  		PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate, CurrentMemoryContext));


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers