Re: Initial review of xslt with no limits patch

From: Mike Fowler <mike(at)mlfowler(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Initial review of xslt with no limits patch
Date: 2010-08-08 15:36:25
Message-ID: 4C5ECEF9.3030806@mlfowler.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 06/08/10 17:50, Pavel Stehule wrote:
>
> attached updated patch with regression test
>

>

Bravely ignoring the quotation/varidic/<favourite_scheme_here>
conversations, I've taken a look at the patch as is. Thanks to Tom's
input I can now correctly drive the function. I can also report that
code is now behaving in the expected way.

I have two other observations, more directed at the community than Pavel:

1) XML2 is largely undocumented, giving rise to the problems
encountered. Since the module is deprecated anyways, does it make more
sense to get xslt handling moved into core and get it fully documented?

2) Pavel's regression test exposes a bug in libxslt. The stylesheet
declares 5 parameters, but uses 12. Simplifying, take the stylesheet:

<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
version="1.0">
<xsl:output method="xml" omit-xml-declaration="yes" indent="yes"/>
<xsl:strip-space elements="*"/>
<xsl:param name="n1"/>
<xsl:template match="*">
<xsl:element name="samples">
<xsl:element name="sample">
<xsl:value-of select="$n1"/>
</xsl:element>
<xsl:element name="sample">
<xsl:value-of select="$n2"/>
</xsl:element>
</xsl:element>
</xsl:template>
</xsl:stylesheet>

and run the command:

~/temp$ xsltproc --stringparam n2 "v2" Untitled2.xsl Untitled1.xml
<samples>
<sample/>
<sample>v2</sample>
</samples>

All looks good. However if you run:

~/temp$ xsltproc --stringparam n1 "v1" Untitled2.xsl Untitled1.xml
runtime error: file Untitled2.xsl line 28 element value-of
Variable 'n2' has not been declared.
xmlXPathCompiledEval: evaluation failed
runtime error: file Untitled2.xsl line 28 element value-of
XPath evaluation returned no result.
<samples>
<sample>v1</sample>
<sample/>
</samples>

The xslt_process function ignores these errors and returns cleanly.

To summarize, the bug in libxslt is that it allows the processing of
unnamed parameters - most other parsers would reject this stylesheet.
Secondly, the xslt_process does not return the errors reported when
running without passing the unnamed parameter.

Personally I would like to see this documented and moved to core so that
the whole of xml2 can be dropped. I also think that the errors should be
reported, even if libxslt doesn't behave properly in all scenarios.

Of course there's that whole other issue around how you pass the
parameters in the first place that needs resolving too...

Regards,
--
Mike Fowler
Registered Linux user: 379787

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Fetter 2010-08-08 15:40:13 Re: pg_stat_user_functions' notion of user
Previous Message Kevin Grittner 2010-08-08 14:11:52 Re: Surprising dead_tuple_count from pgstattuple