Initial review of xslt with no limits patch
in [PgSql]
|
Prev: english parser in text search: support for multiplewords in the same position
Next: [HACKERS] Postgres as Historian
From: Pavel Stehule on 2 Aug 2010 04:21 Hello 2010/8/2 Mike Fowler <mike(a)mlfowler.com>: > Hi Pavel, > > Currently your patch isn't applying to head, from the looks of things a > function signature has changed. Can you update your patch please? > yes - see attachment > Also, having had a read through the patch itself I note that there are no > tests and no changes to documentation. Shouldn't the documentation advertise > that the there are no limits on the numbers of parameters? A couple of tests > will also help me to test your patch, > the limit of 10 parameters was not documented. With this patch, there are not limit - or limit comes from libxml2. Test are a problem - for me - I don't understand to xslt too much - so I am not able to write xslt template with more than 10 params. I looked there and I the params are not tested now - so it is necessary to write a new set of regress tests. But I am not able to do it :(. > Below is the results of running patch: > > patch -p0 < ../nolimits.diff > patching file ./contrib/xml2/xslt_proc.c > Hunk #1 FAILED at 42. > Hunk #2 succeeded at 57 (offset -2 lines). > Hunk #3 succeeded at 69 (offset -2 lines). > Hunk #4 succeeded at 142 (offset -4 lines). > Hunk #5 succeeded at 179 (offset -4 lines). > Hunk #6 succeeded at 192 with fuzz 1 (offset -4 lines). > 1 out of 6 hunks FAILED -- saving rejects to file > ./contrib/xml2/xslt_proc.c.rej > > The rejects were: > > > *** ./contrib/xml2/xslt_proc.c.orig   2010-03-03 20:10:22.000000000 +0100 > --- ./contrib/xml2/xslt_proc.c  2010-05-03 15:07:17.010918303 +0200 > *************** > *** 42,50 **** >  extern void pgxml_parser_init(void); > >  /* local defs */ > ! static void parse_params(const char **params, text *paramstr); > > ! #define MAXPARAMS 20          /* must be even, see parse_params() > */ > >  #endif /* USE_LIBXSLT */ > > --- 42,51 ---- >  extern void pgxml_parser_init(void); > >  /* local defs */ > ! const char **parse_params(text *paramstr); > > ! #define INIT_PARAMS 20             /* must be even, see > parse_params() */ > ! #define EXTEND_PARAMS 20        /* must be even, see parse_params() > */ > >  #endif /* USE_LIBXSLT */ > > > Regards, > -- > Mike Fowler > Registered Linux user: 379787 > Regards Pavel Stehule
From: Mike Fowler on 5 Aug 2010 18:56 Hi Pavel, On 02/08/10 09:21, Pavel Stehule wrote: > Hello > > 2010/8/2 Mike Fowler<mike(a)mlfowler.com>: >> Hi Pavel, >> >> Currently your patch isn't applying to head, from the looks of things a >> function signature has changed. Can you update your patch please? >> > > yes - see attachment > Thanks, the new patch applies cleanly. However I've been attempting to run the parameterised XSLT this evening but to no avail. Reverting your code I've discovered that it does not work in the old version either. Given the complete lack of documentation (not your fault) it's always possible that I'm doing something wrong. Given the query below, you should get the XML that follows, and indeed in oXygen (a standalone XML tool) you do: SELECT xslt_process('<employee><name>cim</name><age>30</age><pay>400</pay></employee>'::text, $$<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:param name="n2"/> <xsl:param name="n3"/> <xsl:param name="n4"/> <xsl:param name="n5" select="'me'"/> <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 name="sample"> <xsl:value-of select="$n3"/> </xsl:element> <xsl:element name="sample"> <xsl:value-of select="$n4"/> </xsl:element> <xsl:element name="sample"> <xsl:value-of select="$n5"/> </xsl:element> </xsl:element> </xsl:template> </xsl:stylesheet>$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) <samples> <sample>v1</sample> <sample>v2</sample> <sample>v3</sample> <sample>v4</sample> <sample>v5</sample> </samples> Sadly I get the following in both versions: <samples> <sample/> <sample/> <sample/> <sample/> <sample/> </samples> Unless you can see anything I'm doing wrong I suggest we mark this patch either 'Returned with feedback' or 'Rejected'. Since contrib/xml2 is deprecated, perhaps a better way forward is to pull XSLT handling into core and fix both the apparent inability to handle parameters as well as the limited number of parameters. Regards, -- Mike Fowler Registered Linux user: 379787 -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Pavel Stehule on 5 Aug 2010 23:11 2010/8/6 Mike Fowler <mike(a)mlfowler.com>: > Hi Pavel, > > On 02/08/10 09:21, Pavel Stehule wrote: >> >> Hello >> >> 2010/8/2 Mike Fowler<mike(a)mlfowler.com>: >>> >>> Hi Pavel, >>> >>> Currently your patch isn't applying to head, from the looks of things a >>> function signature has changed. Can you update your patch please? >>> >> >> yes - see attachment >> > > Thanks, the new patch applies cleanly. However I've been attempting to run > the parameterised XSLT this evening but to no avail. Reverting your code > I've discovered that it does not work in the old version either. > > Given the complete lack of documentation (not your fault) it's always > possible that I'm doing something wrong. Given the query below, you should > get the XML that follows, and indeed in oXygen (a standalone XML tool) you > do: > > SELECT > xslt_process('<employee><name>cim</name><age>30</age><pay>400</pay></employee>'::text, > $$<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:param name="n2"/> > <xsl:param name="n3"/> > <xsl:param name="n4"/> > <xsl:param name="n5" select="'me'"/> > <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 name="sample"> > <xsl:value-of select="$n3"/> > </xsl:element> > <xsl:element name="sample"> > <xsl:value-of select="$n4"/> > </xsl:element> > <xsl:element name="sample"> > <xsl:value-of select="$n5"/> > </xsl:element> > </xsl:element> > </xsl:template> > </xsl:stylesheet>$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) > > <samples> > <sample>v1</sample> > <sample>v2</sample> > <sample>v3</sample> > <sample>v4</sample> > <sample>v5</sample> > </samples> > > Sadly I get the following in both versions: > > <samples> > <sample/> > <sample/> > <sample/> > <sample/> > <sample/> > </samples> > > > Unless you can see anything I'm doing wrong I suggest we mark this patch > either 'Returned with feedback' or 'Rejected'. Since contrib/xml2 is > deprecated, perhaps a better way forward is to pull XSLT handling into core > and fix both the apparent inability to handle parameters as well as the > limited number of parameters. there is some wrong, but I am not able to sey what now. But this patch is very simply. I'll fix it today. Pavel > > Regards, > > -- > Mike Fowler > Registered Linux user: 379787 > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Andrew Dunstan on 5 Aug 2010 23:37 On 08/05/2010 06:56 PM, Mike Fowler wrote: > > SELECT > xslt_process('<employee><name>cim</name><age>30</age><pay>400</pay></employee>'::text, > $$<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" > version="1.0"> > <xsl:output method="xml" omit-xml-declaration="yes" indent="yes"/> > [snip] > </xsl:stylesheet>$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) > > I haven't been paying attention to this, so sorry if this has been discussed before, but it just caught my eye. Why are we passing these params as a comma-separated list rather than as an array or as a variadic list of params? This looks rather ugly. What if you want to have a param that includes a comma? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Pavel Stehule on 5 Aug 2010 23:57
2010/8/6 Andrew Dunstan <andrew(a)dunslane.net>: > > > On 08/05/2010 06:56 PM, Mike Fowler wrote: >> >> SELECT >> xslt_process('<employee><name>cim</name><age>30</age><pay>400</pay></employee>'::text, >> $$<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform" >> version="1.0"> >> <xsl:output method="xml" omit-xml-declaration="yes" indent="yes"/> >> > [snip] >> >> </xsl:stylesheet>$$::text, 'n1=v1,n2=v2,n3=v3,n4=v4,n5=v5'::text) >> >> > > I haven't been paying attention to this, so sorry if this has been discussed > before, but it just caught my eye. Why are we passing these params as a > comma-separated list rather than as an array or as a variadic list of > params? This looks rather ugly. What if you want to have a param that > includes a comma? > There is probably problem in pairs - label = value. Can be nice, if we can use a variadic functions for this, but I am afraid, ... using a variadic function isn't too much nice now some xslt_process(xmlsrc, 'n1=v1','n2=v2','n3=v3' The same is true for array. Pg hasn't hash available from SQL level I am thinking about new kind of functions - with only positionals arguments. And internal parameter can be a array of used labels. Regards Pavel Stehule > cheers > > andrew > -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |